New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 788186 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug

Blocking:
issue 788138



Sign in to add a comment

How to deal with the "WebView UI thread" from the WebView support library

Project Member Reported by gsennton@chromium.org, Nov 23 2017

Issue description

WebView has its own way of defining its "UI thread" - that seems to be the thread from which we call the WebView ctor:

http://cs/android/frameworks/base/core/java/android/webkit/WebView.java?rcl=78252a23d6d38e4b1c938fef3d7b1dc6b7dfbe05&l=2552

private final Looper mWebViewThread = Looper.myLooper();

WebView also has a private method checkThread(), which is used to ensure that WebView methods are being called on the WebView UI thread.

In the WebView Support Library we should probably also ensure our method calls are being called on the correct thread. But to do so we need to access either the private mWebViewThread variable, or the private checkThread method from the support library. So we might want to turn one of those into a System-API, and then use reflection to get that variable/method on earlier Android versions.

Torne/Bo, IIRC you two are usually involved in discussions on calling WebView from the correct thread, WDYT?

Note: it looks like we have two checks here - one in WebView (which will cause a strictmodeviolation when an app uses the wrong thread), and one in WebViewChromium (which just posts the method call to the UI thread instead), so maybe the WebView check is not that important?
 

Comment 1 by boliu@chromium.org, Nov 27 2017

I wrote a doc about this: https://docs.google.com/document/d/1sDMN-K4TPh8osNBt-4ANfhf9FrRWl3xxrGRp7O-hkB8/edit?usp=sharing

mWebViewThread isn't necessarily the chromium UI thread. It's super confusing..
So the WebView.checkThread() call just ensures the Android-requirement of always calling a single View on the same thread is upheld. Technically I guess this isn't required from the support library since we are not making calls on a View, but it would seem more consistent to run the thread-check in the support library as well.. WDYT? 

Comment 3 by boliu@chromium.org, Jan 2 2018

consistency sgtm

Comment 4 by torne@chromium.org, Jan 2 2018

Yeah, all the things in the support library which are equivalent to the non-static calls on WebView objects should be thread-checked in the same way. Static calls are generally expected to be threadsafe.
Update here: we're gonna make WebView.checkThread() public so that it can be used from the support library, and we'll use it for the same methods as WebView.
Labels: -M-65 M-66
Cc: tobiasjs@chromium.org
Toby pointed out that it would be nice to clean up our differing views of 'main threads' in the framework vs. the webview apk, and share as many of the thread checks as possible between the support library glue and the old glue.

Right now we are leaning towards publishing WebView.getLooper() as an API to get the thread on which WebView APIs should be called, and then implement WebViewCompat.checkThread() using that Looper from the support library.
This would mean the support library wouldn't trigger the strictmode violation for calling webview on the wrong thread, but that's probably no biggie as any apps targeting post-ICS will crash when calling webview on the wrong thread..
Status: Fixed (was: Assigned)
Marking this as fixed as we are adding WebViewCompat.checkThread() in
ag/3614605

Comment 10 Deleted

Sign in to add a comment