Bug 1370668 - Do not assume that uid and deviceID will be present in the sync telemetry bundle r=nalexander draft
authorGrigory Kruglov <gkruglov@mozilla.com>
Tue, 06 Jun 2017 18:00:41 -0400
changeset 589808 f0c60d9bafc1e78e9119f0c77f6b145df72169d0
parent 589802 b68b44951361727015c2a10895e42f6a34806b27
child 632025 484969afe81028193e50cc5b6c3b3d3ae59b5a24
push id62528
push userbmo:gkruglov@mozilla.com
push dateTue, 06 Jun 2017 22:03:08 +0000
reviewersnalexander
bugs1370668
milestone55.0a1
Bug 1370668 - Do not assume that uid and deviceID will be present in the sync telemetry bundle r=nalexander It's possible that a sync fails very early, for example while talking to the token server. In that case, we wouldn't have uid/deviceID available in the sync telemetry bundle. MozReview-Commit-ID: 7OB1Er37qo8
mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryBackgroundReceiver.java
mobile/android/base/java/org/mozilla/gecko/telemetry/pingbuilders/TelemetrySyncPingBuilder.java
--- a/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryBackgroundReceiver.java
+++ b/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryBackgroundReceiver.java
@@ -6,16 +6,17 @@ package org.mozilla.gecko.telemetry;
 
 import android.content.BroadcastReceiver;
 import android.content.Context;
 import android.content.Intent;
 import android.content.IntentFilter;
 import android.content.SharedPreferences;
 import android.os.Bundle;
 import android.os.Parcelable;
+import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
 import android.support.v4.content.LocalBroadcastManager;
 import android.util.Log;
 
 import org.mozilla.gecko.AppConstants;
 import org.mozilla.gecko.sync.telemetry.TelemetryContract;
 import org.mozilla.gecko.telemetry.pingbuilders.TelemetrySyncEventPingBuilder;
 import org.mozilla.gecko.telemetry.pingbuilders.TelemetrySyncPingBuilder;
@@ -172,17 +173,24 @@ public class TelemetryBackgroundReceiver
 
                 // Determine if we should try uploading at this point, and attempt to do so.
                 final SharedPreferences sharedPreferences = context.getSharedPreferences(
                         PREF_FILE_BACKGROUND_TELEMETRY, Context.MODE_PRIVATE);
                 final TelemetryPingStore syncPingStore = new TelemetryJSONFilePingStore(
                         context.getFileStreamPath(SYNC_BUNDLE_STORE_DIR), DEFAULT_PROFILE);
 
                 long lastAttemptedSyncPingUpload = sharedPreferences.getLong(PREF_LAST_ATTEMPTED_UPLOADED, 0L);
-                boolean idsChanged = setOrUpdateIDsIfChanged(sharedPreferences, uid, deviceID);
+
+                // Changed IDs (uid or deviceID) is normally a reason to upload. However, if we
+                // didn't receive uid or deviceID, we skip this check. This might happen if Sync
+                // fails very early on, for example while talking to the token server.
+                boolean idsChanged = false;
+                if (uid != null && deviceID != null) {
+                    idsChanged = setOrUpdateIDsIfChanged(sharedPreferences, uid, deviceID);
+                }
 
                 // Is there a good reason to upload at this time?
                 final String reasonToUpload = reasonToUpload(
                         idsChanged,
                         syncTelemetryStore.getCount(),
                         syncEventTelemetryStore.getCount(),
                         lastAttemptedSyncPingUpload
                 );
@@ -320,17 +328,17 @@ public class TelemetryBackgroundReceiver
             return TelemetrySyncPingBundleBuilder.UPLOAD_REASON_SCHEDULE;
         }
 
         // No reason to upload.
         return null;
     }
 
     // This has storage side-effects.
-    private boolean setOrUpdateIDsIfChanged(SharedPreferences prefs, String uid, String deviceID) {
+    private boolean setOrUpdateIDsIfChanged(@NonNull SharedPreferences prefs, @NonNull String uid, @NonNull String deviceID) {
         final String currentIDsCombined = uid.concat(deviceID);
         final String previousIDsHash = prefs.getString(PREF_IDS, "");
 
         // Persist IDs for the first time, declare them as "not changed".
         if (previousIDsHash.equals("")) {
             final SharedPreferences.Editor prefsEditor = prefs.edit();
             prefsEditor.putString(PREF_IDS, currentIDsCombined);
             prefsEditor.apply();
--- a/mobile/android/base/java/org/mozilla/gecko/telemetry/pingbuilders/TelemetrySyncPingBuilder.java
+++ b/mobile/android/base/java/org/mozilla/gecko/telemetry/pingbuilders/TelemetrySyncPingBuilder.java
@@ -106,17 +106,17 @@ public class TelemetrySyncPingBuilder ex
         if (!didRestart) {
             return this;
         }
 
         payload.put("restarted", true);
         return this;
     }
 
-    public TelemetrySyncPingBuilder setDevices(ArrayList<Parcelable> devices) {
+    public TelemetrySyncPingBuilder setDevices(@NonNull ArrayList<Parcelable> devices) {
         final JSONArray devicesJSON = new JSONArray();
 
         for (Parcelable device : devices) {
             final Bundle deviceBundle = (Bundle) device;
             final ExtendedJSONObject deviceJSON = new ExtendedJSONObject();
 
             deviceJSON.put("os", deviceBundle.getString(TelemetryContract.KEY_DEVICE_OS));
             deviceJSON.put("version", deviceBundle.getString(TelemetryContract.KEY_DEVICE_VERSION));