Prepare to call GMS APIs from WebView

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 8818ce0ffe80aa130af2fe31b9a84122f4bce74e..e38271d379baa8086b2f7fad1d8e6d0b86e67c10 100644
--- a/android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java
+++ b/android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java
@@ -24,7 +24,7 @@ public class PlatformServiceBridge {
 
     protected PlatformServiceBridge() {}
 
-    public static PlatformServiceBridge getInstance(Context applicationContext) {
+    public static PlatformServiceBridge getInstance(Context appContext) {
         if (sInstance != null) {
             return sInstance;
         }
@@ -33,7 +33,7 @@ public class PlatformServiceBridge {
         try {
             Class<?> cls = Class.forName(PLATFORM_SERVICE_BRIDGE);
             sInstance = (PlatformServiceBridge) cls.getDeclaredConstructor(Context.class)
-                    .newInstance(applicationContext);
+                    .newInstance(appContext);
             return sInstance;
         } catch (ClassNotFoundException e) {
             // This is not an error; it just means this device doesn't have specialized services.
@@ -57,5 +57,9 @@ public class PlatformServiceBridge {
         return false;
     }
 
-    public void setMetricsSettingListener(ValueCallback<Boolean> callback) {}
+    // Overriding implementations may call "callback" asynchronously, but they must still call

[gsennton, 2017/01/04 10:09:55]

So we will use this method by spawning a new thread, calling queryMetricsEnabled
on that thread, and then wait for that thread to receive the callback? 

[paulmiller, 2017/01/04 18:12:41]

You could do that. I don't. See the AwMetricsServiceClient constructor. It's all
on the main thread; it calls queryMetricsEnabled, and then the thread is free to
do other things, and it'll come back and actually enable metrics whenever the
callback fires.

I could make queryMetricsEnabled take a Handler, so you could specify which
thread you want the callback on. But I didn't imagine anyone would need that. So
I just promised that the callback comes back on the same thread, since that
seemed reasonable and unsurprising. Do you need something else?

[gsennton, 2017/01/04 18:35:04]

Well, I think we (silent feedback) would want to wait until we got a response
from GmsCore before we start uploading minidumps.

Our upload service is triggered on the main thread and then performs minidump
uploading on a worker thread. So I guess we could just ask GmsCore on the main
thread, and later wait for the response inside the worker thread (sleeping until
the main thread receives the value).

[paulmiller, 2017/01/04 18:45:00]

If the worker thread is persistent, yes, that would work. If not, then you don't
need to sleep anywhere. Just spawn the worker in the callback, on the main
thread. Adding the Handler argument is easy, too. Would you use it if I added
it? Then you could query on the main thread, and the callback could run on the
worker thread (which would need to be persistent, and have a Looper).

[gsennton, 2017/01/04 20:31:01]

Oops, I meant waiting in the worker thread as in sleeping until the main thread
received an answer :). It would be a bit fiddly to do as you suggest here I
think (but I might be wrong at this time of day ;)) - because we would need a
special case for when we have explicitly added a command-line-flag to enable the
feature even if the U&D toggle is OFF.

So to clarify, I suggest keeping the current implementation of receiving the
callback on the main thread, have silent-feedback use this by calling gms core
from the main thread, receiving the answer on the main thread, and have the
worker thread that uploads minidumps just sleep until the main thread receives
an answer. Does that sound reasonable/correct? I think this solution is easier
in terms of me not having to change any instrumentation tests ;).

[sgurun-gerrit only, 2017/01/04 20:33:05]

As far as I remember GMSCore always calls you back on the main thread (please
confirm it), which means the callback here is going to be called from main
thread.

I think it is simple to simply require this method to be called from main
thread.

[paulmiller, 2017/01/04 22:26:12]

By default, GMS calls back on the app's UI thread. But you can pass a Handler
for any other thread. We need to pass a Handler anyway, since WebView's UI
thread might not be the app's UI thread.

But, sure. I'll just put ThreadUtils.assertOnUiThread() everywhere.

+    // "callback" on the same thread on which "queryMetricsEnabled" was called.
+    public void queryMetricsEnabled(ValueCallback<Boolean> callback) {

[sgurun-gerrit only, 2017/01/04 00:05:24]

I prefer allowMetricsService() as the method name. 

[paulmiller, 2017/01/04 00:10:15]

Why is that? That sounds like we're setting something rather than getting it. I
also try to avoid words like "service"--I think they're too vague.

[sgurun-gerrit only, 2017/01/04 00:31:53]

queryMetricsValue could be better choice. queryMetricsEnabled does not sound
right to my ear. 

[paulmiller, 2017/01/04 00:54:40]

How about "queryMetricsSetting"? That's slightly more specific than "value".

+        callback.onReceiveValue(false);
+    }
 }