Prepare to call GMS APIs from WebView

Prepare to call GMS APIs from WebView

- Switch setMetricsSettingListener from a listener to a 1-time query.
- Replace tryEnableGms with canUseGms. (tryEnableGms will actually be
  removed in a follow-up, since overloading implementations elsewhere
  must be removed first.)
- Fix the race condition between native AwMetricsServiceClient
  initialization and Java metrics setting query. The Java side will wait
  for native initialization before switching on metrics.
- Move the native AwMetricsServiceClient implementation to native/,
  replacing AwMetricsServiceSwitch, so it can signal the Java
  AwMetricsServiceClient when it's ready.
- Fix AwTestBase, which was incorrectly creating AwBrowserContext on the 
  test thread. That was preventing us from using assertOnUiThread.
- ...which requires fixing AwStrictModeTest, which was going out of its 
  way to create AwBrowserContext on the UI thread, as runOnMainSync
  doesn't allow redundant invocations.
- Some miscellaneous comment & whitespace cleanup.

BUG=676523
Review-Url: https://codereview.chromium.org/2611883002
Cr-Commit-Position: refs/heads/master@{#443776}
Committed: https://chromium.googlesource.com/chromium/src/+/85990128f90e66efb2d1a83caeca578ab93c275e

[paulmiller, 2017-01-03 23:26:19]

Description was changed from

==========
Prepare to call GMS APIs from WebView

Switch setMetricsSettingListener from a listener to a 1-time query.

BUG=676523
==========

to

==========
Prepare to call GMS APIs from WebView

Switch setMetricsSettingListener from a listener to a 1-time query.

BUG=676523
==========

[paulmiller, 2017-01-03 23:26:19]

paulmiller@chromium.org changed reviewers:
+ sgurun@chromium.org

[paulmiller, 2017-01-03 23:26:31]

On 2017/01/03 23:26:19, paulmiller wrote:
> mailto:paulmiller@chromium.org changed reviewers:
> + mailto:sgurun@chromium.org

PTAL

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

On 2017/01/03 23:26:31, paulmiller wrote:
> On 2017/01/03 23:26:19, paulmiller wrote:
> > mailto:paulmiller@chromium.org changed reviewers:
> > + mailto:sgurun@chromium.org
> 
> PTAL

lgtm with one comment.

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

https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
File
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java
(right):

https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java:62:
public void queryMetricsEnabled(ValueCallback<Boolean> callback) {
I prefer allowMetricsService() as the method name.

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

https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
File
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java
(right):

https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java:62:
public void queryMetricsEnabled(ValueCallback<Boolean> callback) {
On 2017/01/04 00:05:24, sgurun wrote:
> I prefer allowMetricsService() as the method name. 

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]

https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
File
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java
(right):

https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java:56:
public boolean tryEnableGms() {
remove this method.

https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java:62:
public void queryMetricsEnabled(ValueCallback<Boolean> callback) {
On 2017/01/04 00:10:15, paulmiller wrote:
> On 2017/01/04 00:05:24, sgurun wrote:
> > I prefer allowMetricsService() as the method name. 
> 
> 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.

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

[paulmiller, 2017-01-04 00:54:41]

https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
File
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java
(right):

https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java:62:
public void queryMetricsEnabled(ValueCallback<Boolean> callback) {
On 2017/01/04 00:31:53, sgurun wrote:
> On 2017/01/04 00:10:15, paulmiller wrote:
> > On 2017/01/04 00:05:24, sgurun wrote:
> > > I prefer allowMetricsService() as the method name. 
> > 
> > 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.
> 
> queryMetricsValue could be better choice. queryMetricsEnabled does not sound
> right to my ear. 

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

[sgurun-gerrit only, 2017-01-04 01:09:08]

On 2017/01/04 00:54:41, paulmiller wrote:
>
https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
> File
>
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java
> (right):
> 
>
https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
>
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java:62:
> public void queryMetricsEnabled(ValueCallback<Boolean> callback) {
> On 2017/01/04 00:31:53, sgurun wrote:
> > On 2017/01/04 00:10:15, paulmiller wrote:
> > > On 2017/01/04 00:05:24, sgurun wrote:
> > > > I prefer allowMetricsService() as the method name. 
> > > 
> > > 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.
> > 
> > queryMetricsValue could be better choice. queryMetricsEnabled does not sound
> > right to my ear. 
> 
> How about "queryMetricsSetting"? That's slightly more specific than "value".

good.

[gsennton, 2017-01-04 10:09:54]

gsennton@chromium.org changed reviewers:
+ gsennton@chromium.org

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

https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
File
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java
(right):

https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java:60:
// Overriding implementations may call "callback" asynchronously, but they must
still call
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]

https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
File
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java
(right):

https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java:60:
// Overriding implementations may call "callback" asynchronously, but they must
still call
On 2017/01/04 10:09:55, gsennton wrote:
> 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? 

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]

https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
File
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java
(right):

https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java:60:
// Overriding implementations may call "callback" asynchronously, but they must
still call
On 2017/01/04 18:12:41, paulmiller wrote:
> On 2017/01/04 10:09:55, gsennton wrote:
> > 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? 
> 
> 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?

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]

Hmmm, trybots failing with nativeSetMetricsEnabled "Native method not found". It
works for me locally, though.

https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
File
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java
(right):

https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java:60:
// Overriding implementations may call "callback" asynchronously, but they must
still call
On 2017/01/04 18:35:04, gsennton wrote:
> On 2017/01/04 18:12:41, paulmiller wrote:
> > On 2017/01/04 10:09:55, gsennton wrote:
> > > 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? 
> > 
> > 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?
> 
> 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).

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]

https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
File
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java
(right):

https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java:60:
// Overriding implementations may call "callback" asynchronously, but they must
still call
On 2017/01/04 18:45:00, paulmiller wrote:
> On 2017/01/04 18:35:04, gsennton wrote:
> > On 2017/01/04 18:12:41, paulmiller wrote:
> > > On 2017/01/04 10:09:55, gsennton wrote:
> > > > 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? 
> > > 
> > > 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?
> > 
> > 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).
> 
> 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).

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]

I don't know why the bots fail, skimmed through but nothing popped up. let me
know if you cannot figure it out.

https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
File
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java
(right):

https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java:60:
// Overriding implementations may call "callback" asynchronously, but they must
still call
On 2017/01/04 18:45:00, paulmiller wrote:
> On 2017/01/04 18:35:04, gsennton wrote:
> > On 2017/01/04 18:12:41, paulmiller wrote:
> > > On 2017/01/04 10:09:55, gsennton wrote:
> > > > 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? 
> > > 
> > > 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?
> > 
> > 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).
> 
> 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).

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]

https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
File
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java
(right):

https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java:60:
// Overriding implementations may call "callback" asynchronously, but they must
still call
On 2017/01/04 20:33:05, sgurun wrote:
> On 2017/01/04 18:45:00, paulmiller wrote:
> > On 2017/01/04 18:35:04, gsennton wrote:
> > > On 2017/01/04 18:12:41, paulmiller wrote:
> > > > On 2017/01/04 10:09:55, gsennton wrote:
> > > > > 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?

> > > > 
> > > > 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?
> > > 
> > > 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).
> > 
> > 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).
> 
> 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.
> 

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.

[paulmiller, 2017-01-07 02:20:22]

Description was changed from

==========
Prepare to call GMS APIs from WebView

Switch setMetricsSettingListener from a listener to a 1-time query.

BUG=676523
==========

to

==========
Prepare to call GMS APIs from WebView

- Switch setMetricsSettingListener from a listener to a 1-time query.
- Replace tryEnableGms with canUseGms. (canUseGms will actually be
removed in a follow-up, since overloading implementations elsewhere
must be removed first.)
- Fix AwTestBase, which was incorrectly creating AwBrowserContext on
the test thread. That was preventing us from using assertOnUiThread.
- Since AwBrowserContext now requires native code, create the Activity
before the AwBrowserContext in AwTestBase.
- Remove "static" from aw_metrics_switch.cc, for consistency.

BUG=676523
==========

[paulmiller, 2017-01-09 18:09:41]

Description was changed from

==========
Prepare to call GMS APIs from WebView

- Switch setMetricsSettingListener from a listener to a 1-time query.
- Replace tryEnableGms with canUseGms. (canUseGms will actually be
removed in a follow-up, since overloading implementations elsewhere
must be removed first.)
- Fix AwTestBase, which was incorrectly creating AwBrowserContext on
the test thread. That was preventing us from using assertOnUiThread.
- Since AwBrowserContext now requires native code, create the Activity
before the AwBrowserContext in AwTestBase.
- Remove "static" from aw_metrics_switch.cc, for consistency.

BUG=676523
==========

to

==========
Prepare to call GMS APIs from WebView

- Switch setMetricsSettingListener from a listener to a 1-time query.
- Replace tryEnableGms with canUseGms. (tryEnableGms will actually be
removed in a follow-up, since overloading implementations elsewhere
must be removed first.)
- Fix AwTestBase, which was incorrectly creating AwBrowserContext on
the test thread. That was preventing us from using assertOnUiThread.
- Since AwBrowserContext now requires native code, create the Activity
before the AwBrowserContext in AwTestBase.
- Remove "static" from aw_metrics_switch.cc, for consistency.

BUG=676523
==========

[gsennton, 2017-01-10 12:47:54]

On 2017/01/04 22:26:12, paulmiller wrote:
>
https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
> File
>
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java
> (right):
> 
>
https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
>
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java:60:
> // Overriding implementations may call "callback" asynchronously, but they
must
> still call
> On 2017/01/04 20:33:05, sgurun wrote:
> > On 2017/01/04 18:45:00, paulmiller wrote:
> > > On 2017/01/04 18:35:04, gsennton wrote:
> > > > On 2017/01/04 18:12:41, paulmiller wrote:
> > > > > On 2017/01/04 10:09:55, gsennton wrote:
> > > > > > 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?
> 
> > > > > 
> > > > > 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?
> > > > 
> > > > 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).
> > > 
> > > 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).
> > 
> > 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.
> > 
> 
> 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.

Hi Paul, sorry for being a pain - I talked to Toby today, and to use your API to
check whether to enabled/disable minidump generation on WebView startup we need
to go with the post-async-back-to-handler solution. This is because
minidump-generation is enabled/disabled in native on the ui thread of startup,
so we can't post to the ui thread - because that call will then occur after the
native startup.
So what we would like is basically some kind of "Promise" that we can get from
your API - and when we reach the code where we start native we will just wait
for the Promise to be fulfilled/non-fulfilled.

We can talk about this more later today :).

[gsennton, 2017-01-10 12:48:25]

gsennton@chromium.org changed reviewers:
+ tobiasjs@chromium.org

[gsennton, 2017-01-10 12:48:26]

also cc'ing Toby as an FYI

[paulmiller, 2017-01-10 18:04:38]

On 2017/01/10 12:47:54, gsennton wrote:
> On 2017/01/04 22:26:12, paulmiller wrote:
> >
>
https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
> > File
> >
>
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java
> > (right):
> > 
> >
>
https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
> >
>
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java:60:
> > // Overriding implementations may call "callback" asynchronously, but they
> must
> > still call
> > On 2017/01/04 20:33:05, sgurun wrote:
> > > On 2017/01/04 18:45:00, paulmiller wrote:
> > > > On 2017/01/04 18:35:04, gsennton wrote:
> > > > > On 2017/01/04 18:12:41, paulmiller wrote:
> > > > > > On 2017/01/04 10:09:55, gsennton wrote:
> > > > > > > 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?
> > 
> > > > > > 
> > > > > > 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?
> > > > > 
> > > > > 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).
> > > > 
> > > > 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).
> > > 
> > > 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.
> > > 
> > 
> > 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.
> 
> Hi Paul, sorry for being a pain - I talked to Toby today, and to use your API
to
> check whether to enabled/disable minidump generation on WebView startup we
need
> to go with the post-async-back-to-handler solution. This is because
> minidump-generation is enabled/disabled in native on the ui thread of startup,
> so we can't post to the ui thread - because that call will then occur after
the
> native startup.
> So what we would like is basically some kind of "Promise" that we can get from
> your API - and when we reach the code where we start native we will just wait
> for the Promise to be fulfilled/non-fulfilled.
> 
> We can talk about this more later today :).

So what API do you need? Should I add a Handler argument after all? Do you want
a Future? Calling GMS is very slow; what exactly are you planning to block?

[gsennton, 2017-01-10 18:30:28]

On 2017/01/10 18:04:38, paulmiller wrote:
> On 2017/01/10 12:47:54, gsennton wrote:
> > On 2017/01/04 22:26:12, paulmiller wrote:
> > >
> >
>
https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
> > > File
> > >
> >
>
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java
> > > (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
> > >
> >
>
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java:60:
> > > // Overriding implementations may call "callback" asynchronously, but they
> > must
> > > still call
> > > On 2017/01/04 20:33:05, sgurun wrote:
> > > > On 2017/01/04 18:45:00, paulmiller wrote:
> > > > > On 2017/01/04 18:35:04, gsennton wrote:
> > > > > > On 2017/01/04 18:12:41, paulmiller wrote:
> > > > > > > On 2017/01/04 10:09:55, gsennton wrote:
> > > > > > > > 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?
> > > 
> > > > > > > 
> > > > > > > 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?
> > > > > > 
> > > > > > 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).
> > > > > 
> > > > > 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).
> > > > 
> > > > 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.
> > > > 
> > > 
> > > 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.
> > 
> > Hi Paul, sorry for being a pain - I talked to Toby today, and to use your
API
> to
> > check whether to enabled/disable minidump generation on WebView startup we
> need
> > to go with the post-async-back-to-handler solution. This is because
> > minidump-generation is enabled/disabled in native on the ui thread of
startup,
> > so we can't post to the ui thread - because that call will then occur after
> the
> > native startup.
> > So what we would like is basically some kind of "Promise" that we can get
from
> > your API - and when we reach the code where we start native we will just
wait
> > for the Promise to be fulfilled/non-fulfilled.
> > 
> > We can talk about this more later today :).
> 
> So what API do you need? Should I add a Handler argument after all? Do you
want
> a Future? Calling GMS is very slow; what exactly are you planning to block?

We want to get a response from GMS before we enable minidump generation - I
believe this means we want a response from GMS before we initialize native (but
I haven't looked at the code specifically - Toby knows this better than me).

If we can (are allowed) to block the UI thread, then what we want is a Future -
i.e. an object which we can wait for on the UI thread and which your API fills
in on the handler thread from the GmsCore callback.

We would block the initialization of the native side of WebView IIUC, though
this sounds scary :/
I'm not super familiar with the breakpad-enabling code (and I believe Toby still
needs to look over it) so I don't know whether we have a good alternative (i.e.
whether can defer the decision until later during startup).

[sgurun-gerrit only, 2017-01-10 20:07:29]

On 2017/01/10 18:30:28, gsennton wrote:
> On 2017/01/10 18:04:38, paulmiller wrote:
> > On 2017/01/10 12:47:54, gsennton wrote:
> > > On 2017/01/04 22:26:12, paulmiller wrote:
> > > >
> > >
> >
>
https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
> > > > File
> > > >
> > >
> >
>
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java
> > > > (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/2611883002/diff/1/android_webview/java/src/or...
> > > >
> > >
> >
>
android_webview/java/src/org/chromium/android_webview/PlatformServiceBridge.java:60:
> > > > // Overriding implementations may call "callback" asynchronously, but
they
> > > must
> > > > still call
> > > > On 2017/01/04 20:33:05, sgurun wrote:
> > > > > On 2017/01/04 18:45:00, paulmiller wrote:
> > > > > > On 2017/01/04 18:35:04, gsennton wrote:
> > > > > > > On 2017/01/04 18:12:41, paulmiller wrote:
> > > > > > > > On 2017/01/04 10:09:55, gsennton wrote:
> > > > > > > > > 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?
> > > > 
> > > > > > > > 
> > > > > > > > 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?
> > > > > > > 
> > > > > > > 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).
> > > > > > 
> > > > > > 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).
> > > > > 
> > > > > 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.
> > > > > 
> > > > 
> > > > 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.
> > > 
> > > Hi Paul, sorry for being a pain - I talked to Toby today, and to use your
> API
> > to
> > > check whether to enabled/disable minidump generation on WebView startup we
> > need
> > > to go with the post-async-back-to-handler solution. This is because
> > > minidump-generation is enabled/disabled in native on the ui thread of
> startup,
> > > so we can't post to the ui thread - because that call will then occur
after
> > the
> > > native startup.
> > > So what we would like is basically some kind of "Promise" that we can get
> from
> > > your API - and when we reach the code where we start native we will just
> wait
> > > for the Promise to be fulfilled/non-fulfilled.
> > > 
> > > We can talk about this more later today :).
> > 
> > So what API do you need? Should I add a Handler argument after all? Do you
> want
> > a Future? Calling GMS is very slow; what exactly are you planning to block?
> 
> We want to get a response from GMS before we enable minidump generation - I
> believe this means we want a response from GMS before we initialize native
(but
> I haven't looked at the code specifically - Toby knows this better than me).
> 
> If we can (are allowed) to block the UI thread, then what we want is a Future
-
> i.e. an object which we can wait for on the UI thread and which your API fills
> in on the handler thread from the GmsCore callback.
> 
> We would block the initialization of the native side of WebView IIUC, though
> this sounds scary :/
> I'm not super familiar with the breakpad-enabling code (and I believe Toby
still
> needs to look over it) so I don't know whether we have a good alternative
(i.e.
> whether can defer the decision until later during startup).

yeah let's defer the decision here.

[paulmiller, 2017-01-12 22:13:26]

Description was changed from

==========
Prepare to call GMS APIs from WebView

- Switch setMetricsSettingListener from a listener to a 1-time query.
- Replace tryEnableGms with canUseGms. (tryEnableGms will actually be
removed in a follow-up, since overloading implementations elsewhere
must be removed first.)
- Fix AwTestBase, which was incorrectly creating AwBrowserContext on
the test thread. That was preventing us from using assertOnUiThread.
- Since AwBrowserContext now requires native code, create the Activity
before the AwBrowserContext in AwTestBase.
- Remove "static" from aw_metrics_switch.cc, for consistency.

BUG=676523
==========

to

==========
Prepare to call GMS APIs from WebView

- Switch setMetricsSettingListener from a listener to a 1-time query.
- Replace tryEnableGms with canUseGms. (tryEnableGms will actually be
  removed in a follow-up, since overloading implementations elsewhere
  must be removed first.)
- Fix the race condition between native AwMetricsServiceClient
  initialization and Java metrics setting query. The Java side will wait
  for native initialization before switching on metrics.
- Move the native AwMetricsServiceClient implementation to native/,
  replacing AwMetricsServiceSwitch, so it can signal the Java
  AwMetricsServiceClient when it's ready.
- Fix AwTestBase, which was incorrectly creating AwBrowserContext on the 
  test thread. That was preventing us from using assertOnUiThread.
- Create the Activity before the AwBrowserContext in AwTestBase,
  allowing native code to work earlier.
- Minor, miscellaneous cleanup.

BUG=676523
==========

[paulmiller, 2017-01-13 02:29:02]

Description was changed from

==========
Prepare to call GMS APIs from WebView

- Switch setMetricsSettingListener from a listener to a 1-time query.
- Replace tryEnableGms with canUseGms. (tryEnableGms will actually be
  removed in a follow-up, since overloading implementations elsewhere
  must be removed first.)
- Fix the race condition between native AwMetricsServiceClient
  initialization and Java metrics setting query. The Java side will wait
  for native initialization before switching on metrics.
- Move the native AwMetricsServiceClient implementation to native/,
  replacing AwMetricsServiceSwitch, so it can signal the Java
  AwMetricsServiceClient when it's ready.
- Fix AwTestBase, which was incorrectly creating AwBrowserContext on the 
  test thread. That was preventing us from using assertOnUiThread.
- Create the Activity before the AwBrowserContext in AwTestBase,
  allowing native code to work earlier.
- Minor, miscellaneous cleanup.

BUG=676523
==========

to

==========
Prepare to call GMS APIs from WebView

- Switch setMetricsSettingListener from a listener to a 1-time query.
- Replace tryEnableGms with canUseGms. (tryEnableGms will actually be
  removed in a follow-up, since overloading implementations elsewhere
  must be removed first.)
- Fix the race condition between native AwMetricsServiceClient
  initialization and Java metrics setting query. The Java side will wait
  for native initialization before switching on metrics.
- Move the native AwMetricsServiceClient implementation to native/,
  replacing AwMetricsServiceSwitch, so it can signal the Java
  AwMetricsServiceClient when it's ready.
- Fix AwTestBase, which was incorrectly creating AwBrowserContext on the 
  test thread. That was preventing us from using assertOnUiThread.
- ...which requires fixing AwStrictModeTest, which was going out of its 
  way to create AwBrowserContext on the UI thread, as runOnMainSync
  doesn't allow redundant invocations.
- Some miscellaneous comment & whitespace cleanup.

BUG=676523
==========

[sgurun-gerrit only, 2017-01-13 07:12:16]

a few comments for now. I will look throughly once you answer them.

https://codereview.chromium.org/2611883002/diff/120001/android_webview/java/s...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/2611883002/diff/120001/android_webview/java/s...
android_webview/java/src/org/chromium/android_webview/AwContents.java:805:
AwMetricsServiceClient.queryMetricsSetting(mContext);
that is not the right place to do. AwMetricsServiceClient is not per AwContent.
Why start from here?

https://codereview.chromium.org/2611883002/diff/120001/android_webview/java/s...
File
android_webview/java/src/org/chromium/android_webview/AwMetricsServiceClient.java
(right):

https://codereview.chromium.org/2611883002/diff/120001/android_webview/java/s...
android_webview/java/src/org/chromium/android_webview/AwMetricsServiceClient.java:24:
* 4) If enabled, inform the native AwMetricsServiceClient via
nativeSetMetricsEnabled.
I think because you start it from Java (AwContents), you need to deal with an
unnecessary extra state. Is there a reason starting the query from Native does
not work?

https://codereview.chromium.org/2611883002/diff/120001/android_webview/native...
File android_webview/native/aw_metrics_service_client_impl.cc (right):

https://codereview.chromium.org/2611883002/diff/120001/android_webview/native...
android_webview/native/aw_metrics_service_client_impl.cc:76: void
AwMetricsServiceClientImpl::Initialize(
I thought this would call to java to query?

[paulmiller, 2017-01-13 20:43:59]

PTAL at patch set 8. linux_android_rel_ng passed; linux_android_dbg_ng, failed,
but I ran that test locally, and it fails both with and without my change, so
ignoring that.

https://codereview.chromium.org/2611883002/diff/120001/android_webview/java/s...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/2611883002/diff/120001/android_webview/java/s...
android_webview/java/src/org/chromium/android_webview/AwContents.java:805:
AwMetricsServiceClient.queryMetricsSetting(mContext);
On 2017/01/13 07:12:15, sgurun wrote:
> that is not the right place to do. AwMetricsServiceClient is not per
AwContent.
> Why start from here?

I agree it doesn't really make sense to put this here, but where would be a
better place? The consent query (which will eventually be shared between metrics
and crash uploading) needs to happen once per startup, somewhere early on, on
the Java side.

[sgurun-gerrit only, 2017-01-14 00:24:35]

On 2017/01/13 20:43:59, paulmiller wrote:
> PTAL at patch set 8. linux_android_rel_ng passed; linux_android_dbg_ng,
failed,
> but I ran that test locally, and it fails both with and without my change, so
> ignoring that.
> 
>
https://codereview.chromium.org/2611883002/diff/120001/android_webview/java/s...
> File android_webview/java/src/org/chromium/android_webview/AwContents.java
> (right):
> 
>
https://codereview.chromium.org/2611883002/diff/120001/android_webview/java/s...
> android_webview/java/src/org/chromium/android_webview/AwContents.java:805:
> AwMetricsServiceClient.queryMetricsSetting(mContext);
> On 2017/01/13 07:12:15, sgurun wrote:
> > that is not the right place to do. AwMetricsServiceClient is not per
> AwContent.
> > Why start from here?
> 
> I agree it doesn't really make sense to put this here, but where would be a
> better place? The consent query (which will eventually be shared between
metrics
> and crash uploading) needs to happen once per startup, somewhere early on, on
> the Java side.

lgtm

[paulmiller, 2017-01-14 00:48:25]

The CQ bit was checked by paulmiller@chromium.org

[commit-bot: I haz the power, 2017-01-14 00:49:12]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[paulmiller, 2017-01-14 02:05:58]

The CQ bit was checked by paulmiller@chromium.org

[paulmiller, 2017-01-14 02:05:58]

The patchset sent to the CQ was uploaded after l-g-t-m from sgurun@chromium.org
Link to the patchset: https://codereview.chromium.org/2611883002/#ps170001
(title: "explicit destructor for style checker")

[commit-bot: I haz the power, 2017-01-14 02:06:16]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-14 03:03:23]

CQ is committing da patch.

Bot data: {"patchset_id": 170001, "attempt_start_ts": 1484359558109550,
"parent_rev": "529286631559b7ca25924dca925fdcc62aa6d9bd", "commit_rev":
"85990128f90e66efb2d1a83caeca578ab93c275e"}

[commit-bot: I haz the power, 2017-01-14 03:03:50]

Description was changed from

==========
Prepare to call GMS APIs from WebView

- Switch setMetricsSettingListener from a listener to a 1-time query.
- Replace tryEnableGms with canUseGms. (tryEnableGms will actually be
  removed in a follow-up, since overloading implementations elsewhere
  must be removed first.)
- Fix the race condition between native AwMetricsServiceClient
  initialization and Java metrics setting query. The Java side will wait
  for native initialization before switching on metrics.
- Move the native AwMetricsServiceClient implementation to native/,
  replacing AwMetricsServiceSwitch, so it can signal the Java
  AwMetricsServiceClient when it's ready.
- Fix AwTestBase, which was incorrectly creating AwBrowserContext on the 
  test thread. That was preventing us from using assertOnUiThread.
- ...which requires fixing AwStrictModeTest, which was going out of its 
  way to create AwBrowserContext on the UI thread, as runOnMainSync
  doesn't allow redundant invocations.
- Some miscellaneous comment & whitespace cleanup.

BUG=676523
==========

to

==========
Prepare to call GMS APIs from WebView

- Switch setMetricsSettingListener from a listener to a 1-time query.
- Replace tryEnableGms with canUseGms. (tryEnableGms will actually be
  removed in a follow-up, since overloading implementations elsewhere
  must be removed first.)
- Fix the race condition between native AwMetricsServiceClient
  initialization and Java metrics setting query. The Java side will wait
  for native initialization before switching on metrics.
- Move the native AwMetricsServiceClient implementation to native/,
  replacing AwMetricsServiceSwitch, so it can signal the Java
  AwMetricsServiceClient when it's ready.
- Fix AwTestBase, which was incorrectly creating AwBrowserContext on the 
  test thread. That was preventing us from using assertOnUiThread.
- ...which requires fixing AwStrictModeTest, which was going out of its 
  way to create AwBrowserContext on the UI thread, as runOnMainSync
  doesn't allow redundant invocations.
- Some miscellaneous comment & whitespace cleanup.

BUG=676523

Review-Url: https://codereview.chromium.org/2611883002
Cr-Commit-Position: refs/heads/master@{#443776}
Committed:
https://chromium.googlesource.com/chromium/src/+/85990128f90e66efb2d1a83caeca...
==========

[commit-bot: I haz the power, 2017-01-14 03:03:51]

Committed patchset #10 (id:170001) as
https://chromium.googlesource.com/chromium/src/+/85990128f90e66efb2d1a83caeca...