Bug 1370668 - Do not assume that uid and deviceID will be present in the sync telemetry bundle r=nalexander
authorGrigory Kruglov <gkruglov@mozilla.com>
Tue, 06 Jun 2017 18:00:41 -0400
changeset 410889 9581f9e1b988c7320589effa3aefda47b51c8aca
parent 410888 215afe84f16e8a1d191a85ce3d9e5ceea5038bbe
child 410890 85b383c5a7a8b187d71e7706fe4c38efb5f243bc
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnalexander
bugs1370668
milestone55.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 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
@@ -105,17 +105,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));