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

Issue 616955 link

Starred by 1 user
Status: Fixed
Owner:
█ hush@chromium.org
inactive
Closed: Jun 2016
Cc:
boliu@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Android WebView multiprocess: Inputs go to the wrong compositor under certain circumstances.

Project Member Reported by hush@chromium.org, Jun 2 2016 
Inputs can go to a wrong compositor when a new RVH (that contains the new compositor) is constructed but never swapped to be the current RVH.
This could happen when the new domain that the webview tries to navigate to has an SSL error.

Steps to reproduce:
1. enable multi process in Developer Options.
2. go to https://www.google.com
3. go to https://www.google.co --- this page has an SSL error.
4. observe that what ever action you do on https://www.google.co is fed into the compositor that holds https://www.google.com. 

The reason is because we currently hook the "compositor change" signal to RWHVA::SetContentViewCore, which is called in RWHVA constructor. Unfortunately, a RWHVA does not always end up being swapped to be current, as is seen in our SSL error case. 

The proposal is to make android_webview::BrowserViewRenderer a contents::WebContentsObserver that listens to RenderViewHostChanged. In the mean time, we don't want to make any timing relationships between RenderViewHostChanged and the new compositor being constructed.
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 17 2016 

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

commit 284add6d41ef58f8b2b2574767e6b7e03ebb0923
Author: hush <hush@chromium.org>
Date: Fri Jun 17 20:47:33 2016

Rewire Android WebView's compositor changed signal.

Make android_webview::BrowserViewRenderer a contents::WebContentsObserver that
listens to RenderViewHostChanged.

WebView used to rely on RWHVA::SetContentViewCore call to swap compositors,
which assumed any RWHVA that is constructed will soon be current. This turned out
to be false when WebView navigates to a page that has an SSL error (in which
case a new RVH will be created for the new domain, but the new RVH won't be
swapped).

To be able to map from RVH to the compositor contained in it, we've introduced a
CompositorID type, that combines the process_id and the routing_id of the RVH.

BUG= 616955 

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

[modify] https://crrev.com/284add6d41ef58f8b2b2574767e6b7e03ebb0923/android_webview/BUILD.gn
[modify] https://crrev.com/284add6d41ef58f8b2b2574767e6b7e03ebb0923/android_webview/android_webview.gyp
[modify] https://crrev.com/284add6d41ef58f8b2b2574767e6b7e03ebb0923/android_webview/browser/browser_view_renderer.cc
[modify] https://crrev.com/284add6d41ef58f8b2b2574767e6b7e03ebb0923/android_webview/browser/browser_view_renderer.h
[modify] https://crrev.com/284add6d41ef58f8b2b2574767e6b7e03ebb0923/android_webview/browser/child_frame.cc
[modify] https://crrev.com/284add6d41ef58f8b2b2574767e6b7e03ebb0923/android_webview/browser/child_frame.h
[modify] https://crrev.com/284add6d41ef58f8b2b2574767e6b7e03ebb0923/android_webview/browser/compositor_frame_consumer.h
[add] https://crrev.com/284add6d41ef58f8b2b2574767e6b7e03ebb0923/android_webview/browser/compositor_id.cc
[add] https://crrev.com/284add6d41ef58f8b2b2574767e6b7e03ebb0923/android_webview/browser/compositor_id.h
[modify] https://crrev.com/284add6d41ef58f8b2b2574767e6b7e03ebb0923/android_webview/browser/hardware_renderer.cc
[modify] https://crrev.com/284add6d41ef58f8b2b2574767e6b7e03ebb0923/android_webview/browser/hardware_renderer.h
[modify] https://crrev.com/284add6d41ef58f8b2b2574767e6b7e03ebb0923/android_webview/browser/render_thread_manager.cc
[modify] https://crrev.com/284add6d41ef58f8b2b2574767e6b7e03ebb0923/android_webview/browser/render_thread_manager.h
[modify] https://crrev.com/284add6d41ef58f8b2b2574767e6b7e03ebb0923/android_webview/browser/test/rendering_test.cc
[modify] https://crrev.com/284add6d41ef58f8b2b2574767e6b7e03ebb0923/android_webview/native/aw_contents.cc
[modify] https://crrev.com/284add6d41ef58f8b2b2574767e6b7e03ebb0923/android_webview/native/aw_contents.h
[modify] https://crrev.com/284add6d41ef58f8b2b2574767e6b7e03ebb0923/content/browser/android/synchronous_compositor_host.cc
[modify] https://crrev.com/284add6d41ef58f8b2b2574767e6b7e03ebb0923/content/browser/android/synchronous_compositor_host.h
[modify] https://crrev.com/284add6d41ef58f8b2b2574767e6b7e03ebb0923/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/284add6d41ef58f8b2b2574767e6b7e03ebb0923/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/284add6d41ef58f8b2b2574767e6b7e03ebb0923/content/public/browser/android/synchronous_compositor_client.h
[modify] https://crrev.com/284add6d41ef58f8b2b2574767e6b7e03ebb0923/content/public/test/test_synchronous_compositor_android.cc

Comment 2 by satyavat...@chromium.org, Jun 21 2016 

Thankyou, hush@

Verified this fix on latest N with latest Webview on Nexus 5X.

Steps:
1. Go to nytimes.com 
2. Then go to https://www.google.co in the address bar
3. Verify that the page still stays at nytimes.com and we can still interact with the page, like clicking on the links etc

Comment 4 by hush@chromium.org, Jun 24 2016

Status: Fixed (was: Assigned)
►

Sign in to add a comment