Bug 1502435 - 1 - Set valid clientId and save if previous was canary clientId; r=jchen
authorPetru Lingurar <petru.lingurar@softvision.ro>
Tue, 30 Oct 2018 16:28:02 +0000
changeset 443582 e0a9b2e68b8f8554bd1c410f3def294b2084eeea
parent 443581 3ae8da2e4b2bf8487ca5cf01c2236faebdb386ad
child 443583 e80f0318d599a2ba909aaba08fe016f1a47f505a
push id34963
push useraciure@mozilla.com
push dateWed, 31 Oct 2018 05:07:31 +0000
treeherdermozilla-central@9e08e4f36d5e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjchen
bugs1502435
milestone65.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 1502435 - 1 - Set valid clientId and save if previous was canary clientId; r=jchen Differential Revision: https://phabricator.services.mozilla.com/D10202
mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoProfile.java
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoProfile.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoProfile.java
@@ -1,33 +1,32 @@
 /* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*-
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko;
 
-import android.app.Activity;
 import android.content.Context;
-import android.content.SharedPreferences;
+import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
 import android.support.annotation.WorkerThread;
 import android.text.TextUtils;
 import android.util.Log;
 
 import org.json.JSONArray;
 import org.json.JSONException;
 import org.json.JSONObject;
 import org.mozilla.gecko.GeckoProfileDirectories.NoMozillaDirectoryException;
 import org.mozilla.gecko.GeckoProfileDirectories.NoSuchProfileException;
 import org.mozilla.gecko.annotation.RobocopTarget;
 import org.mozilla.gecko.util.FileUtils;
 import org.mozilla.gecko.util.GeckoBundle;
 import org.mozilla.gecko.util.INIParser;
 import org.mozilla.gecko.util.INISection;
-import org.mozilla.gecko.util.IntentUtils;
 
 import java.io.BufferedWriter;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.FileReader;
 import java.io.FileWriter;
 import java.io.IOException;
 import java.io.OutputStreamWriter;
@@ -41,16 +40,19 @@ import java.util.regex.Pattern;
 
 public final class GeckoProfile {
     private static final String LOGTAG = "GeckoProfile";
 
     // The path in the profile to the file containing the client ID.
     private static final String CLIENT_ID_FILE_PATH = "datareporting/state.json";
     // In the client ID file, the attribute title in the JSON object containing the client ID value.
     private static final String CLIENT_ID_JSON_ATTR = "clientID";
+    private static final String HAD_CANARY_CLIENT_ID_JSON_ATTR = "wasCanary";
+    // Must match the one from TelemetryUtils.jsm
+    private static final String CANARY_CLIENT_ID = "c0ffeec0-ffee-c0ff-eec0-ffeec0ffeec0";
 
     private static final String TIMES_PATH = "times.json";
     private static final String PROFILE_CREATION_DATE_JSON_ATTR = "created";
 
     // Only tests should need to do this.  We can remove this entirely once we
     // fix Bug 1069687.
     private static volatile boolean sAcceptDirectoryChanges = true;
 
@@ -444,86 +446,104 @@ public final class GeckoProfile {
      *
      * [1]: https://dxr.mozilla.org/mozilla-central/source/toolkit/modules/ClientID.jsm
      *
      * @throws IOException if the client ID could not be retrieved.
      */
     // Mimics ClientID.jsm - _doLoadClientID.
     @WorkerThread
     public String getClientId() throws IOException {
+        String clientId = "";
         try {
-            return getValidClientIdFromDisk(CLIENT_ID_FILE_PATH);
+            clientId = getClientIdFromDisk(CLIENT_ID_FILE_PATH);
         } catch (final IOException e) {
             // Avoid log spam: don't log the full Exception w/ the stack trace.
             Log.d(LOGTAG, "Could not get client ID - creating a new one: " + e.getLocalizedMessage());
         }
 
-        String clientIdToWrite = generateNewClientId();
+        if (isClientIdValid(clientId)) {
+            return clientId;
+        } else {
+            String newClientId = generateNewClientId();
+            // There is a possibility Gecko is running and the Gecko telemetry implementation decided it's time to generate
+            // the client ID, writing client ID underneath us. Since it's highly unlikely (e.g. we run in onStart before
+            // Gecko is started), we don't handle that possibility besides writing the ID and then reading from the file
+            // again (rather than just returning the value we generated before writing).
+            //
+            // In the event it does happen, any discrepancy will be resolved after a restart. In the mean time, both this
+            // implementation and the Gecko implementation could upload documents with inconsistent IDs.
+            //
+            // In any case, if we get an exception, intentionally throw - there's nothing more to do here.
+            persistNewClientId(clientId, newClientId);
+        }
 
-        // There is a possibility Gecko is running and the Gecko telemetry implementation decided it's time to generate
-        // the client ID, writing client ID underneath us. Since it's highly unlikely (e.g. we run in onStart before
-        // Gecko is started), we don't handle that possibility besides writing the ID and then reading from the file
-        // again (rather than just returning the value we generated before writing).
-        //
-        // In the event it does happen, any discrepancy will be resolved after a restart. In the mean time, both this
-        // implementation and the Gecko implementation could upload documents with inconsistent IDs.
-        //
-        // In any case, if we get an exception, intentionally throw - there's nothing more to do here.
-        persistClientId(clientIdToWrite);
-        return getValidClientIdFromDisk(CLIENT_ID_FILE_PATH);
+        return getClientIdFromDisk(CLIENT_ID_FILE_PATH);
+    }
+
+    @WorkerThread
+    public boolean getIfHadCanaryClientId() throws IOException {
+        final JSONObject obj = readJSONObjectFromFile(CLIENT_ID_FILE_PATH);
+        return obj.optBoolean(HAD_CANARY_CLIENT_ID_JSON_ATTR);
     }
 
     protected static String generateNewClientId() {
         return UUID.randomUUID().toString();
     }
 
     /**
      * @return a valid client ID
      * @throws IOException if a valid client ID could not be retrieved
      */
     @WorkerThread
-    private String getValidClientIdFromDisk(final String filePath) throws IOException {
+    private String getClientIdFromDisk(final String filePath) throws IOException {
         final JSONObject obj = readJSONObjectFromFile(filePath);
-        final String clientId = obj.optString(CLIENT_ID_JSON_ATTR);
-        if (isClientIdValid(clientId)) {
-            return clientId;
-        }
-        throw new IOException("Received client ID is invalid: " + clientId);
+        return obj.optString(CLIENT_ID_JSON_ATTR);
     }
 
     /**
      * Persists the given client ID to disk. This will overwrite any existing files.
      */
     @WorkerThread
-    private void persistClientId(final String clientId) throws IOException {
+    private void persistNewClientId(@Nullable final String oldClientId,
+                                    @NonNull final String newClientId) throws IOException {
         if (!ensureParentDirs(CLIENT_ID_FILE_PATH)) {
             throw new IOException("Could not create client ID parent directories");
         }
 
         final JSONObject obj = new JSONObject();
         try {
-            obj.put(CLIENT_ID_JSON_ATTR, clientId);
+            obj.put(CLIENT_ID_JSON_ATTR, newClientId);
+            obj.put(HAD_CANARY_CLIENT_ID_JSON_ATTR, isCanaryClientId(oldClientId));
         } catch (final JSONException e) {
             throw new IOException("Could not create client ID JSON object", e);
         }
 
         // ClientID.jsm overwrites the file to store the client ID so it's okay if we do it too.
-        Log.d(LOGTAG, "Attempting to write new client ID");
+        Log.d(LOGTAG, "Attempting to write new client ID properties");
         writeFile(CLIENT_ID_FILE_PATH, obj.toString()); // Logs errors within function: ideally we'd throw.
     }
 
     // From ClientID.jsm - isValidClientID.
-    public static boolean isClientIdValid(final String clientId) {
+    public static boolean isClientIdValid(@Nullable final String clientId) {
         // We could use UUID.fromString but, for consistency, we take the implementation from ClientID.jsm.
         if (TextUtils.isEmpty(clientId)) {
             return false;
         }
+
+        if (CANARY_CLIENT_ID.equals(clientId)) {
+            return false;
+        }
+
         return clientId.matches("(?i:[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})");
     }
 
+    private static boolean isCanaryClientId(@Nullable final String clientId) {
+        return CANARY_CLIENT_ID.equals(clientId);
+    }
+
     /**
      * Gets the profile creation date and persists it if it had to be generated.
      *
      * To get this value, we first look in times.json. If that could not be accessed, we
      * return the package's first install date. This is not a perfect solution because a
      * user may have large gap between install time and first use.
      *
      * A more correct algorithm could be the one performed by the JS code in ProfileAge.jsm
@@ -953,17 +973,17 @@ public final class GeckoProfile {
         } catch (Exception e) {
             // Best-effort.
             Log.w(LOGTAG, "Couldn't write " + TIMES_PATH, e);
         }
 
         // Create the client ID file before Gecko starts (we assume this method
         // is called before Gecko starts). If we let Gecko start, the JS telemetry
         // code may try to write to the file at the same time Java does.
-        persistClientId(generateNewClientId());
+        persistNewClientId(null, generateNewClientId());
 
         return profileDir;
     }
 
     /**
      * This method is called once, immediately before creation of the profile
      * directory completes.
      *