Add fake for InstanceIDWithSubtype.java, in order to re-use unit test

components/gcm_driver/instance_id/android/java/src/org/chromium/components/gcm_driver/instance_id/InstanceIDWithSubtype.java

Index: components/gcm_driver/instance_id/android/java/src/org/chromium/components/gcm_driver/instance_id/InstanceIDWithSubtype.java
diff --git a/components/gcm_driver/instance_id/android/java/src/org/chromium/components/gcm_driver/instance_id/InstanceIDWithSubtype.java b/components/gcm_driver/instance_id/android/java/src/org/chromium/components/gcm_driver/instance_id/InstanceIDWithSubtype.java
index 8af5f6e3a3ab82669d5b8bb60d3fb302b45da8d7..704006c7efcc2db5ea1370c94eee47e8f45b42d4 100644
--- a/components/gcm_driver/instance_id/android/java/src/org/chromium/components/gcm_driver/instance_id/InstanceIDWithSubtype.java
+++ b/components/gcm_driver/instance_id/android/java/src/org/chromium/components/gcm_driver/instance_id/InstanceIDWithSubtype.java
@@ -23,7 +23,10 @@ public class InstanceIDWithSubtype extends InstanceID {
     /** Cached instances. May be accessed from multiple threads; synchronize on InstanceID.class. */
     private static Map<String, InstanceIDWithSubtype> sSubtypeInstances = new HashMap<>();
 
-    private InstanceIDWithSubtype(Context context, String subtype) {
+    /** Fake subclasses can set this so getInstance creates instances of them. */
+    protected static FakeFactory sFakeFactoryForTesting = null;

[Peter Beverloo, 2016/04/15 00:00:17]

Maybe remove "protected" and mark this as @VisibleForTesting?

[johnme, 2016/04/15 16:00:42]

Marked it as @VisibleForTesting, but kept protected to hint that the subclass is
the one accessing it for testing purposes. Also made
FakeInstanceIDWithSubtype.clearDataAndSetEnabled set sFakeFactoryForTesting
within its synchronized block, in case someone calls clearDataAndSetEnabled
while an AsyncTask is executing getInstance.

+
+    protected InstanceIDWithSubtype(Context context, String subtype) {
         super(context, subtype, null /* options */);
         mSubtype = subtype;
     }
@@ -41,7 +44,7 @@ public class InstanceIDWithSubtype extends InstanceID {
         // Synchronize on the base class, to match the synchronized statements in
         // InstanceID.getInstance.
         synchronized (InstanceID.class) {
-            if (sSubtypeInstances.isEmpty()) {
+            if (sSubtypeInstances.isEmpty() && sFakeFactoryForTesting == null) {
                 // The static InstanceID.getInstance method performs some one-time initialization
                 // logic that is also necessary for users of this sub-class. To work around this,
                 // first get (but don't use) the default InstanceID.
@@ -50,7 +53,11 @@ public class InstanceIDWithSubtype extends InstanceID {
 
             InstanceIDWithSubtype existing = sSubtypeInstances.get(subtype);
             if (existing == null) {
-                existing = new InstanceIDWithSubtype(context, subtype);
+                if (sFakeFactoryForTesting != null) {
+                    existing = sFakeFactoryForTesting.create(context, subtype);
+                } else {
+                    existing = new InstanceIDWithSubtype(context, subtype);
+                }
                 sSubtypeInstances.put(subtype, existing);
             }
             return existing;
@@ -69,4 +76,21 @@ public class InstanceIDWithSubtype extends InstanceID {
     public String getSubtype() {
         return mSubtype;
     }
+
+    /** Stored instances. May be accessed from multiple threads; synchronize on InstanceID.class. */
+    public static Map<String, InstanceIDWithSubtype> getInstanceIDsForTesting() {

[Peter Beverloo, 2016/04/15 00:00:17]

Why are you making sFakeFactoryForTesting visible to the sub-class, but require
the use of getInstanceIDsForTesting() for clearing or removing a subtype? Let's
trust ourselves to do the right thing (regarding the exception you throw on :83)
and mark both as @VisibleForTesting?

[johnme, 2016/04/15 16:00:43]

Because I'm going to need access to getInstanceIDsForTesting() from a Java test
in a later patch :p. But on second thoughts, I'll just mark sSubtypeInstances
"@VisibleForTesting public" in the follow-on patch, which is neater. So done.

+        if (!Thread.holdsLock(InstanceID.class)) {
+            throw new AssertionError("Should be synchronized on InstanceID.class");
+        }
+        return sSubtypeInstances;
+    }
+
+    public static boolean usingFake() {

[Peter Beverloo, 2016/04/15 00:00:17]

Unused.

[johnme, 2016/04/15 16:00:43]

Removed.

+        return sFakeFactoryForTesting != null;
+    }
+
+    /** Fake subclasses can set {@link #sFakeFactoryForTesting} to an implementation of this. */
+    public interface FakeFactory {
+        public InstanceIDWithSubtype create(Context context, String subtype);
+    }
 }