Prevent speculative RWHVAs from handling input meant for existing views

Currently, when we create a RenderWidgetHostViewAndroid, we attach
its corresponding native view to the hierarchy. However, RWHVAs
may be created for a speculative RenderFrameHost for a cross-site
navigation, and would not replace the old RWHVA until the navigation
commits. This gives us two RWHVAs in the hierarchy. In this state,
input events are targeted to the speculative RWHVA.

Moreover, if the cross-site navigation is canceled before it commits,
then we stay in this state where input events are sent to the unused
RWHVA, causing the page to be seemingly unresponsive to input.

We now move new RWHVAs behind any existing ones in the view hierarchy.

Bug: 867932
Change-Id: I845d99ecbde7f721f6fa4b6e1b0243abb89a97fc
Reviewed-on: https://chromium-review.googlesource.com/1165482Reviewed-by: Bo <boliu@chromium.org>
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582206}

content/browser/frame_host/interstitial_page_impl.cc

@@ -528,6 +528,7 @@ void InterstitialPageImpl::DidNavigate(
   if (!controller_->delegate()->IsHidden())
     render_view_host_->GetWidget()->GetView()->Show();
   controller_->delegate()->AttachInterstitialPage(this);
+  render_view_host_->GetWidget()->GetView()->OnInterstitialPageAttached();
 
   RenderWidgetHostView* rwh_view =
       controller_->delegate()->GetRenderViewHost()->GetWidget()->GetView();

content/browser/renderer_host/render_widget_host_view_android.cc

@@ -216,7 +216,14 @@ RenderWidgetHostViewAndroid::RenderWidgetHostViewAndroid(
   host()->SetView(this);
   touch_selection_controller_client_manager_ =
       std::make_unique<TouchSelectionControllerClientManagerAndroid>(this);
+
   UpdateNativeViewTree(parent_native_view);
+  // This RWHVA may have been created speculatively. We should give any
+  // existing RWHVAs priority for receiving input events, otherwise a
+  // speculative RWHVA could be sent input events intended for the currently
+  // showing RWHVA.
+  if (parent_native_view)
+    parent_native_view->MoveToBack(&view_);
 
   CreateOverscrollControllerIfPossible();
 
@@ -818,6 +825,8 @@ void RenderWidgetHostViewAndroid::ResetGestureDetection() {
 }
 
 void RenderWidgetHostViewAndroid::OnDidNavigateMainFrameToNewPage() {
+  if (view_.parent())
+    view_.parent()->MoveToFront(&view_);
   ResetGestureDetection();
 }
 
@@ -913,6 +922,11 @@ uint32_t RenderWidgetHostViewAndroid::GetCaptureSequenceNumber() const {
   return latest_capture_sequence_number_;
 }
 
+void RenderWidgetHostViewAndroid::OnInterstitialPageAttached() {
+  if (view_.parent())
+    view_.parent()->MoveToFront(&view_);
+}
+
 void RenderWidgetHostViewAndroid::OnInterstitialPageGoingAway() {
   sync_compositor_.reset();
 }

content/browser/renderer_host/render_widget_host_view_android.h

@@ -160,6 +160,7 @@ class CONTENT_EXPORT RenderWidgetHostViewAndroid
   bool IsInVR() const override;
   void DidOverscroll(const ui::DidOverscrollParams& params) override;
   void DidStopFlinging() override;
+  void OnInterstitialPageAttached() override;
   void OnInterstitialPageGoingAway() override;
   std::unique_ptr<SyntheticGestureTarget> CreateSyntheticGestureTarget()
       override;

content/browser/renderer_host/render_widget_host_view_base.h

@@ -413,6 +413,9 @@ class CONTENT_EXPORT RenderWidgetHostViewBase
   // Returns true if this view's size have been initialized.
   virtual bool HasSize() const;
 
+  // Informs the view that the assocaited InterstitialPage was attached.
+  virtual void OnInterstitialPageAttached() {}
+
   // Tells the view that the assocaited InterstitialPage will going away (but is
   // not yet destroyed, as InterstitialPage destruction is asynchronous). The
   // view may use this notification to clean up associated resources. This

content/browser/site_per_process_browsertest.cc

@@ -136,8 +136,12 @@
 #include "content/browser/android/ime_adapter_android.h"
 #include "content/browser/renderer_host/input/touch_selection_controller_client_manager_android.h"
 #include "content/browser/renderer_host/render_widget_host_view_android.h"
+#include "content/browser/web_contents/web_contents_view_android.h"
 #include "content/public/browser/android/child_process_importance.h"
 #include "content/test/mock_overscroll_refresh_handler_android.h"
+#include "ui/android/view_android.h"
+#include "ui/android/window_android.h"
+#include "ui/events/android/event_handler_android.h"
 #include "ui/events/android/motion_event_android.h"
 #include "ui/gfx/geometry/point_f.h"
 #endif
@@ -10132,6 +10136,76 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
 }
 
 #if defined(OS_ANDROID)
+
+namespace {
+
+class MockEventHandlerAndroid : public ui::EventHandlerAndroid {
+ public:
+  bool OnTouchEvent(const ui::MotionEventAndroid& event) override {
+    did_receive_event_ = true;
+    return true;
+  }
+
+  bool did_receive_event() { return did_receive_event_; }
+
+ private:
+  bool did_receive_event_ = false;
+};
+
+}  // namespace
+
+IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
+                       SpeculativeRenderFrameHostDoesNotReceiveInput) {
+  GURL url1(embedded_test_server()->GetURL("a.com", "/title1.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), url1));
+
+  RenderWidgetHostViewAndroid* rwhva =
+      static_cast<RenderWidgetHostViewAndroid*>(
+          shell()->web_contents()->GetRenderWidgetHostView());
+  ui::ViewAndroid* rwhva_native_view = rwhva->GetNativeView();
+  FrameTreeNode* root = web_contents()->GetFrameTree()->root();
+
+  // Start a cross-site navigation.
+  GURL url2(embedded_test_server()->GetURL("b.com", "/title2.html"));
+  TestNavigationManager nav_manager(web_contents(), url2);
+  shell()->LoadURL(url2);
+
+  // Wait for the request, but don't commit it yet. This should create a
+  // speculative RenderFrameHost.
+  ASSERT_TRUE(nav_manager.WaitForRequestStart());
+  RenderFrameHostImpl* root_speculative_rfh =
+      root->render_manager()->speculative_frame_host();
+  EXPECT_TRUE(root_speculative_rfh);
+  RenderWidgetHostViewAndroid* rwhv_speculative =
+      static_cast<RenderWidgetHostViewAndroid*>(
+          root_speculative_rfh->GetView());
+  ui::ViewAndroid* rwhv_speculative_native_view =
+      rwhv_speculative->GetNativeView();
+
+  ui::ViewAndroid* root_view = web_contents()->GetView()->GetNativeView();
+  EXPECT_TRUE(root_view);
+
+  MockEventHandlerAndroid mock_handler;
+  rwhva_native_view->set_event_handler(&mock_handler);
+  MockEventHandlerAndroid mock_handler_speculative;
+  rwhv_speculative_native_view->set_event_handler(&mock_handler_speculative);
+  // Avoid having the root try to handle the following event.
+  root_view->set_event_handler(nullptr);
+
+  auto size = root_view->GetSize();
+  float x = size.width() / 2;
+  float y = size.height() / 2;
+  ui::MotionEventAndroid::Pointer pointer0(0, x, y, 0, 0, 0, 0, 0);
+  ui::MotionEventAndroid::Pointer pointer1(0, 0, 0, 0, 0, 0, 0, 0);
+  ui::MotionEventAndroid event(nullptr, nullptr, 1.f / root_view->GetDipScale(),
+                               0.f, 0.f, 0.f, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0,
+                               false, &pointer0, &pointer1);
+  root_view->OnTouchEventForTesting(event);
+
+  EXPECT_TRUE(mock_handler.did_receive_event());
+  EXPECT_FALSE(mock_handler_speculative.did_receive_event());
+}
+
 IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, TestChildProcessImportance) {
   web_contents()->SetMainFrameImportance(ChildProcessImportance::MODERATE);
 

ui/android/view_android.cc

@@ -182,6 +182,16 @@ void ViewAndroid::MoveToFront(ViewAndroid* child) {
     children_.splice(children_.end(), children_, it);
 }
 
+void ViewAndroid::MoveToBack(ViewAndroid* child) {
+  DCHECK(child);
+  auto it = std::find(children_.begin(), children_.end(), child);
+  DCHECK(it != children_.end());
+
+  // Bottom element is placed at the beginning of the list.
+  if (*it != children_.front())
+    children_.splice(children_.begin(), children_, it);
+}
+
 void ViewAndroid::RemoveFromParent() {
   if (parent_)
     parent_->RemoveChild(this);

ui/android/view_android.h

@@ -126,6 +126,9 @@ class UI_ANDROID_EXPORT ViewAndroid {
   // Moves the give child ViewAndroid to the front of the list so that it can be
   // the first responder of events.
   void MoveToFront(ViewAndroid* child);
+  // Moves the given child ViewAndroid to the back of the list so that any other
+  // view may respond to events first.
+  void MoveToBack(ViewAndroid* child);
 
   // Detaches this view from its parent.
   void RemoveFromParent();
@@ -178,6 +181,10 @@ class UI_ANDROID_EXPORT ViewAndroid {
 
   ViewAndroid* parent() const { return parent_; }
 
+  bool OnTouchEventForTesting(const MotionEventAndroid& event) {
+    return OnTouchEvent(event);
+  }
+
  protected:
   void RemoveAllChildren(bool attached_to_window);
 

ui/android/view_android_unittests.cc

@@ -118,6 +118,11 @@ TEST_F(ViewAndroidBoundsTest, MatchesViewInFront) {
   root_.MoveToFront(&view2_);
   GenerateTouchEventAt(100.f, 100.f);
   ExpectHit(handler2_);
+
+  // View 2 moves back to the bottom, and events should hit View 1 again.
+  root_.MoveToBack(&view2_);
+  GenerateTouchEventAt(100.f, 100.f);
+  ExpectHit(handler1_);
 }
 
 TEST_F(ViewAndroidBoundsTest, MatchesViewArea) {