Bug 1273689 - review - remove inner class & move methods to containing class; use BrowserAppDelegateWithReference. r=me
authorMichael Comella <michael.l.comella@gmail.com>
Wed, 01 Jun 2016 17:46:19 -0700
changeset 338909 7c0dce574b3f2d5e8f6e4e60caab5532f8e13fd6
parent 338908 8644fcfe29da58befd2c3ea28e230570570af925
child 339058 34a8be4346a9231e472fc36b1d7c0531e0fbf7c5
child 339063 034b45ace455de8b62f389fc0b940583d9c62ea6
push id6249
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 13:59:36 +0000
treeherdermozilla-beta@bad9d4f5bf7e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersme
bugs1273689
milestone49.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1273689 - review - remove inner class & move methods to containing class; use BrowserAppDelegateWithReference. r=me It's all a little cleaner.
mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryCorePingDelegate.java
--- a/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryCorePingDelegate.java
+++ b/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryCorePingDelegate.java
@@ -8,33 +8,33 @@ package org.mozilla.gecko.telemetry;
 
 import android.content.SharedPreferences;
 import android.support.annotation.Nullable;
 import android.support.annotation.WorkerThread;
 import android.util.Log;
 import org.mozilla.gecko.BrowserApp;
 import org.mozilla.gecko.GeckoProfile;
 import org.mozilla.gecko.GeckoSharedPrefs;
-import org.mozilla.gecko.delegates.BrowserAppDelegate;
+import org.mozilla.gecko.delegates.BrowserAppDelegateWithReference;
 import org.mozilla.gecko.distribution.DistributionStoreCallback;
 import org.mozilla.gecko.search.SearchEngineManager;
 import org.mozilla.gecko.sync.ExtendedJSONObject;
 import org.mozilla.gecko.telemetry.measurements.SearchCountMeasurements;
 import org.mozilla.gecko.telemetry.measurements.SessionMeasurements;
 import org.mozilla.gecko.telemetry.pingbuilders.TelemetryCorePingBuilder;
 import org.mozilla.gecko.util.StringUtils;
 import org.mozilla.gecko.util.ThreadUtils;
 
 import java.io.IOException;
-import java.lang.ref.WeakReference;
 
 /**
  * An activity-lifecycle delegate for uploading the core ping.
  */
-public class TelemetryCorePingDelegate extends BrowserAppDelegate {
+public class TelemetryCorePingDelegate extends BrowserAppDelegateWithReference
+        implements SearchEngineManager.SearchEngineCallback {
     private static final String LOGTAG = StringUtils.safeSubstring(
             "Gecko" + TelemetryCorePingDelegate.class.getSimpleName(), 0, 23);
 
     private static final String PREF_IS_FIRST_RUN = "telemetry-isFirstRun";
 
     private TelemetryDispatcher telemetryDispatcher; // lazy
     private final SessionMeasurements sessionMeasurements = new SessionMeasurements();
 
@@ -70,17 +70,17 @@ public class TelemetryCorePingDelegate e
                     .putBoolean(PREF_IS_FIRST_RUN, false)
                     .apply();
             uploadPing(browserApp);
         }
     }
 
     private void uploadPing(final BrowserApp browserApp) {
         final SearchEngineManager searchEngineManager = browserApp.getSearchEngineManager();
-        searchEngineManager.getEngine(new UploadTelemetryCorePingCallback(browserApp));
+        searchEngineManager.getEngine(this);
     }
 
     @Override
     public void onResume(BrowserApp browserApp) {
         sessionMeasurements.recordSessionStart();
     }
 
     @Override
@@ -98,81 +98,73 @@ public class TelemetryCorePingDelegate e
         }
         return telemetryDispatcher;
     }
 
     private SharedPreferences getSharedPreferences(final BrowserApp activity) {
         return GeckoSharedPrefs.forProfileName(activity, activity.getProfile().getName());
     }
 
-    private class UploadTelemetryCorePingCallback implements SearchEngineManager.SearchEngineCallback {
-        private final WeakReference<BrowserApp> activityWeakReference;
-
-        private UploadTelemetryCorePingCallback(final BrowserApp activity) {
-            this.activityWeakReference = new WeakReference<>(activity);
+    // via SearchEngineCallback - may be called from any thread.
+    @Override
+    public void execute(@Nullable final org.mozilla.gecko.search.SearchEngine engine) {
+        // Don't waste resources queueing to the background thread if we don't have a reference.
+        if (getBrowserApp() == null) {
+            return;
         }
 
-        // May be called from any thread.
-        @Override
-        public void execute(@Nullable final org.mozilla.gecko.search.SearchEngine engine) {
-            // Don't waste resources queueing to the background thread if we don't have a reference.
-            if (this.activityWeakReference.get() == null) {
-                return;
-            }
+        // The containing method can be called from onStart: queue this work so that
+        // the first launch of the activity doesn't trigger profile init too early.
+        //
+        // Additionally, getAndIncrementSequenceNumber must be called from a worker thread.
+        ThreadUtils.postToBackgroundThread(new Runnable() {
+            @WorkerThread
+            @Override
+            public void run() {
+                final BrowserApp activity = getBrowserApp();
+                if (activity == null) {
+                    return;
+                }
 
-            // The containing method can be called from onStart: queue this work so that
-            // the first launch of the activity doesn't trigger profile init too early.
-            //
-            // Additionally, getAndIncrementSequenceNumber must be called from a worker thread.
-            ThreadUtils.postToBackgroundThread(new Runnable() {
-                @WorkerThread
-                @Override
-                public void run() {
-                    final BrowserApp activity = activityWeakReference.get();
-                    if (activity == null) {
-                        return;
-                    }
+                final GeckoProfile profile = activity.getProfile();
+                if (!TelemetryUploadService.isUploadEnabledByProfileConfig(activity, profile)) {
+                    Log.d(LOGTAG, "Core ping upload disabled by profile config. Returning.");
+                    return;
+                }
 
-                    final GeckoProfile profile = activity.getProfile();
-                    if (!TelemetryUploadService.isUploadEnabledByProfileConfig(activity, profile)) {
-                        Log.d(LOGTAG, "Core ping upload disabled by profile config. Returning.");
-                        return;
-                    }
-
-                    final String clientID;
-                    try {
-                        clientID = profile.getClientId();
-                    } catch (final IOException e) {
-                        Log.w(LOGTAG, "Unable to get client ID to generate core ping: " + e);
-                        return;
-                    }
+                final String clientID;
+                try {
+                    clientID = profile.getClientId();
+                } catch (final IOException e) {
+                    Log.w(LOGTAG, "Unable to get client ID to generate core ping: " + e);
+                    return;
+                }
 
-                    // Each profile can have different telemetry data so we intentionally grab the shared prefs for the profile.
-                    final SharedPreferences sharedPrefs = getSharedPreferences(activity);
-                    final SessionMeasurements.SessionMeasurementsContainer sessionMeasurementsContainer =
-                            sessionMeasurements.getAndResetSessionMeasurements(activity);
-                    final TelemetryCorePingBuilder pingBuilder = new TelemetryCorePingBuilder(activity)
-                            .setClientID(clientID)
-                            .setDefaultSearchEngine(TelemetryCorePingBuilder.getEngineIdentifier(engine))
-                            .setProfileCreationDate(TelemetryCorePingBuilder.getProfileCreationDate(activity, profile))
-                            .setSequenceNumber(TelemetryCorePingBuilder.getAndIncrementSequenceNumber(sharedPrefs))
-                            .setSessionCount(sessionMeasurementsContainer.sessionCount)
-                            .setSessionDuration(sessionMeasurementsContainer.elapsedSeconds);
-                    maybeSetOptionalMeasurements(sharedPrefs, pingBuilder);
+                // Each profile can have different telemetry data so we intentionally grab the shared prefs for the profile.
+                final SharedPreferences sharedPrefs = getSharedPreferences(activity);
+                final SessionMeasurements.SessionMeasurementsContainer sessionMeasurementsContainer =
+                        sessionMeasurements.getAndResetSessionMeasurements(activity);
+                final TelemetryCorePingBuilder pingBuilder = new TelemetryCorePingBuilder(activity)
+                        .setClientID(clientID)
+                        .setDefaultSearchEngine(TelemetryCorePingBuilder.getEngineIdentifier(engine))
+                        .setProfileCreationDate(TelemetryCorePingBuilder.getProfileCreationDate(activity, profile))
+                        .setSequenceNumber(TelemetryCorePingBuilder.getAndIncrementSequenceNumber(sharedPrefs))
+                        .setSessionCount(sessionMeasurementsContainer.sessionCount)
+                        .setSessionDuration(sessionMeasurementsContainer.elapsedSeconds);
+                maybeSetOptionalMeasurements(sharedPrefs, pingBuilder);
 
-                    getTelemetryDispatcher(activity).queuePingForUpload(activity, pingBuilder);
-                }
-            });
+                getTelemetryDispatcher(activity).queuePingForUpload(activity, pingBuilder);
+            }
+        });
+    }
+
+    private void maybeSetOptionalMeasurements(final SharedPreferences sharedPrefs, final TelemetryCorePingBuilder pingBuilder) {
+        final String distributionId = sharedPrefs.getString(DistributionStoreCallback.PREF_DISTRIBUTION_ID, null);
+        if (distributionId != null) {
+            pingBuilder.setOptDistributionID(distributionId);
         }
 
-        private void maybeSetOptionalMeasurements(final SharedPreferences sharedPrefs, final TelemetryCorePingBuilder pingBuilder) {
-            final String distributionId = sharedPrefs.getString(DistributionStoreCallback.PREF_DISTRIBUTION_ID, null);
-            if (distributionId != null) {
-                pingBuilder.setOptDistributionID(distributionId);
-            }
-
-            final ExtendedJSONObject searchCounts = SearchCountMeasurements.getAndZeroSearch(sharedPrefs);
-            if (searchCounts.size() > 0) {
-                pingBuilder.setOptSearchCounts(searchCounts);
-            }
+        final ExtendedJSONObject searchCounts = SearchCountMeasurements.getAndZeroSearch(sharedPrefs);
+        if (searchCounts.size() > 0) {
+            pingBuilder.setOptSearchCounts(searchCounts);
         }
     }
 }