then I think said exception needs to be reverted, or fixed...

or webview has a bug somewhere about marking which thread is UI, hmm...

just disable the test short term

I guess the question is, can WebStorage be used on a non-UI thread before any webview is created.. I think if someone does that, we should just pick the main thread and the UI thread, and post everything to main thread. Torne, thoughts?

This definitely feels like a bug in WebView or the test, not something in my change as it would have failed if asserts worked on any WebView compatible OS

Current implementation marks whatever thread WebStorage is created on as the UI thread. But WebStorageAdapter lacks any thread hopping code (unlike say WebViewChromium). That's easy to fix. Anyone feeling up to it?

WebViewChromiumFactoryProvider doesn't have any thread checks/hops when interacting with chromium classes either.

> This definitely feels like a bug in WebView or the test, not something in my change as it would have failed if asserts worked on any WebView compatible OS

Yeah we can fix this.

Just to be sure, this doesn't require a fix in test code right?  The test is covering case where static method is called on non-UI thread, but then creates the webview on the UI thread at a later time.  This is to cover a case in 568602, which I assume is valid usage (didn't find in docs to the contrary). 

> Just to be sure, this doesn't require a fix in test code right? 

Right

Bo, I think you described it backward in comment 7. The onMainThread parameter has a confusing name - it means "initialise using the main thread as the UI thread, instead of the current thread", not "we are currently on the main thread" - if you are actually on the main thread then it doesn't matter whether it's true or false as the result is the same.

When you initialise WebView by calling any of the static methods (other than CookieManager which is special), it binds the UI thread to the main thread, *not* the current thread. The code then crashes because it immediately proceeds using the *current*, non-UI-thread, to call chromium functions that require that they're called on the UI thread.

So yeah, anywhere where the glue code calls something in chromium that requires that it be on the UI thread it needs to be wrapped in thread-hopping junk, and this isn't currently true in all cases. The test is indeed correct.

Related question about the test, though: does the way we run tests guarantee that each of these tests will run in a *new process*? You only get one shot at webview initialisation per-process, so if the test isn't forcing a new process to be created each time then it may be more than just this one test that's broken, and it's just that only the one that runs first gets to crash?

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cfdcdf1f07dffeeb6ba0b73cb519b7a4e94a4736

commit cfdcdf1f07dffeeb6ba0b73cb519b7a4e94a4736
Author: mvanouwerkerk <mvanouwerkerk@chromium.org>
Date: Mon Jul 25 15:35:14 2016

Disable testWebViewInitByWebStorage

BUG= 630709 

Review-Url: https://codereview.chromium.org/2177913002
Cr-Commit-Position: refs/heads/master@{#407481}

[modify] https://crrev.com/cfdcdf1f07dffeeb6ba0b73cb519b7a4e94a4736/android_webview/tools/system_webview_shell/layout_tests/src/org/chromium/webview_shell/test/WebViewThreadTest.java

I'll take this I guess

> Bo, I think you described it backward in comment 7.

Oops you are right. It's this stupid line that's impossible to read...

Looper looper = !onMainThread ? Looper.myLooper() : Looper.getMainLooper();

If you're going to be changing stuff near here anyway, I vote change "onMainThread" to "useMainThread" and flip the ternary around so the condition isn't negated. :)

Re comment 12, the tests are run using the test_runner.py script, which does start a new process for each test method.

> Related question about the test, though: does the way we run tests guarantee that each of these tests will run in a *new process*?

These are just plain old instrumentation tests it looks like. So yes, each test runs in a new process. But that's more or less by accident rather than intention.

This is not that trivial. I think I'm going to move all the RunQueue logic from WebViewChromium into WebViewChromiumFactoryProvider instead, so it can be shared.

So there is going to one global queue rather than one per webview, which I think is probably fine..

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fbfe162dd52f7901535ceb70edf719ef4a692046

commit fbfe162dd52f7901535ceb70edf719ef4a692046
Author: boliu <boliu@chromium.org>
Date: Tue Jul 26 17:56:39 2016

aw: Move thread hop code to ProviderFactory

So that the thread hopping code can be used from entry points other
than WebViewChromium.

Large CL, but mostly no-op. Moved these to ProivderFactory:
WebViewChromiumRunQueue
runBlockingFuture
runVoidTaskOnUiThreadBlocking
runOnUiThreadBlocking
addTask

Now FactoryProvider no longer needs to keep a list of WebViewChromiums
yet to be started. So remove all that logic along with startYourEngine,
and no longer need to drainQueue.

Subtle change in behavior is tasks for a single WebViewChromium used to
be grouped together and now are all appended to a single queue that's
drained at start up. This should not cause problems.

BUG= 630709 

Review-Url: https://codereview.chromium.org/2182683003
Cr-Commit-Position: refs/heads/master@{#407848}

[modify] https://crrev.com/fbfe162dd52f7901535ceb70edf719ef4a692046/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromium.java
[modify] https://crrev.com/fbfe162dd52f7901535ceb70edf719ef4a692046/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/85dc69cf31dbc427429f929b18867b0c88df27e0

commit 85dc69cf31dbc427429f929b18867b0c88df27e0
Author: boliu <boliu@chromium.org>
Date: Tue Jul 26 17:56:39 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/993fff8648f2701910b9efdbf1d35244fbf8ca73

commit 993fff8648f2701910b9efdbf1d35244fbf8ca73
Author: boliu <boliu@chromium.org>
Date: Thu Jul 28 17:58:27 2016

aw: Fix FactoryProvider threading

Access thread-unsafe chromium classes on the UI thread only

BUG= 630709 

Review-Url: https://codereview.chromium.org/2180423003
Cr-Commit-Position: refs/heads/master@{#408432}

[modify] https://crrev.com/993fff8648f2701910b9efdbf1d35244fbf8ca73/android_webview/glue/BUILD.gn
[modify] https://crrev.com/993fff8648f2701910b9efdbf1d35244fbf8ca73/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromium.java
[modify] https://crrev.com/993fff8648f2701910b9efdbf1d35244fbf8ca73/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java
[modify] https://crrev.com/993fff8648f2701910b9efdbf1d35244fbf8ca73/android_webview/tools/system_webview_shell/layout_tests/src/org/chromium/webview_shell/test/WebViewThreadTest.java

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/96ea7367196502bd93ec7142901d1bc68efc1a4c

commit 96ea7367196502bd93ec7142901d1bc68efc1a4c
Author: boliu <boliu@chromium.org>
Date: Thu Jul 28 17:58:27 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/058d33e7040e4e79696fd7e8c09da1e86a159d3a

commit 058d33e7040e4e79696fd7e8c09da1e86a159d3a
Author: Bo Liu <boliu@google.com>
Date: Thu Jul 28 20:12:38 2016

test is green now, but there's more stuff to fix..

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/755650c7085bb4c07bfa24112030340e2400727e

commit 755650c7085bb4c07bfa24112030340e2400727e
Author: boliu <boliu@chromium.org>
Date: Fri Jul 29 02:12:47 2016

Revert of aw: Fix FactoryProvider threading (patchset #3 id:40001 of https://codereview.chromium.org/2180423003/ )

Reason for revert:
Broke correct usage:  crbug.com/632547 

BUG= 632547 

Original issue's description:
> aw: Fix FactoryProvider threading
>
> Access thread-unsafe chromium classes on the UI thread only
>
> BUG= 630709 
>
> Committed: https://crrev.com/993fff8648f2701910b9efdbf1d35244fbf8ca73
> Cr-Commit-Position: refs/heads/master@{#408432}

TBR=torne@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 630709 

Review-Url: https://codereview.chromium.org/2197433002
Cr-Commit-Position: refs/heads/master@{#408557}

[modify] https://crrev.com/755650c7085bb4c07bfa24112030340e2400727e/android_webview/glue/BUILD.gn
[modify] https://crrev.com/755650c7085bb4c07bfa24112030340e2400727e/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromium.java
[modify] https://crrev.com/755650c7085bb4c07bfa24112030340e2400727e/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java
[modify] https://crrev.com/755650c7085bb4c07bfa24112030340e2400727e/android_webview/tools/system_webview_shell/layout_tests/src/org/chromium/webview_shell/test/WebViewThreadTest.java

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/52c894ae9f35755a0a3fc682bcb4a852e436d3f3

commit 52c894ae9f35755a0a3fc682bcb4a852e436d3f3
Author: boliu <boliu@chromium.org>
Date: Fri Jul 29 02:12:47 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/52c894ae9f35755a0a3fc682bcb4a852e436d3f3

commit 52c894ae9f35755a0a3fc682bcb4a852e436d3f3
Author: boliu <boliu@chromium.org>
Date: Fri Jul 29 02:12:47 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/502c6b772c56e26a420576be518f9664027877a0

commit 502c6b772c56e26a420576be518f9664027877a0
Author: Bo Liu <boliu@google.com>
Date: Fri Jul 29 16:22:26 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/998e64e0d6d65718f3bc5a7dec2ecba8037be09e

commit 998e64e0d6d65718f3bc5a7dec2ecba8037be09e
Author: Bo Liu <boliu@google.com>
Date: Fri Jul 29 16:22:54 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cd9720621937c9779b54cf8256165dda2385b6f3

commit cd9720621937c9779b54cf8256165dda2385b6f3
Author: boliu <boliu@chromium.org>
Date: Fri Jul 29 16:28:46 2016

aw: Fix WebViewDatabaseAdapter threading

Hope to UI thread where needed.

BUG= 630709 

Review-Url: https://codereview.chromium.org/2191053002
Cr-Commit-Position: refs/heads/master@{#408653}

[modify] https://crrev.com/cd9720621937c9779b54cf8256165dda2385b6f3/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java
[modify] https://crrev.com/cd9720621937c9779b54cf8256165dda2385b6f3/android_webview/glue/java/src/com/android/webview/chromium/WebViewDatabaseAdapter.java

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/955a06f1f17b0a58122d50581162b3e545250868

commit 955a06f1f17b0a58122d50581162b3e545250868
Author: boliu <boliu@chromium.org>
Date: Fri Jul 29 16:28:46 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bab9ae4316310a8377a5132be71142ff7e065d86

commit bab9ae4316310a8377a5132be71142ff7e065d86
Author: boliu <boliu@chromium.org>
Date: Mon Aug 01 16:42:38 2016

aw: Add thread hops to more Adapter classes

BUG= 630709 

Review-Url: https://codereview.chromium.org/2201593002
Cr-Commit-Position: refs/heads/master@{#408979}

[modify] https://crrev.com/bab9ae4316310a8377a5132be71142ff7e065d86/android_webview/glue/java/src/com/android/webview/chromium/GeolocationPermissionsAdapter.java
[modify] https://crrev.com/bab9ae4316310a8377a5132be71142ff7e065d86/android_webview/glue/java/src/com/android/webview/chromium/WebStorageAdapter.java
[modify] https://crrev.com/bab9ae4316310a8377a5132be71142ff7e065d86/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/3cf7898fa4b5a20614f72169ff8660937e497d7f

commit 3cf7898fa4b5a20614f72169ff8660937e497d7f
Author: boliu <boliu@chromium.org>
Date: Mon Aug 01 16:42:38 2016