How to deal with posting WebViewProvider tasks to the UI thread in the support library glue? |
|||
Issue description
Many methods in WebViewChromium uses one of the following mechanisms:
if (checkNeedsPost()) {
mFactory.addTask(new Runnable() {
...
or
if (checkNeedsPost()) {
mFactory.runOnUiThreadBlocking(new Callable<String[]>() {
I'm assuming we should do the same thing in the new glue layer. The question is, should we re-use the existing mechanism for doing so, i.e. WebViewChromiumRunQueue in WebViewChromiumFactoryProvider? If so, maybe we should move that mechanism into a (new) lower layer that can be used by both the old and the new glue layer.
Bo/Torne do you have any thoughts here - do you think we should be using the same queue for scheduling tasks from android.webkit as from the support library?
I think it makes sense to have them on the same queue to preserve call orders between e.g. WebView and WebViewCompat - not preserving that order seems unexpected.
Note: whatever we choose, we should be able to go ahead with the implementation for this without having implemented the rest of the glue layer.
Leaving this as Available for now - we want it for 65, but I don't know when I'll have time to get to it :)
,
Nov 20 2017
So we need to provide the WebViewChromiumRunQueue API to the new glue layer, and also share the instance of WebViewChromiumRunQueue used by WebViewFactoryProvider with the new glue layer objects. I would suggest moving WebViewChromiumRunQueue out of WebViewFactoryProvider into a lower layer (possibly a completely new layer), and then share the WebViewChromiumRunQueue instance used by WebViewFactoryProvider with the new glue layer through some kind of shared object (e.g. named AwSharedState). Nate, WDYT?
,
Nov 20 2017
> I'm assuming we should do the same thing in the new glue layer. It sounds like you were considering alternatives, but I don't actually see any. So what alternatives were you considering? Just a separate queue instead?
,
Nov 21 2017
Yeah, that would be the only alternative I can think of. It sounds like sharing queues makes more sense.
,
Nov 27 2017
,
Dec 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a9e3445a05849125d9a68eb9ebc4db845a040df0 commit a9e3445a05849125d9a68eb9ebc4db845a040df0 Author: Gustav Sennton <gsennton@google.com> Date: Mon Dec 11 19:43:57 2017 Move WebViewChromiumRunQueue into its own class out of the glue layer. The WebView Support Library glue layer should post tasks to the WebViewChromiumRunQueue owned by WebViewChromiumFactoryProvider, so that new glue layer needs access to the WebViewChromiumRunQueue class definition. Therefore we move that definition out of the existing glue layer. BUG= 785928 Change-Id: I5b4e41baf6bef11d8dd6762372e5a764732ad96f Reviewed-on: https://chromium-review.googlesource.com/819638 Commit-Queue: Gustav Sennton <gsennton@chromium.org> Reviewed-by: Bo <boliu@chromium.org> Cr-Commit-Position: refs/heads/master@{#523175} [modify] https://crrev.com/a9e3445a05849125d9a68eb9ebc4db845a040df0/android_webview/BUILD.gn [modify] https://crrev.com/a9e3445a05849125d9a68eb9ebc4db845a040df0/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java [add] https://crrev.com/a9e3445a05849125d9a68eb9ebc4db845a040df0/android_webview/java/src/org/chromium/android_webview/WebViewChromiumRunQueue.java
,
Dec 12 2017
|
|||
►
Sign in to add a comment |
|||
Comment 1 by ntfschr@chromium.org
, Nov 16 2017