[tor-commits] [orbot/master] Used static inner classes for AsyncTasks in OrbotMainActivity, BridgeWizardActivity

n8fr8 at torproject.org n8fr8 at torproject.org
Wed Oct 7 12:37:35 UTC 2020


commit 4d5341e3de6465eba5ced69efde96673ff6de2d6
Author: bim <dsnake at protonmail.com>
Date:   Wed Sep 23 23:25:28 2020 -0400

    Used static inner classes for AsyncTasks in OrbotMainActivity, BridgeWizardActivity
    + AppManagerActivity. Removes huge warnings from code. Anonymous AsyncTask
    implementations (such as Handler) implicitly hold a reference to the outer
    Activity, this could cause a memory leak where the AsyncTask is still running
    after the Activity has terminated
---
 .../org/torproject/android/OrbotMainActivity.java  | 93 ++++++++++------------
 .../torproject/android/ui/AppManagerActivity.java  | 63 ++++++++++-----
 .../ui/onboarding/BridgeWizardActivity.java        | 43 ++++++----
 3 files changed, 112 insertions(+), 87 deletions(-)

diff --git a/app/src/main/java/org/torproject/android/OrbotMainActivity.java b/app/src/main/java/org/torproject/android/OrbotMainActivity.java
index 8c898fce..25e0748e 100644
--- a/app/src/main/java/org/torproject/android/OrbotMainActivity.java
+++ b/app/src/main/java/org/torproject/android/OrbotMainActivity.java
@@ -76,6 +76,7 @@ import org.torproject.android.ui.onboarding.OnboardingActivity;
 
 import java.io.File;
 import java.io.UnsupportedEncodingException;
+import java.lang.ref.WeakReference;
 import java.net.URLDecoder;
 import java.net.URLEncoder;
 import java.text.NumberFormat;
@@ -130,47 +131,8 @@ public class OrbotMainActivity extends AppCompatActivity implements OrbotConstan
     private SharedPreferences mPrefs = null;
     private boolean autoStartFromIntent = false;
     // this is what takes messages or values from the callback threads or other non-mainUI threads
-//and passes them back into the main UI thread for display to the user
-    private Handler mStatusUpdateHandler = new Handler() {
-
-        @Override
-        public void handleMessage(final Message msg) {
-
-
-            Bundle data = msg.getData();
-
-            switch (msg.what) {
-                case MESSAGE_TRAFFIC_COUNT:
-
-                    DataCount datacount = new DataCount(data.getLong("upload"), data.getLong("download"));
-
-                    long totalRead = data.getLong("readTotal");
-                    long totalWrite = data.getLong("writeTotal");
-
-                    downloadText.setText(String.format("%s / %s", formatCount(datacount.Download), formatTotal(totalRead)));
-                    uploadText.setText(String.format("%s / %s", formatCount(datacount.Upload), formatTotal(totalWrite)));
-
-                    break;
-                case MESSAGE_PORTS:
-
-                    int socksPort = data.getInt("socks");
-                    int httpPort = data.getInt("http");
-
-                    lblPorts.setText(String.format(Locale.getDefault(), "SOCKS: %d | HTTP: %d", socksPort, httpPort));
-
-                    break;
-                default:
-                    String newTorStatus = msg.getData().getString("status");
-                    String log = (String) msg.obj;
-
-                    if (torStatus == null && newTorStatus != null) //first time status
-                        findViewById(R.id.frameMain).setVisibility(View.VISIBLE);
-                    updateStatus(log, newTorStatus);
-                    super.handleMessage(msg);
-                    break;
-            }
-        }
-    };
+    // and passes them back into the main UI thread for display to the user
+    private Handler mStatusUpdateHandler = new MainActivityStatusUpdateHandler(this);
     /**
      * The state and log info from {@link OrbotService} are sent to the UI here in
      * the form of a local broadcast. Regular broadcasts can be sent by any app,
@@ -1233,15 +1195,48 @@ public class OrbotMainActivity extends AppCompatActivity implements OrbotConstan
 
     }
 
-    public static class DataCount {
-        // data uploaded
-        final long Upload;
-        // data downloaded
-        final long Download;
+    private static class MainActivityStatusUpdateHandler extends Handler {
+        private WeakReference<OrbotMainActivity> ref;
+
+        MainActivityStatusUpdateHandler(OrbotMainActivity oma) {
+            ref = new WeakReference<>(oma);
+        }
+
+        private boolean shouldStop() {
+            return ref.get() == null || ref.get().isFinishing();
+        }
+
+        @Override
+        public void handleMessage(final Message msg) {
+            if (shouldStop()) return;
+            OrbotMainActivity oma = ref.get();
+            Bundle data = msg.getData();
+
+            switch (msg.what) {
+                case MESSAGE_TRAFFIC_COUNT:
+                    long upload = data.getLong("upload");
+                    long download = data.getLong("download");
+                    long totalRead = data.getLong("readTotal");
+                    long totalWrite = data.getLong("writeTotal");
+                    oma.downloadText.setText(String.format("%s / %s", oma.formatCount(download), oma.formatTotal(totalRead)));
+                    oma.uploadText.setText(String.format("%s / %s", oma.formatCount(upload), oma.formatTotal(totalWrite)));
+                    break;
+
+                case MESSAGE_PORTS:
+                    int socksPort = data.getInt("socks");
+                    int httpPort = data.getInt("http");
+                    oma.lblPorts.setText(String.format(Locale.getDefault(), "SOCKS: %d | HTTP: %d", socksPort, httpPort));
+                    break;
+
+                default:
+                    String newTorStatus = msg.getData().getString("status");
+                    String log = (String) msg.obj;
 
-        DataCount(long Upload, long Download) {
-            this.Upload = Upload;
-            this.Download = Download;
+                    if (oma.torStatus == null && newTorStatus != null) //first time status
+                        oma.findViewById(R.id.frameMain).setVisibility(View.VISIBLE);
+                    oma.updateStatus(log, newTorStatus);
+                    super.handleMessage(msg);
+            }
         }
     }
 }
diff --git a/app/src/main/java/org/torproject/android/ui/AppManagerActivity.java b/app/src/main/java/org/torproject/android/ui/AppManagerActivity.java
index aae3d72a..5d2493a4 100644
--- a/app/src/main/java/org/torproject/android/ui/AppManagerActivity.java
+++ b/app/src/main/java/org/torproject/android/ui/AppManagerActivity.java
@@ -35,6 +35,7 @@ import org.torproject.android.service.OrbotConstants;
 import org.torproject.android.service.util.Prefs;
 import org.torproject.android.service.vpn.TorifiedApp;
 
+import java.lang.ref.WeakReference;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -105,29 +106,13 @@ public class AppManagerActivity extends AppCompatActivity implements OnClickList
     }
 
     private void reloadApps() {
-        new AsyncTask<Void, Void, Void>() {
-            protected void onPreExecute() {
-                // Pre Code
-                progressBar.setVisibility(View.VISIBLE);
-            }
-
-            protected Void doInBackground(Void... unused) {
-                loadApps(mPrefs);
-                return null;
-            }
-
-            protected void onPostExecute(Void unused) {
-                listApps.setAdapter(adapterApps);
-                progressBar.setVisibility(View.GONE);
-            }
-        }.execute();
-
-
+        new ReloadAppsAsyncTask(this).execute();
     }
 
-    private void loadApps(SharedPreferences prefs) {
+    private void loadApps() {
+
         if (mApps == null)
-            mApps = getApps(prefs);
+            mApps = getApps(mPrefs);
 
         Collections.sort(mApps, (o1, o2) -> {
             /* Some apps start with lowercase letters and without the sorting being case
@@ -279,7 +264,6 @@ public class AppManagerActivity extends AppCompatActivity implements OnClickList
         return apps;
     }
 
-
     public void saveAppSettings() {
 
         StringBuilder tordApps = new StringBuilder();
@@ -300,7 +284,6 @@ public class AppManagerActivity extends AppCompatActivity implements OnClickList
         setResult(RESULT_OK, response);
     }
 
-
     public void onClick(View v) {
 
         CheckBox cbox = null;
@@ -321,6 +304,42 @@ public class AppManagerActivity extends AppCompatActivity implements OnClickList
         }
     }
 
+    private static class ReloadAppsAsyncTask extends AsyncTask<Void, Void, Void> {
+
+        private WeakReference<AppManagerActivity> activity;
+
+        ReloadAppsAsyncTask(AppManagerActivity activity) {
+            this.activity = new WeakReference<>(activity);
+        }
+
+        @Override
+        protected void onPreExecute() {
+            if (shouldStop()) return;
+            activity.get().progressBar.setVisibility(View.VISIBLE);
+        }
+
+        @Override
+        protected Void doInBackground(Void... voids) {
+            if (shouldStop()) return null;
+            activity.get().loadApps();
+            return null;
+        }
+
+        @Override
+        protected void onPostExecute(Void unused) {
+            if (shouldStop()) return;
+            AppManagerActivity ama = activity.get();
+            ama.listApps.setAdapter(ama.adapterApps);
+            ama.progressBar.setVisibility(View.GONE);
+        }
+
+        private boolean shouldStop() {
+            AppManagerActivity ama = activity.get();
+            return ama == null || ama.isFinishing();
+        }
+
+    }
+
     private static class ListEntry {
         private CheckBox box;
         private TextView text;
diff --git a/app/src/main/java/org/torproject/android/ui/onboarding/BridgeWizardActivity.java b/app/src/main/java/org/torproject/android/ui/onboarding/BridgeWizardActivity.java
index abc89ff2..8a359b1b 100644
--- a/app/src/main/java/org/torproject/android/ui/onboarding/BridgeWizardActivity.java
+++ b/app/src/main/java/org/torproject/android/ui/onboarding/BridgeWizardActivity.java
@@ -23,6 +23,7 @@ import org.torproject.android.service.util.Prefs;
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStreamReader;
+import java.lang.ref.WeakReference;
 import java.net.InetSocketAddress;
 import java.net.Socket;
 import java.net.SocketAddress;
@@ -34,7 +35,7 @@ public class BridgeWizardActivity extends AppCompatActivity {
     private static final int CUSTOM_BRIDGES_REQUEST_CODE = 1312;
     private static final String BUNDLE_KEY_TV_STATUS_VISIBILITY = "visibility";
     private static final String BUNDLE_KEY_TV_STATUS_TEXT = "text";
-    private static TextView mTvStatus;
+    private TextView mTvStatus;
     private static HostTester runningHostTest;
     private RadioButton mBtDirect;
     private RadioButton mBtObfs4;
@@ -182,7 +183,7 @@ public class BridgeWizardActivity extends AppCompatActivity {
 
     private void testBridgeConnection() {
         cancelHostTestIfRunning();
-        HostTester hostTester = new HostTester();
+        HostTester hostTester = new HostTester(this);
         if (TextUtils.isEmpty(Prefs.getBridgesList()) || (!Prefs.bridgesEnabled())) {
             hostTester.execute("check.torproject.org", "443");
         } else if (Prefs.getBridgesList().equals("meek")) {
@@ -253,22 +254,32 @@ public class BridgeWizardActivity extends AppCompatActivity {
         }
     }
 
-    private class HostTester extends AsyncTask<String, Void, Boolean> {
+    private static class HostTester extends AsyncTask<String, Void, Boolean> {
+
+        private WeakReference<BridgeWizardActivity> ref;
+
+        HostTester(BridgeWizardActivity activity) {
+            ref = new WeakReference<>(activity);
+        }
+
+        private boolean shouldStop() {
+            return ref.get() == null || ref.get().isFinishing();
+        }
+
         @Override
         protected void onPreExecute() {
-
-            if (mTvStatus != null) {
-                // Pre Code
-                mTvStatus.setVisibility(View.VISIBLE);
-                mTvStatus.setText(mBtDirect.isChecked() ? R.string.testing_tor_direct : R.string.testing_bridges);
+            if (shouldStop()) return;
+            BridgeWizardActivity bwa = ref.get();
+            if (bwa.mTvStatus != null) {
+                bwa.mTvStatus.setVisibility(View.VISIBLE);
+                bwa.mTvStatus.setText(bwa.mBtDirect.isChecked() ? R.string.testing_tor_direct : R.string.testing_bridges);
             }
         }
 
         @Override
         protected Boolean doInBackground(String... host) {
-            // Background Code
             for (int i = 0; i < host.length; i++) {
-                if (isCancelled()) return null;
+                if (shouldStop() || isCancelled()) return null;
                 String testHost = host[i];
                 i++; //move to the port
                 int testPort = Integer.parseInt(host[i]);
@@ -276,20 +287,20 @@ public class BridgeWizardActivity extends AppCompatActivity {
                     return true;
                 }
             }
-
             return false;
         }
 
         @Override
         protected void onPostExecute(Boolean result) {
-            // Post Code
-            if (mTvStatus != null) {
+            if (shouldStop()) return;
+            BridgeWizardActivity bwa = ref.get();
+            if (bwa.mTvStatus != null) {
                 runningHostTest = null;
                 if (result) {
-                    int stringRes = mBtDirect.isChecked() ? R.string.testing_tor_direct_success : R.string.testing_bridges_success;
-                    mTvStatus.setText(stringRes);
+                    int stringRes = bwa.mBtDirect.isChecked() ? R.string.testing_tor_direct_success : R.string.testing_bridges_success;
+                    bwa.mTvStatus.setText(stringRes);
                 } else {
-                    mTvStatus.setText(R.string.testing_bridges_fail);
+                    bwa.mTvStatus.setText(R.string.testing_bridges_fail);
                 }
             }
         }





More information about the tor-commits mailing list