Get rid of Pull based hit testing for Android WebView.

Android WebView always requested hit testing for ACTION_DOWN but this
is incompatible with OOPIF. The hit test would always be sent to the
main frame but it needs to be routed to the appropriate LocalRoot.

This mitigates the bug by moving to a push based approach where the
renderer will push the last hit test result from an ACTION_DOWN
event (which corresponds to a TouchStart).

BUG=1338006

Change-Id: I9012672cccc945822cb6bd35ff3ba78aac91a965
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3715455
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Reviewed-by: Bo Liu <boliu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1018302}

android_webview/browser/aw_contents.cc

@@ -915,36 +915,35 @@ void AwContents::RequestNewHitTestDataAt(JNIEnv* env,
 
 void AwContents::UpdateLastHitTestData(JNIEnv* env) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  if (!render_view_host_ext_->HasNewHitTestData())
-    return;
 
   ScopedJavaLocalRef<jobject> obj = java_ref_.get(env);
   if (!obj)
     return;
 
-  const android_webview::mojom::HitTestData& data =
-      render_view_host_ext_->GetLastHitTestData();
-  render_view_host_ext_->MarkHitTestDataRead();
+  android_webview::mojom::HitTestDataPtr data =
+      render_view_host_ext_->TakeLastHitTestData();
+  if (!data)
+    return;
 
   // Make sure to null the Java object if data is empty/invalid.
   ScopedJavaLocalRef<jstring> extra_data_for_type;
-  if (data.extra_data_for_type.length())
+  if (data->extra_data_for_type.length())
     extra_data_for_type =
-        ConvertUTF8ToJavaString(env, data.extra_data_for_type);
+        ConvertUTF8ToJavaString(env, data->extra_data_for_type);
 
   ScopedJavaLocalRef<jstring> href;
-  if (data.href.length())
-    href = ConvertUTF16ToJavaString(env, data.href);
+  if (data->href.length())
+    href = ConvertUTF16ToJavaString(env, data->href);
 
   ScopedJavaLocalRef<jstring> anchor_text;
-  if (data.anchor_text.length())
-    anchor_text = ConvertUTF16ToJavaString(env, data.anchor_text);
+  if (data->anchor_text.length())
+    anchor_text = ConvertUTF16ToJavaString(env, data->anchor_text);
 
   ScopedJavaLocalRef<jstring> img_src;
-  if (data.img_src.is_valid())
-    img_src = ConvertUTF8ToJavaString(env, data.img_src.spec());
+  if (data->img_src.is_valid())
+    img_src = ConvertUTF8ToJavaString(env, data->img_src.spec());
 
-  Java_AwContents_updateHitTestData(env, obj, static_cast<jint>(data.type),
+  Java_AwContents_updateHitTestData(env, obj, static_cast<jint>(data->type),
                                     extra_data_for_type, href, anchor_text,
                                     img_src);
 }

android_webview/browser/renderer_host/aw_render_view_host_ext.cc

@@ -7,6 +7,7 @@
 #include "android_webview/browser/aw_browser_context.h"
 #include "android_webview/browser/aw_contents.h"
 #include "android_webview/browser/aw_contents_client_bridge.h"
+#include "android_webview/common/aw_features.h"
 #include "base/android/scoped_java_ref.h"
 #include "base/bind.h"
 #include "base/callback.h"
@@ -43,7 +44,6 @@ AwRenderViewHostExt::AwRenderViewHostExt(AwRenderViewHostExtClient* client,
                                          content::WebContents* contents)
     : content::WebContentsObserver(contents),
       client_(client),
-      has_new_hit_test_data_(false),
       frame_host_receivers_(contents, this) {
   DCHECK(client_);
 }
@@ -68,28 +68,33 @@ void AwRenderViewHostExt::DocumentHasImages(DocumentHasImagesResult result) {
   }
 }
 
-bool AwRenderViewHostExt::HasNewHitTestData() const {
-  return has_new_hit_test_data_;
-}
-
-void AwRenderViewHostExt::MarkHitTestDataRead() {
-  has_new_hit_test_data_ = false;
-}
-
 void AwRenderViewHostExt::RequestNewHitTestDataAt(
     const gfx::PointF& touch_center,
     const gfx::SizeF& touch_area) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  // If the new hit testing approach for touch start is enabled we just early
+  // return.
+  if (base::FeatureList::IsEnabled(
+          features::kWebViewHitTestInBlinkOnTouchStart)) {
+    return;
+  }
+
+  // The following code is broken for OOPIF and fenced frames. The hit testing
+  // feature for touch start replaces this code and works correctly in those
+  // scenarios. For mitigating risk we've put the old code behind a feature
+  // flag.
+  //
   // We only need to get blink::WebView on the renderer side to invoke the
   // blink hit test Mojo method, so sending this message via LocalMainFrame
   // interface is enough.
   if (auto* local_main_frame_remote = GetLocalMainFrameRemote())
     local_main_frame_remote->HitTest(touch_center, touch_area);
 }
 
-const mojom::HitTestData& AwRenderViewHostExt::GetLastHitTestData() const {
+mojom::HitTestDataPtr AwRenderViewHostExt::TakeLastHitTestData() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  return *last_hit_test_data_;
+  return std::move(last_hit_test_data_);
 }
 
 void AwRenderViewHostExt::SetTextZoomFactor(float factor) {
@@ -145,19 +150,15 @@ void AwRenderViewHostExt::OnPageScaleFactorChanged(float page_scale_factor) {
 
 void AwRenderViewHostExt::UpdateHitTestData(
     mojom::HitTestDataPtr hit_test_data) {
-  content::RenderFrameHost* main_frame_host =
+  content::RenderFrameHost* render_frame_host =
       frame_host_receivers_.GetCurrentTargetFrame();
-  while (main_frame_host->GetParent())
-    main_frame_host = main_frame_host->GetParent();
-
-  // Make sense from any frame of the current frame tree, because a focused
+  // Make sense from any frame of the active frame tree, because a focused
   // node could be in either the mainframe or a subframe.
-  if (main_frame_host != web_contents()->GetPrimaryMainFrame())
+  if (!render_frame_host->IsActive())
     return;
 
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   last_hit_test_data_ = std::move(hit_test_data);
-  has_new_hit_test_data_ = true;
 }
 
 void AwRenderViewHostExt::ContentsSizeChanged(const gfx::Size& contents_size) {

android_webview/browser/renderer_host/aw_render_view_host_ext.h

@@ -62,14 +62,9 @@ class AwRenderViewHostExt : public content::WebContentsObserver,
   // independent pixels used by blink::WebView.
   void RequestNewHitTestDataAt(const gfx::PointF& touch_center,
                                const gfx::SizeF& touch_area);
-
-  // Optimization to avoid unnecessary Java object creation on hit test.
-  bool HasNewHitTestData() const;
-  void MarkHitTestDataRead();
-
   // Return |last_hit_test_data_|. Note that this is unavoidably racy;
   // the corresponding public WebView API is as well.
-  const mojom::HitTestData& GetLastHitTestData() const;
+  mojom::HitTestDataPtr TakeLastHitTestData();
 
   // Sets the zoom factor for text only. Used in layout modes other than
   // Text Autosizing.
@@ -112,8 +107,6 @@ class AwRenderViewHostExt : public content::WebContentsObserver,
   // is called in AwRenderViewExt.
   android_webview::mojom::HitTestDataPtr last_hit_test_data_;
 
-  bool has_new_hit_test_data_;
-
   // Some WebView users might want to show their own error pages / logic.
   bool will_suppress_error_page_ = false;
 

android_webview/common/aw_features.cc

@@ -46,6 +46,9 @@ const base::Feature kWebViewConnectionlessSafeBrowsing{
 const base::Feature kWebViewForceDarkModeMatchTheme{
     "WebViewForceDarkModeMatchTheme", base::FEATURE_DISABLED_BY_DEFAULT};
 
+const base::Feature kWebViewHitTestInBlinkOnTouchStart{
+    "WebViewHitTestInBlinkOnTouchStart", base::FEATURE_ENABLED_BY_DEFAULT};
+
 // Enable display cutout support for Android P and above.
 const base::Feature kWebViewDisplayCutout{"WebViewDisplayCutout",
                                           base::FEATURE_ENABLED_BY_DEFAULT};

android_webview/common/aw_features.h

@@ -27,6 +27,7 @@ extern const base::Feature kWebViewDisplayCutout;
 extern const base::Feature kWebViewEmptyComponentLoaderPolicy;
 extern const base::Feature kWebViewExtraHeadersSameOriginOnly;
 extern const base::Feature kWebViewForceDarkModeMatchTheme;
+extern const base::Feature kWebViewHitTestInBlinkOnTouchStart;
 extern const base::Feature kWebViewJavaJsBridgeMojo;
 extern const base::Feature kWebViewLegacyTlsSupport;
 extern const base::Feature kWebViewMeasureScreenCoverage;

android_webview/java/src/org/chromium/android_webview/common/ProductionSupportedFlagList.java

@@ -337,6 +337,8 @@ private ProductionSupportedFlagList() {}
             Flag.baseFeature(BlinkFeatures.TIMED_HTML_PARSER_BUDGET,
                     "If enabled, the HTMLDocumentParser will use a budget based on elapsed time"
                             + " rather than token count."),
+            Flag.baseFeature(AwFeatures.WEBVIEW_HIT_TEST_IN_BLINK_ON_TOUCH_START,
+                    "Hit test on touch start in blink"),
             // Add new commandline switches and features above. The final entry should have a
             // trailing comma for cleaner diffs.
     };

android_webview/renderer/aw_render_frame_ext.cc

@@ -7,6 +7,7 @@
 #include <map>
 #include <memory>
 
+#include "android_webview/common/aw_features.h"
 #include "android_webview/common/mojom/frame.mojom.h"
 #include "base/no_destructor.h"
 #include "base/strings/string_util.h"
@@ -216,6 +217,16 @@ bool AwRenderFrameExt::OnAssociatedInterfaceRequestForFrame(
   return registry_.TryBindInterface(interface_name, handle);
 }
 
+void AwRenderFrameExt::DidCreateDocumentElement() {
+  if (!base::FeatureList::IsEnabled(
+          features::kWebViewHitTestInBlinkOnTouchStart)) {
+    return;
+  }
+  render_frame()->GetWebFrame()->AddHitTestOnTouchStartCallback(
+      base::BindRepeating(&AwRenderFrameExt::HandleHitTestResult,
+                          base::Unretained(this)));
+}
+
 void AwRenderFrameExt::DidCommitProvisionalLoad(
     ui::PageTransition transition) {
   // Clear the cache when we cross site boundaries in the main frame.
@@ -268,6 +279,11 @@ void AwRenderFrameExt::HitTest(const gfx::PointF& touch_center,
   const blink::WebHitTestResult result = webview->HitTestResultForTap(
       gfx::Point(touch_center.x(), touch_center.y()),
       gfx::Size(touch_area.width(), touch_area.height()));
+  HandleHitTestResult(result);
+}
+
+void AwRenderFrameExt::HandleHitTestResult(
+    const blink::WebHitTestResult& result) {
   auto data = mojom::HitTestData::New();
 
   GURL absolute_image_url = result.AbsoluteImageURL();

android_webview/renderer/aw_render_frame_ext.h

@@ -19,6 +19,7 @@
 
 namespace blink {
 class WebFrameWidget;
+class WebHitTestResult;
 class WebView;
 }
 
@@ -47,6 +48,7 @@ class AwRenderFrameExt : public content::RenderFrameObserver,
   void DidCommitProvisionalLoad(ui::PageTransition transition) override;
 
   void FocusedElementChanged(const blink::WebElement& element) override;
+  void DidCreateDocumentElement() override;
   void OnDestruct() override;
 
   // mojom::LocalMainFrame overrides:
@@ -63,6 +65,8 @@ class AwRenderFrameExt : public content::RenderFrameObserver,
   void BindLocalMainFrame(
       mojo::PendingAssociatedReceiver<mojom::LocalMainFrame> pending_receiver);
 
+  void HandleHitTestResult(const blink::WebHitTestResult& result);
+
   const mojo::AssociatedRemote<mojom::FrameHost>& GetFrameHost();
 
   blink::WebView* GetWebView();

third_party/blink/public/web/web_local_frame.h

@@ -82,6 +82,7 @@ class WebContentSettingsClient;
 class WebLocalFrameClient;
 class WebFrameWidget;
 class WebHistoryItem;
+class WebHitTestResult;
 class WebInputMethodController;
 class WebPerformance;
 class WebPlugin;
@@ -903,6 +904,14 @@ class WebLocalFrame : public WebFrame {
       CrossVariantMojoRemote<mojom::StorageAreaInterfaceBase>
           session_storage_area) = 0;
 
+  // Android WebView requires notification of hit tests from blink. It requires
+  // hit tests on touchstart. So this method installs a passive event listener
+  // on touchstart and does a GestureTap hit test providing the results to the
+  // callback.
+  virtual void AddHitTestOnTouchStartCallback(
+      base::RepeatingCallback<void(const blink::WebHitTestResult&)>
+          callback) = 0;
+
  protected:
   explicit WebLocalFrame(mojom::TreeScopeType scope,
                          const LocalFrameToken& frame_token)

third_party/blink/renderer/core/frame/web_local_frame_impl.cc

@@ -152,6 +152,7 @@
 #include "third_party/blink/renderer/core/clipboard/system_clipboard.h"
 #include "third_party/blink/renderer/core/core_initializer.h"
 #include "third_party/blink/renderer/core/dom/document.h"
+#include "third_party/blink/renderer/core/dom/events/add_event_listener_options_resolved.h"
 #include "third_party/blink/renderer/core/dom/icon_url.h"
 #include "third_party/blink/renderer/core/dom/node.h"
 #include "third_party/blink/renderer/core/dom/node_traversal.h"
@@ -176,6 +177,7 @@
 #include "third_party/blink/renderer/core/editing/visible_position.h"
 #include "third_party/blink/renderer/core/events/after_print_event.h"
 #include "third_party/blink/renderer/core/events/before_print_event.h"
+#include "third_party/blink/renderer/core/events/touch_event.h"
 #include "third_party/blink/renderer/core/exported/web_dev_tools_agent_impl.h"
 #include "third_party/blink/renderer/core/exported/web_plugin_container_impl.h"
 #include "third_party/blink/renderer/core/exported/web_view_impl.h"
@@ -591,6 +593,64 @@ class PaintPreviewContext : public PrintContext {
   }
 };
 
+// Android WebView requires hit testing results on every touch event. This
+// pushes the hit test result to the callback that is registered.
+class TouchStartEventListener : public NativeEventListener {
+ public:
+  explicit TouchStartEventListener(
+      base::RepeatingCallback<void(const blink::WebHitTestResult&)> callback)
+      : callback_(std::move(callback)) {}
+
+  void Invoke(ExecutionContext*, Event* event) override {
+    auto* touch_event = DynamicTo<TouchEvent>(event);
+    if (!touch_event)
+      return;
+    const auto* native_event = touch_event->NativeEvent();
+    if (!native_event)
+      return;
+
+    DCHECK_EQ(WebInputEvent::Type::kTouchStart,
+              native_event->Event().GetType());
+    const auto& web_touch_event =
+        static_cast<const WebTouchEvent&>(native_event->Event());
+
+    if (web_touch_event.touches_length != 1u)
+      return;
+
+    LocalDOMWindow* dom_window = event->currentTarget()->ToLocalDOMWindow();
+    CHECK(dom_window);
+
+    WebGestureEvent tap_event(
+        WebInputEvent::Type::kGestureTap, WebInputEvent::kNoModifiers,
+        base::TimeTicks::Now(), WebGestureDevice::kTouchscreen);
+    // GestureTap is only ever from a touchscreen.
+    tap_event.SetPositionInWidget(
+        web_touch_event.touches[0].PositionInWidget());
+    tap_event.SetPositionInScreen(
+        web_touch_event.touches[0].PositionInScreen());
+    tap_event.SetFrameScale(web_touch_event.FrameScale());
+    tap_event.SetFrameTranslate(web_touch_event.FrameTranslate());
+    tap_event.data.tap.tap_count = 1;
+    tap_event.data.tap.height = tap_event.data.tap.width =
+        std::max(web_touch_event.touches[0].radius_x,
+                 web_touch_event.touches[0].radius_y);
+
+    HitTestResult result =
+        dom_window->GetFrame()
+            ->GetEventHandler()
+            .HitTestResultForGestureEvent(
+                tap_event, HitTestRequest::kReadOnly | HitTestRequest::kActive)
+            .GetHitTestResult();
+
+    result.SetToShadowHostIfInRestrictedShadowRoot();
+
+    callback_.Run(result);
+  }
+
+ private:
+  base::RepeatingCallback<void(const blink::WebHitTestResult&)> callback_;
+};
+
 // WebFrame -------------------------------------------------------------------
 
 static CreateWebFrameWidgetCallback* g_create_web_frame_widget = nullptr;
@@ -2871,6 +2931,18 @@ void WebLocalFrameImpl::SetSessionStorageArea(
       *GetFrame(), std::move(session_storage_area));
 }
 
+void WebLocalFrameImpl::AddHitTestOnTouchStartCallback(
+    base::RepeatingCallback<void(const blink::WebHitTestResult&)> callback) {
+  TouchStartEventListener* touch_start_event_listener =
+      MakeGarbageCollected<TouchStartEventListener>(std::move(callback));
+  AddEventListenerOptionsResolved options;
+  options.setPassive(true);
+  options.SetPassiveSpecified(true);
+  options.setCapture(true);
+  GetFrame()->DomWindow()->addEventListener(
+      event_type_names::kTouchstart, touch_start_event_listener, &options);
+}
+
 void WebLocalFrameImpl::SetTargetToCurrentHistoryItem(const WebString& target) {
   current_history_item_.SetTarget(target);
 }

third_party/blink/renderer/core/frame/web_local_frame_impl.h

@@ -343,6 +343,9 @@ class CORE_EXPORT WebLocalFrameImpl final
   void SetSessionStorageArea(
       CrossVariantMojoRemote<mojom::StorageAreaInterfaceBase>
           session_storage_area) override;
+  void AddHitTestOnTouchStartCallback(
+      base::RepeatingCallback<void(const blink::WebHitTestResult&)> callback)
+      override;
 
   // WebNavigationControl overrides:
   bool DispatchBeforeUnloadEvent(bool) override;

tools/metrics/histograms/enums.xml

@@ -57152,6 +57152,7 @@ from previous Chrome versions.
   <int value="-785528415" label="FedCm:disabled"/>
   <int value="-784199026" label="EnableFilesAppCopyImage:enabled"/>
   <int value="-783890018" label="LacrosProfileMigrationForAnyUser:disabled"/>
+  <int value="-783093620" label="WebViewHitTestInBlinkOnTouchStart:enabled"/>
   <int value="-780798969" label="disable-single-click-autofill"/>
   <int value="-778126349" label="DownloadsLocationChange:enabled"/>
   <int value="-778098896" label="EnableAggregatedMlSearchRanking:disabled"/>
@@ -59359,6 +59360,7 @@ from previous Chrome versions.
   <int value="663294302" label="ForceUseAPDownloadProtection:disabled"/>
   <int value="664363259" label="SiteCharacteristicsDatabase:enabled"/>
   <int value="664591021" label="EnableContinueReading:enabled"/>
+  <int value="665320858" label="WebViewHitTestInBlinkOnTouchStart:disabled"/>
   <int value="665409384"
       label="AutofillToolkitViewsCreditCardDialogsMac:enabled"/>
   <int value="665647051" label="DiscountConsentV2:enabled"/>