Attempt to fix Invalidations client name crash

The first attempt at setting consistent invalidator IDs, r234524, has
turned out to be a failure.  It was based on the assumption that the
InvalidationController was the only object that would start the
InvalidationService.  That assumption was wrong.

This CL takes a new approach.  Rather than having the
InvalidationController manage the ID and send it to the
InvalidationService, we now managed it in the
InvalidationClientNameProvider class, which is avialable to both the
InvalidationController and InvalidationService.  This allows both
classes to synchronously request a client name as they need it.

The InvalidationClientNameProvider is, by default, backed by an
InvalidationClientNameGenerator that provides it with fully random IDs.
This is not ideal, but it's better than the other alternatives.

Once this change is fully implemented, the downstream code will invoke
UniqueIdInvalidationClientNameGenerator.doInitializeAndInstallGenerator()
during early init.  This will initialize a
UniqueIdInvalidationClientNameGenerator with a
UuuidBasedUniqueIdentificationGenerator that will provide a consistent
identifier across restarts, and inject this
InvalidationClientNameGenerator into the InvalidationClientNameProvider.

BUG=320873

Review URL: https://codereview.chromium.org/77243002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@236681 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 171 insertions, 29 deletions

diff --git a/sync/android/java/src/org/chromium/sync/notifier/InvalidationClientNameGenerator.java b/sync/android/java/src/org/chromium/sync/notifier/InvalidationClientNameGenerator.java
new file mode 100644
index 0000000..dcc95e4
--- /dev/null
+++ b/sync/android/java/src/org/chromium/sync/notifier/InvalidationClientNameGenerator.java
@@ -0,0 +1,10 @@
+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.sync.notifier;
+
+/** Interface for classes that create an Invalidation client's name. */
+public interface InvalidationClientNameGenerator {
+  public byte[] generateInvalidatorClientName();
+}
diff --git a/sync/android/java/src/org/chromium/sync/notifier/InvalidationClientNameProvider.java b/sync/android/java/src/org/chromium/sync/notifier/InvalidationClientNameProvider.java
new file mode 100644
index 0000000..08e0062
--- /dev/null
+++ b/sync/android/java/src/org/chromium/sync/notifier/InvalidationClientNameProvider.java
@@ -0,0 +1,51 @@
+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.sync.notifier;
+
+/**
+ * An injectable singleton that provides an invalidation client with an appropriate unique name.
+ *
+ * This singleton will always provide a somewhat reasonable name.  With proper support from outside
+ * components, it will be able to provide a name that is consistent across restarts.
+ */
+public class InvalidationClientNameProvider {
+    private static final Object LOCK = new Object();
+
+    private static InvalidationClientNameProvider sInstance;
+
+    private final Object mLock;
+
+    private InvalidationClientNameGenerator mGenerator;
+
+    private byte[] mUniqueId;
+
+    public static InvalidationClientNameProvider get() {
+        synchronized (LOCK) {
+           if (sInstance == null) {
+               sInstance = new InvalidationClientNameProvider();
+           }
+           return sInstance;
+        }
+    }
+
+    InvalidationClientNameProvider() {
+        mLock = new Object();
+        mGenerator = new RandomizedInvalidationClientNameGenerator();
+    }
+
+    /** Returns a consistent unique string of bytes for use as an invalidator client ID. */
+    public byte[] getInvalidatorClientName() {
+        synchronized (mLock) {
+            if (mUniqueId == null) {
+                mUniqueId = mGenerator.generateInvalidatorClientName();
+            }
+            return mUniqueId;
+        }
+    }
+
+    public void setPreferredClientNameGenerator(InvalidationClientNameGenerator generator) {
+        mGenerator = generator;
+    }
+}
diff --git a/sync/android/java/src/org/chromium/sync/notifier/InvalidationIntentProtocol.java b/sync/android/java/src/org/chromium/sync/notifier/InvalidationIntentProtocol.java
index e3d5135..c87b894 100644
--- a/sync/android/java/src/org/chromium/sync/notifier/InvalidationIntentProtocol.java
+++ b/sync/android/java/src/org/chromium/sync/notifier/InvalidationIntentProtocol.java
@@ -35,11 +35,6 @@ public class InvalidationIntentProtocol {
     public static final String EXTRA_ACCOUNT = "account";
 
     /**
-     * byte[]-valued intent extra containing the unique client ID.
-     */
-    public static final String EXTRA_CLIENT_NAME = "client_name";
-
-    /**
      * String-list-valued intent extra of the syncable types to sync.
      */
     public static final String EXTRA_REGISTERED_TYPES = "registered_types";
diff --git a/sync/android/java/src/org/chromium/sync/notifier/InvalidationService.java b/sync/android/java/src/org/chromium/sync/notifier/InvalidationService.java
index cd8b605..24a77cd 100644
--- a/sync/android/java/src/org/chromium/sync/notifier/InvalidationService.java
+++ b/sync/android/java/src/org/chromium/sync/notifier/InvalidationService.java
@@ -75,11 +75,6 @@ public class InvalidationService extends AndroidListener {
      */
     @Nullable private static byte[] sClientId;
 
-    /**
-     * A ID that uniquely identifies this client.  Used for reflection blocking.
-     */
-    @Nullable private byte[] mClientName;
-
     @Override
     public void onHandleIntent(Intent intent) {
         // Ensure that a client is or is not running, as appropriate, and that it is for the
@@ -91,16 +86,6 @@ public class InvalidationService extends AndroidListener {
                 (Account) intent.getParcelableExtra(InvalidationIntentProtocol.EXTRA_ACCOUNT)
                 : null;
 
-        // Any intents sent to the InvalidationService should include the EXTRA_CLIENT_NAME.  The
-        // call to ensureClientStartState() might need a client name, and would break if we don't
-        // have one.
-        //
-        // Intents that are addressed to the AndroidListener portion of this class do not need to
-        // include the EXTRA_CLIENT_NAME.
-        if (intent.hasExtra(InvalidationIntentProtocol.EXTRA_CLIENT_NAME)) {
-            mClientName = intent.getByteArrayExtra(InvalidationIntentProtocol.EXTRA_CLIENT_NAME);
-        }
-
         ensureAccount(account);
         ensureClientStartState();
 
@@ -281,8 +266,8 @@ public class InvalidationService extends AndroidListener {
      * {@link InvalidationPreferences#setAccount}.
      */
     private void startClient() {
-        assert (mClientName != null);
-        Intent startIntent = AndroidListener.createStartIntent(this, CLIENT_TYPE, mClientName);
+        byte[] clientName = InvalidationClientNameProvider.get().getInvalidatorClientName();
+        Intent startIntent = AndroidListener.createStartIntent(this, CLIENT_TYPE, clientName);
         startService(startIntent);
         setIsClientStarted(true);
     }
diff --git a/sync/android/java/src/org/chromium/sync/notifier/RandomizedInvalidationClientNameGenerator.java b/sync/android/java/src/org/chromium/sync/notifier/RandomizedInvalidationClientNameGenerator.java
new file mode 100644
index 0000000..6374f51
--- /dev/null
+++ b/sync/android/java/src/org/chromium/sync/notifier/RandomizedInvalidationClientNameGenerator.java
@@ -0,0 +1,41 @@
+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.sync.notifier;
+
+import android.util.Base64;
+
+import org.chromium.sync.notifier.InvalidationClientNameGenerator;
+
+import java.util.Random;
+
+/**
+ * Generates a fully random client ID.
+ *
+ * This ID will not persist across restarts.  Using this ID will break the invalidator's "reflection
+ * blocking" feature.  That's unfortunate, but better than using a hard-coded ID.  A hard-coded ID
+ * could prevent invalidations from being delivered.
+ */
+class RandomizedInvalidationClientNameGenerator implements InvalidationClientNameGenerator {
+    private static final Random RANDOM = new Random();
+
+    RandomizedInvalidationClientNameGenerator() {}
+
+    /**
+     * Generates a random ID prefixed with the string "BadID".
+     *
+     * The prefix is intended to grab attention.  We should never use it in real builds.  Hopefully,
+     * it will induce someone to file a bug if they see it.
+     *
+     * However, as bad as it is, this ID is better than a hard-coded default or none at all.  See
+     * the class description for more details.
+     */
+    public byte[] generateInvalidatorClientName() {
+        byte[] randomBytes = new byte[8];
+        RANDOM.nextBytes(randomBytes);
+        String encoded = Base64.encodeToString(randomBytes, 0, randomBytes.length, Base64.NO_WRAP);
+        String idString = "BadID" + encoded;
+        return idString.getBytes();
+    }
+}
diff --git a/sync/android/javatests/src/org/chromium/sync/notifier/InvalidationClientNameProviderTest.java b/sync/android/javatests/src/org/chromium/sync/notifier/InvalidationClientNameProviderTest.java
new file mode 100644
index 0000000..d24aa89
--- /dev/null
+++ b/sync/android/javatests/src/org/chromium/sync/notifier/InvalidationClientNameProviderTest.java
@@ -0,0 +1,67 @@
+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.sync.notifier;
+
+import android.test.InstrumentationTestCase;
+import android.test.suitebuilder.annotation.SmallTest;
+
+import org.chromium.base.test.util.Feature;
+import org.chromium.sync.notifier.InvalidationClientNameProvider;
+
+import java.util.Arrays;
+
+/** Tests for the {@link InvalidationClientNameProvider} */
+public class InvalidationClientNameProviderTest extends InstrumentationTestCase {
+    private InvalidationClientNameProvider mProvider;
+
+    @Override
+    protected void setUp() {
+        mProvider = new InvalidationClientNameProvider();
+    }
+
+    @SmallTest
+    @Feature({"Sync"})
+    public void testFallbackClientId() {
+        // Test that the InvalidationController consistently returns the same ID even when it has to
+        // resort to its "fallback" ID generation code.
+        byte[] id1 = mProvider.getInvalidatorClientName();
+        byte[] id2 = mProvider.getInvalidatorClientName();
+
+        // We expect the returned IDs to be consistent in every call.
+        assertTrue("Expected returned IDs to be consistent", Arrays.equals(id1, id2));
+
+        // Even if initialize the generator late, the ID will remain consistent.
+        registerHardCodedGenerator(mProvider);
+
+        // IDs should still be consistent, even if we change the generator.
+        // (In the real program, the generator should be set before anyone invokes the
+        // getInvalidatorClientName() and never change afterwards.  We test this anyway to make sure
+        // nothing will blow up if someone accidentally violates that constraint.)
+        byte[] id3 = mProvider.getInvalidatorClientName();
+        assertTrue("Changing generators should not affect returned ID consistency",
+            Arrays.equals(id2, id3));
+    }
+
+    @SmallTest
+    @Feature({"Sync"})
+    public void testPreRegisteredGenerator() {
+        registerHardCodedGenerator(mProvider);
+
+        byte[] id = mProvider.getInvalidatorClientName();
+        byte[] id2 = mProvider.getInvalidatorClientName();
+
+        // Expect that consistent IDs are maintained when using a custom generator, too.
+        assertTrue("Custom generators should return consistent IDs", Arrays.equals(id, id2));
+    }
+
+    private static void registerHardCodedGenerator(InvalidationClientNameProvider provider) {
+        provider.setPreferredClientNameGenerator(
+            new InvalidationClientNameGenerator() {
+                public byte[] generateInvalidatorClientName() {
+                    return "Testable ID".getBytes();
+                }
+            });
+    }
+}
diff --git a/sync/android/javatests/src/org/chromium/sync/notifier/InvalidationServiceTest.java b/sync/android/javatests/src/org/chromium/sync/notifier/InvalidationServiceTest.java
index b4b536f..fcdc65e 100644
--- a/sync/android/javatests/src/org/chromium/sync/notifier/InvalidationServiceTest.java
+++ b/sync/android/javatests/src/org/chromium/sync/notifier/InvalidationServiceTest.java
@@ -41,9 +41,6 @@ public class InvalidationServiceTest extends ServiceTestCase<TestableInvalidatio
     /** Id used when creating clients. */
     private static final byte[] CLIENT_ID = new byte[]{0, 4, 7};
 
-    /** Id used to uniquely name this client instance. */
-    private static final byte[] TEST_CLIENT_NAME = "UNIQUE_CLIENT_NAME".getBytes();
-
     /** Intents provided to {@link #startService}. */
     private List<Intent> mStartServiceIntents;
 
@@ -817,7 +814,6 @@ public class InvalidationServiceTest extends ServiceTestCase<TestableInvalidatio
     /** Creates an intent to start the InvalidationService. */
     private Intent createStartIntent() {
       Intent intent = new Intent();
-      intent.putExtra(InvalidationIntentProtocol.EXTRA_CLIENT_NAME, TEST_CLIENT_NAME);
       return intent;
     }
 
@@ -825,14 +821,12 @@ public class InvalidationServiceTest extends ServiceTestCase<TestableInvalidatio
     private Intent createStopIntent() {
       Intent intent = new Intent();
       intent.putExtra(InvalidationIntentProtocol.EXTRA_STOP, true);
-      intent.putExtra(InvalidationIntentProtocol.EXTRA_CLIENT_NAME, TEST_CLIENT_NAME);
       return intent;
     }
 
     /** Creates an intent to register some types with the InvalidationService. */
     private Intent createRegisterIntent(Account account, boolean allTypes, Set<ModelType> types) {
       Intent intent = InvalidationIntentProtocol.createRegisterIntent(account, allTypes, types);
-      intent.putExtra(InvalidationIntentProtocol.EXTRA_CLIENT_NAME, TEST_CLIENT_NAME);
       return intent;
     }
 
@@ -841,7 +835,6 @@ public class InvalidationServiceTest extends ServiceTestCase<TestableInvalidatio
         Account account, int[] objectSources, String[] objectNames) {
       Intent intent = InvalidationIntentProtocol.createRegisterIntent(
           account, objectSources, objectNames);
-      intent.putExtra(InvalidationIntentProtocol.EXTRA_CLIENT_NAME, TEST_CLIENT_NAME);
       return intent;
     }