Bug 1025937 - Silently drop null intents in background IntentServices. r=mcomella
authorRichard Newman <rnewman@mozilla.com>
Mon, 16 Jun 2014 16:18:59 -0700
changeset 189006 e1c288cf638aa67cc00c76a46df00a8518f800c7
parent 189005 62cce2cf895f36ebe35aa8795e55d19a011f2b3f
child 189007 63dacdff674cf989c0c86c1245abe3fe359ace91
push id26973
push usercbook@mozilla.com
push dateTue, 17 Jun 2014 12:17:11 +0000
treeherdermozilla-central@22ddd7bc3689 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmcomella
bugs1025937
milestone33.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 1025937 - Silently drop null intents in background IntentServices. r=mcomella
mobile/android/base/background/announcements/AnnouncementsBroadcastService.java
mobile/android/base/background/announcements/AnnouncementsService.java
mobile/android/base/background/healthreport/HealthReportBroadcastService.java
mobile/android/base/background/healthreport/prune/HealthReportPruneService.java
mobile/android/base/background/healthreport/upload/HealthReportUploadService.java
mobile/android/base/fxa/receivers/FxAccountDeletedService.java
mobile/android/base/sync/receivers/SyncAccountDeletedService.java
mobile/android/tests/background/junit3/src/healthreport/prune/TestHealthReportPruneService.java
--- a/mobile/android/base/background/announcements/AnnouncementsBroadcastService.java
+++ b/mobile/android/base/background/announcements/AnnouncementsBroadcastService.java
@@ -114,16 +114,22 @@ public class AnnouncementsBroadcastServi
   public static void setPollInterval(final Context context, long interval) {
     final SharedPreferences preferences = getSharedPreferences(context);
     preferences.edit().putLong(AnnouncementsConstants.PREF_ANNOUNCE_FETCH_INTERVAL_MSEC, interval).commit();
   }
 
   @Override
   protected void onHandleIntent(Intent intent) {
     Logger.setThreadLogTag(AnnouncementsConstants.GLOBAL_LOG_TAG);
+
+    // Intent can be null. Bug 1025937.
+    if (intent == null) {
+        Logger.debug(LOG_TAG, "Short-circuiting on null intent.");
+    }
+
     final String action = intent.getAction();
     Logger.debug(LOG_TAG, "Broadcast onReceive. Intent is " + action);
 
     if (AnnouncementsConstants.ACTION_ANNOUNCEMENTS_PREF.equals(action)) {
       handlePrefIntent(intent);
       return;
     }
 
@@ -138,18 +144,20 @@ public class AnnouncementsBroadcastServi
     // Failure case.
     Logger.warn(LOG_TAG, "Unknown intent " + action);
   }
 
   /**
    * Handle the intent sent by the browser when it wishes to notify us
    * of the value of the user preference. Look at the value and toggle the
    * alarm service accordingly.
+   *
+   * @param intent must be non-null.
    */
-  protected void handlePrefIntent(Intent intent) {
+  private void handlePrefIntent(Intent intent) {
     if (!intent.hasExtra("enabled")) {
       Logger.warn(LOG_TAG, "Got ANNOUNCEMENTS_PREF intent without enabled. Ignoring.");
       return;
     }
 
     final boolean enabled = intent.getBooleanExtra("enabled", true);
     Logger.debug(LOG_TAG, intent.getStringExtra("branch") + "/" +
                           intent.getStringExtra("pref")   + " = " +
--- a/mobile/android/base/background/announcements/AnnouncementsService.java
+++ b/mobile/android/base/background/announcements/AnnouncementsService.java
@@ -112,16 +112,22 @@ public class AnnouncementsService extend
    * If it's time to do a fetch -- we've waited long enough,
    * we're allowed to use background data, etc. -- then issue
    * a fetch. The subsequent background check is handled implicitly
    * by the AlarmManager.
    */
   @Override
   public void onHandleIntent(Intent intent) {
     Logger.setThreadLogTag(AnnouncementsConstants.GLOBAL_LOG_TAG);
+
+    // Intent can be null. Bug 1025937.
+    if (intent == null) {
+        Logger.debug(LOG_TAG, "Short-circuiting on null intent.");
+    }
+
     Logger.debug(LOG_TAG, "Running AnnouncementsService.");
 
     if (AnnouncementsConstants.DISABLED) {
       Logger.debug(LOG_TAG, "Announcements disabled. Returning from AnnouncementsService.");
       return;
     }
 
     if (!shouldFetchAnnouncements()) {
--- a/mobile/android/base/background/healthreport/HealthReportBroadcastService.java
+++ b/mobile/android/base/background/healthreport/HealthReportBroadcastService.java
@@ -95,29 +95,36 @@ public class HealthReportBroadcastServic
     final long pollInterval = getSubmissionPollInterval();
     scheduleAlarm(pollInterval, pending);
   }
 
   @Override
   protected void onHandleIntent(Intent intent) {
     Logger.setThreadLogTag(HealthReportConstants.GLOBAL_LOG_TAG);
 
+    // Intent can be null. Bug 1025937.
+    if (intent == null) {
+        Logger.debug(LOG_TAG, "Short-circuiting on null intent.");
+    }
+
     // The same intent can be handled by multiple methods so do not short-circuit evaluate.
     boolean handled = attemptHandleIntentForUpload(intent);
     handled = attemptHandleIntentForPrune(intent) ? true : handled;
 
     if (!handled) {
       Logger.warn(LOG_TAG, "Unhandled intent with action " + intent.getAction() + ".");
     }
   }
 
   /**
    * Attempts to handle the given intent for FHR document upload. If it cannot, false is returned.
+   *
+   * @param intent must be non-null.
    */
-  protected boolean attemptHandleIntentForUpload(final Intent intent) {
+  private boolean attemptHandleIntentForUpload(final Intent intent) {
     if (HealthReportConstants.UPLOAD_FEATURE_DISABLED) {
       Logger.debug(LOG_TAG, "Health report upload feature is compile-time disabled; not handling intent.");
       return false;
     }
 
     final String action = intent.getAction();
     Logger.debug(LOG_TAG, "Health report upload feature is compile-time enabled; attempting to " +
         "handle intent with action " + action + ".");
@@ -137,18 +144,20 @@ public class HealthReportBroadcastServic
 
     return false;
   }
 
   /**
    * Handle the intent sent by the browser when it wishes to notify us
    * of the value of the user preference. Look at the value and toggle the
    * alarm service accordingly.
+   *
+   * @param intent must be non-null.
    */
-  protected void handleUploadPrefIntent(Intent intent) {
+  private void handleUploadPrefIntent(Intent intent) {
     if (!intent.hasExtra("enabled")) {
       Logger.warn(LOG_TAG, "Got " + HealthReportConstants.ACTION_HEALTHREPORT_UPLOAD_PREF + " intent without enabled. Ignoring.");
       return;
     }
 
     final boolean enabled = intent.getBooleanExtra("enabled", true);
     Logger.debug(LOG_TAG, intent.getStringExtra("branch") + "/" +
                           intent.getStringExtra("pref")   + " = " +
@@ -189,18 +198,20 @@ public class HealthReportBroadcastServic
     // The user can toggle us off or on, or we can have obsolete documents to
     // remove.
     final boolean serviceEnabled = hasObsoleteIds || enabled;
     toggleSubmissionAlarm(this, profileName, profilePath, enabled, serviceEnabled);
   }
 
   /**
    * Attempts to handle the given intent for FHR data pruning. If it cannot, false is returned.
+   *
+   * @param intent must be non-null.
    */
-  protected boolean attemptHandleIntentForPrune(final Intent intent) {
+  private boolean attemptHandleIntentForPrune(final Intent intent) {
     final String action = intent.getAction();
     Logger.debug(LOG_TAG, "Prune: Attempting to handle intent with action, " + action + ".");
 
     if (HealthReportConstants.ACTION_HEALTHREPORT_PRUNE.equals(action)) {
       handlePruneIntent(intent);
       return true;
     }
 
@@ -210,17 +221,20 @@ public class HealthReportBroadcastServic
           GlobalConstants.GECKO_PREFERENCES_CLASS,
           GlobalConstants.GECKO_BROADCAST_HEALTHREPORT_PRUNE_METHOD);
       return true;
     }
 
     return false;
   }
 
-  protected void handlePruneIntent(final Intent intent) {
+  /**
+   * @param intent must be non-null.
+   */
+  private void handlePruneIntent(final Intent intent) {
     final String profileName = intent.getStringExtra("profileName");
     final String profilePath = intent.getStringExtra("profilePath");
 
     if (profileName == null || profilePath == null) {
       Logger.warn(LOG_TAG, "Got " + HealthReportConstants.ACTION_HEALTHREPORT_PRUNE + " intent " +
           "without profilePath or profileName. Ignoring.");
       return;
     }
--- a/mobile/android/base/background/healthreport/prune/HealthReportPruneService.java
+++ b/mobile/android/base/background/healthreport/prune/HealthReportPruneService.java
@@ -34,16 +34,22 @@ public class HealthReportPruneService ex
 
   protected SharedPreferences getSharedPreferences() {
     return this.getSharedPreferences(HealthReportConstants.PREFS_BRANCH, GlobalConstants.SHARED_PREFERENCES_MODE);
   }
 
   @Override
   public void onHandleIntent(Intent intent) {
     Logger.setThreadLogTag(HealthReportConstants.GLOBAL_LOG_TAG);
+
+    // Intent can be null. Bug 1025937.
+    if (intent == null) {
+        Logger.debug(LOG_TAG, "Short-circuiting on null intent.");
+    }
+
     Logger.debug(LOG_TAG, "Handling prune intent.");
 
     if (!isIntentValid(intent)) {
       Logger.warn(LOG_TAG, "Intent not valid - returning.");
       return;
     }
 
     final String profileName = intent.getStringExtra("profileName");
@@ -54,17 +60,21 @@ public class HealthReportPruneService ex
   }
 
   // Generator function wraps constructor for testing purposes.
   protected PrunePolicy getPrunePolicy(final String profilePath) {
     final PrunePolicyStorage storage = new PrunePolicyDatabaseStorage(this, profilePath);
     return new PrunePolicy(storage, getSharedPreferences());
   }
 
-  protected boolean isIntentValid(final Intent intent) {
+  /**
+   * @param intent must be non-null.
+   * @return true if the supplied intent contains both profileName and profilePath.
+   */
+  private static boolean isIntentValid(final Intent intent) {
     boolean isValid = true;
 
     final String profileName = intent.getStringExtra("profileName");
     if (profileName == null) {
       Logger.warn(LOG_TAG, "Got intent without profileName.");
       isValid = false;
     }
 
--- a/mobile/android/base/background/healthreport/upload/HealthReportUploadService.java
+++ b/mobile/android/base/background/healthreport/upload/HealthReportUploadService.java
@@ -38,16 +38,21 @@ public class HealthReportUploadService e
   protected SharedPreferences getSharedPreferences() {
     return this.getSharedPreferences(HealthReportConstants.PREFS_BRANCH, GlobalConstants.SHARED_PREFERENCES_MODE);
   }
 
   @Override
   public void onHandleIntent(Intent intent) {
     Logger.setThreadLogTag(HealthReportConstants.GLOBAL_LOG_TAG);
 
+    // Intent can be null. Bug 1025937.
+    if (intent == null) {
+        Logger.debug(LOG_TAG, "Short-circuiting on null intent.");
+    }
+
     if (HealthReportConstants.UPLOAD_FEATURE_DISABLED) {
       Logger.debug(LOG_TAG, "Health report upload feature is compile-time disabled; not handling upload intent.");
       return;
     }
 
     Logger.debug(LOG_TAG, "Health report upload feature is compile-time enabled; handling upload intent.");
 
     String profileName = intent.getStringExtra("profileName");
--- a/mobile/android/base/fxa/receivers/FxAccountDeletedService.java
+++ b/mobile/android/base/fxa/receivers/FxAccountDeletedService.java
@@ -24,16 +24,21 @@ public class FxAccountDeletedService ext
   public static final String LOG_TAG = FxAccountDeletedService.class.getSimpleName();
 
   public FxAccountDeletedService() {
     super(LOG_TAG);
   }
 
   @Override
   protected void onHandleIntent(final Intent intent) {
+    // Intent can, in theory, be null. Bug 1025937.
+    if (intent == null) {
+        Logger.debug(LOG_TAG, "Short-circuiting on null intent.");
+    }
+
     final Context context = this;
 
     long intentVersion = intent.getLongExtra(
         FxAccountConstants.ACCOUNT_DELETED_INTENT_VERSION_KEY, 0);
     long expectedVersion = FxAccountConstants.ACCOUNT_DELETED_INTENT_VERSION;
     if (intentVersion != expectedVersion) {
       Logger.warn(LOG_TAG, "Intent malformed: version " + intentVersion + " given but " +
           "version " + expectedVersion + "expected. Not cleaning up after deleted Account.");
--- a/mobile/android/base/sync/receivers/SyncAccountDeletedService.java
+++ b/mobile/android/base/sync/receivers/SyncAccountDeletedService.java
@@ -27,16 +27,21 @@ public class SyncAccountDeletedService e
   public static final String LOG_TAG = "SyncAccountDeletedService";
 
   public SyncAccountDeletedService() {
     super(LOG_TAG);
   }
 
   @Override
   protected void onHandleIntent(Intent intent) {
+    // Intent can, in theory, be null. Bug 1025937.
+    if (intent == null) {
+      Logger.debug(LOG_TAG, "Short-circuiting on null intent.");
+    }
+
     final Context context = this;
 
     long intentVersion = intent.getLongExtra(Constants.JSON_KEY_VERSION, 0);
     long expectedVersion = SyncConstants.SYNC_ACCOUNT_DELETED_INTENT_VERSION;
     if (intentVersion != expectedVersion) {
       Logger.warn(LOG_TAG, "Intent malformed: version " + intentVersion + " given but version " + expectedVersion + "expected. " +
           "Not cleaning up after deleted Account.");
       return;
--- a/mobile/android/tests/background/junit3/src/healthreport/prune/TestHealthReportPruneService.java
+++ b/mobile/android/tests/background/junit3/src/healthreport/prune/TestHealthReportPruneService.java
@@ -31,21 +31,16 @@ public class TestHealthReportPruneServic
       } catch (InterruptedException e) {
         fail("Awaiting thread should not be interrupted.");
       } catch (BrokenBarrierException e) {
         // This will happen on timeout - do nothing.
       }
     }
 
     @Override
-    public boolean isIntentValid(final Intent intent) {
-      return super.isIntentValid(intent);
-    }
-
-    @Override
     public PrunePolicy getPrunePolicy(final String profilePath) {
       final PrunePolicyStorage storage = new PrunePolicyDatabaseStorage(new MockContext(), profilePath);
       prunePolicy = new MockPrunePolicy(storage, getSharedPreferences());
       return prunePolicy;
     }
 
     public boolean wasTickCalled() {
       if (prunePolicy == null) {