WebView: Refactor PlatformServiceBridge.getInstance

android_webview/glue/java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java

Index: android_webview/glue/java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java
diff --git a/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java b/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java
index dd5951fda786f115e10b37f5630957a80a43c4a7..8c6d1e5ed8574190f4e0b1de1d11f6f9d1706a80 100644
--- a/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java
+++ b/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java
@@ -420,8 +420,8 @@ public class WebViewChromiumFactoryProvider implements WebViewFactoryProvider {
         }
 
         // Actions conditioned on whether the Android Checkbox is toggled on

[timvolodine, 2017/04/03 12:18:52]

perhaps also provide a comment that the "createInstance" call creates a
singleton instance used elsewhere and that it's crucial to do this here at
startup i.e. the location of this code is important.

[paulmiller, 2017/04/03 23:34:16]

Done.

-        PlatformServiceBridge.getInstance(context)
-                .queryMetricsSetting(new ValueCallback<Boolean>() {
+        PlatformServiceBridge.createInstance(context).queryMetricsSetting(

[Tobias Sargeant, 2017/04/03 10:40:06]

Keep this getInstance() and add a createInstance() earlier in the method? I had
to check that handleMinidumps didn't rely on the instance having been created,
and I'm not certain that there aren't other methods that start operations on
other threads above.

[paulmiller, 2017/04/03 23:34:16]

IMHO, if we preemptively create our singletons as early as possible, that
removes the create-on-first-use benefit of singletons. The only case we need to
worry about is calling "getInstance(null)" (because that's the version that
can't create if it's the first use) and there's only 1 of those:

https://cs.chromium.org/search/?q=PlatformServiceBridge%5C.getInstance%5C(nul...

[Tobias Sargeant, 2017/04/04 12:30:47]

Is create-on-first-use really that much of a benefit, though? Especially if
there are cases where we can't create? It becomes kind of hard to reason about
whether those cases execute before or after creation of the singleton instance.

In the example here it would be easy for the AsyncTask that handleMinidumps()
creates to be changed to call getInstance (because it doesn't have a context to
call getOrCreateInstance with) and have that be unintentionally racy. If we can
ensure that that can't happen now by creating the instance earlier, shouldn't
we?

+                new ValueCallback<Boolean>() {
                     public void onReceiveValue(Boolean enabled) {
                         ThreadUtils.assertOnUiThread();
                         AwMetricsServiceClient.setConsentSetting(context, enabled);