Large cursor fallback: ensure correct coordinate space for OOPIFs
A previous change ensured that large custom cursors > 32x32
would be dropped if they are not fully contained with the visual
viewport. However, the computation did not account properly for OOPIFs,
where cursor coordinates were not adjusted to the viewport offset.
This CL further adjusts the cursor rect by translating it to the root
view's coordinate space via LocalToAncestorPoint, before
checking for containment within the visual viewport.
Bug: 1099276
Change-Id: I0a03e7cc249cd785f9e76f931cfc7931b127d56b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2278597
Auto-Submit: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Stefan Zager <szager@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#790122}
diff --git a/content/browser/site_per_process_hit_test_browsertest.cc b/content/browser/site_per_process_hit_test_browsertest.cc
index a2878215..219230e2 100644
--- a/content/browser/site_per_process_hit_test_browsertest.cc
+++ b/content/browser/site_per_process_hit_test_browsertest.cc
@@ -4225,14 +4225,18 @@
void SetCursor(const ui::Cursor& cursor) override {
GetForwardingInterface()->SetCursor(cursor);
+ cursor_ = cursor;
run_loop_.Quit();
}
void Wait() { run_loop_.Run(); }
+ base::Optional<ui::Cursor> cursor() const { return cursor_; }
+
private:
base::RunLoop run_loop_;
RenderWidgetHostImpl* render_widget_host_;
+ base::Optional<ui::Cursor> cursor_;
};
// Verify that we receive a mouse cursor update message when we mouse over
@@ -4331,6 +4335,82 @@
CursorUpdateReceivedFromCrossSiteIframeHelper(shell(),
embedded_test_server());
}
+
+// Regression test for https://crbug.com/1099276. An OOPIF at a negative offset
+// from the main document should not allow large cursors to intersect browser
+// UI.
+IN_PROC_BROWSER_TEST_F(SitePerProcessHitTestBrowserTest,
+ LargeCursorRemovedInOffsetOOPIF) {
+ GURL url(R"(data:text/html,
+ <iframe id='iframe'
+ style ='position:absolute; top: -100px'
+ width=1000px height=1000px>
+ </iframe>)");
+ EXPECT_TRUE(NavigateToURL(shell(), url));
+
+ // The large-cursor.html document has a custom cursor that is 120x120 with a
+ // hotspot on the bottom right corner.
+ NavigateIframeToURL(shell()->web_contents(), "iframe",
+ embedded_test_server()->GetURL("/large-cursor.html"));
+
+ auto* web_contents = static_cast<WebContentsImpl*>(shell()->web_contents());
+ FrameTreeNode* root = web_contents->GetFrameTree()->root();
+
+ FrameTreeNode* child_node = root->child_at(0);
+ EXPECT_NE(shell()->web_contents()->GetSiteInstance(),
+ child_node->current_frame_host()->GetSiteInstance());
+
+ WaitForHitTestData(child_node->current_frame_host());
+
+ RenderWidgetHostViewBase* root_view = static_cast<RenderWidgetHostViewBase*>(
+ root->current_frame_host()->GetRenderWidgetHost()->GetView());
+ RenderWidgetHostImpl* rwh_child =
+ root->child_at(0)->current_frame_host()->GetRenderWidgetHost();
+ RenderWidgetHostViewBase* child_view =
+ static_cast<RenderWidgetHostViewBase*>(rwh_child->GetView());
+
+ auto* router = web_contents->GetInputEventRouter();
+ RenderWidgetHostMouseEventMonitor child_monitor(
+ child_view->GetRenderWidgetHost());
+ RenderWidgetHostMouseEventMonitor root_monitor(
+ root_view->GetRenderWidgetHost());
+
+ // A cursor with enough room in the root view to fully display without
+ // blocking native UI should be shown.
+ {
+ blink::WebMouseEvent mouse_event(
+ blink::WebInputEvent::Type::kMouseMove,
+ blink::WebInputEvent::kNoModifiers,
+ blink::WebInputEvent::GetStaticTimeStampForTests());
+ SetWebEventPositions(&mouse_event, gfx::Point(300, 300), root_view);
+ auto set_cursor_interceptor =
+ std::make_unique<SetCursorInterceptor>(rwh_child);
+ RouteMouseEventAndWaitUntilDispatch(router, root_view, child_view,
+ &mouse_event);
+ set_cursor_interceptor->Wait();
+ EXPECT_TRUE(set_cursor_interceptor->cursor().has_value());
+ EXPECT_EQ(120, set_cursor_interceptor->cursor()->custom_bitmap().width());
+ EXPECT_EQ(120, set_cursor_interceptor->cursor()->custom_bitmap().height());
+ }
+ // A cursor without enough room to be fully enclosed within the root view
+ // should not be shown, even if the iframe is at an offset.
+ {
+ blink::WebMouseEvent mouse_event(
+ blink::WebInputEvent::Type::kMouseMove,
+ blink::WebInputEvent::kNoModifiers,
+ blink::WebInputEvent::GetStaticTimeStampForTests());
+ SetWebEventPositions(&mouse_event, gfx::Point(300, 115), root_view);
+ auto set_cursor_interceptor =
+ std::make_unique<SetCursorInterceptor>(rwh_child);
+ RouteMouseEventAndWaitUntilDispatch(router, root_view, child_view,
+ &mouse_event);
+ // We should see a new cursor come in that replaces the large one.
+ set_cursor_interceptor->Wait();
+ EXPECT_TRUE(set_cursor_interceptor->cursor().has_value());
+ EXPECT_NE(120, set_cursor_interceptor->cursor()->custom_bitmap().width());
+ EXPECT_NE(120, set_cursor_interceptor->cursor()->custom_bitmap().height());
+ }
+}
#endif // !defined(OS_ANDROID)
#if defined(USE_AURA)
diff --git a/content/test/data/large-cursor.html b/content/test/data/large-cursor.html
new file mode 100644
index 0000000..d07dbb3
--- /dev/null
+++ b/content/test/data/large-cursor.html
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<style>
+body {
+ height: 1000px;
+ width: 1000px;
+ cursor: -webkit-image-set(url(single_face.jpg) 1x) 120 120, auto;
+}
+</style>
+
diff --git a/third_party/blink/renderer/core/input/event_handler.cc b/third_party/blink/renderer/core/input/event_handler.cc
index c22b03b..c1a44b0 100644
--- a/third_party/blink/renderer/core/input/event_handler.cc
+++ b/third_party/blink/renderer/core/input/event_handler.cc
@@ -588,14 +588,27 @@
continue;
// For large cursors below the max size, limit their ability to cover UI
- // elements by removing them when they intersect with the visual viewport.
+ // elements by removing them when they are not fully contained by the
+ // visual viewport. Careful, we need to make sure to translate coordinate
+ // spaces if we are in an OOPIF.
+ //
// TODO(csharrison): Consider sending a fallback cursor in the IPC to the
- // browser process so we can do that calculation there instead.
+ // browser process so we can do that calculation there instead, this would
+ // ensure even a compromised renderer could not obscure browser UI with a
+ // large cursor. Also, consider augmenting the intervention to drop the
+ // cursor for iframes if the cursor image obscures content in the parent
+ // frame.
if (size.Width() > kMaximumCursorSizeWithoutFallback ||
size.Height() > kMaximumCursorSizeWithoutFallback) {
- IntRect cursor_rect(IntPoint(location.RoundedPoint() - hot_spot), size);
- IntRect visible_rect = page->GetVisualViewport().VisibleContentRect();
- if (!visible_rect.Contains(cursor_rect)) {
+ PhysicalOffset cursor_offset =
+ frame_->ContentLayoutObject()->LocalToAncestorPoint(
+ location.Point(),
+ nullptr, // no ancestor maps all the way up the hierarchy
+ kTraverseDocumentBoundaries | kApplyMainFrameTransform) -
+ PhysicalOffset(hot_spot);
+ PhysicalRect cursor_rect(cursor_offset, LayoutSize(size));
+ if (!PhysicalRect(page->GetVisualViewport().VisibleContentRect())
+ .Contains(cursor_rect)) {
Deprecation::CountDeprecation(
node->GetExecutionContext(),
WebFeature::kCustomCursorIntersectsViewport);
