WebView: Refactor PlatformServiceBridge.getInstance

android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java

Index: android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java
diff --git a/android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java b/android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java
index 0ced33e7389f288c0330013f211c76d31f63f0a4..1c957a0ae3732c548c7d93f25339009e2014ab93 100644
--- a/android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java
+++ b/android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java
@@ -21,41 +21,53 @@ public class PlatformServiceBridge {
     private static final String PLATFORM_SERVICE_BRIDGE =
             "com.android.webview.chromium.PlatformServiceBridgeGoogle";
 
-    private static final Object sInstanceLock = new Object();
     private static PlatformServiceBridge sInstance;

[Tobias Sargeant, 2017/04/05 13:00:21]

Seeing as this is set once, but can be read from multiple threads, mark as
volatile.

[gsennton, 2017/04/05 15:21:13]

Huh, is this not always called on the UI thread? (if it is we should have a
ThreadUtils assertion in getInstance()).

[paulmiller, 2017/04/05 17:02:39]

Done.

[paulmiller, 2017/04/05 17:02:39]

Safe Browsing uses getInstance on a different thread. I'll add a comment to
clarify.

 
     protected PlatformServiceBridge() {}
 
-    public static PlatformServiceBridge getInstance(Context appContext) {
-        // TODO(timvolodine): consider removing the lock, crbug.com/681805.
-        synchronized (sInstanceLock) {
-            if (sInstance != null) {
-                return sInstance;
-            }
-
-            // Try to get a specialized service bridge.
-            try {
-                Class<?> cls = Class.forName(PLATFORM_SERVICE_BRIDGE);
-                sInstance = (PlatformServiceBridge) cls.getDeclaredConstructor(Context.class)
-                                    .newInstance(appContext);
-                return sInstance;
-            } catch (ClassNotFoundException e) {
-                // This is not an error; it just means this device doesn't have specialized
-                // services.
-            } catch (IllegalAccessException | IllegalArgumentException | InstantiationException
-                    | NoSuchMethodException e) {
-                Log.e(TAG, "Failed to get " + PLATFORM_SERVICE_BRIDGE + ": " + e);
-            } catch (InvocationTargetException e) {
-                Log.e(TAG, "Failed invocation to get " + PLATFORM_SERVICE_BRIDGE + ":",
-                        e.getCause());
-            }
-
-            // Otherwise, get the generic service bridge.
-            sInstance = new PlatformServiceBridge();
+    public static PlatformServiceBridge getOrCreateInstance(Context context) {
+        // Just to avoid race conditions - nothing special about the UI thread.
+        ThreadUtils.assertOnUiThread();
+
+        if (sInstance != null) return sInstance;
+
+        // Try to get a specialized service bridge.
+        try {
+            Class<?> cls = Class.forName(PLATFORM_SERVICE_BRIDGE);
+            sInstance = (PlatformServiceBridge) cls.getDeclaredConstructor(Context.class)
+                                .newInstance(context);
+            return sInstance;
+        } catch (ClassNotFoundException e) {
+            // This is not an error; it just means this device doesn't have specialized
+            // services.
+        } catch (IllegalAccessException | IllegalArgumentException | InstantiationException
+                | NoSuchMethodException e) {
+            Log.e(TAG, "Failed to get " + PLATFORM_SERVICE_BRIDGE + ": " + e);
+        } catch (InvocationTargetException e) {
+            Log.e(TAG, "Failed invocation to get " + PLATFORM_SERVICE_BRIDGE + ":", e.getCause());
         }
+
+        // Otherwise, get the generic service bridge.
+        sInstance = new PlatformServiceBridge();
+
+        return sInstance;
+    }
+
+    public static PlatformServiceBridge getInstance() {
+        if (sInstance == null) throw new IllegalStateException("PlatformServiceBridge not created");
         return sInstance;
     }
 
+    // Provide a mocked PlatformServiceBridge for testing.
+    public static void injectInstance(PlatformServiceBridge testBridge) {
+        sInstance = testBridge;
+    }
+
+    // TODO(paulmiller): Remove after changing downstream users.
+    public static PlatformServiceBridge getInstance(Context context) {
+        return getInstance();
+    }
+
     // Can WebView use Google Play Services (a.k.a. GMS)?
     public boolean canUseGms() {
         return false;