Skip to content

Commit

Permalink
[VizHitTesting] Propagate pointer-events:none to RemoteFrame
Browse files Browse the repository at this point in the history
When a remote frame has an local ancestor with pointer-events: none, the
frame and all its descendants should be excluded in hit testing.

See https://docs.google.com/document/d/1xN1bRigzahOgUM4cEuyHGhe069TRc5-0rXz1JZsgCP8/edit?usp=sharing

Bug: 1002245
Change-Id: I9e2f86b1d1e6727a66499639994d269d301f1a34
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1836954
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: Yi Gu <yigu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705125}
  • Loading branch information
yi-gu authored and Commit Bot committed Oct 11, 2019
1 parent 974cd5d commit 0bfab86
Show file tree
Hide file tree
Showing 12 changed files with 161 additions and 52 deletions.
104 changes: 76 additions & 28 deletions content/browser/site_per_process_hit_test_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2810,6 +2810,73 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestBrowserTest,
kHitTestTolerance);
}

IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestBrowserTest,
PointerEventsNoneWithNestedSameOriginIFrame) {
GURL main_url(embedded_test_server()->GetURL(
"/frame_tree/page_with_same_origin_nested_frames.html"));
EXPECT_TRUE(NavigateToURL(shell(), main_url));

FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
->GetFrameTree()
->root();
ASSERT_EQ(1U, root->child_count());
RenderWidgetHostViewBase* root_view = static_cast<RenderWidgetHostViewBase*>(
root->current_frame_host()->GetRenderWidgetHost()->GetView());

EXPECT_EQ(
" Site A ------------ proxies for B\n"
" +--Site A ------- proxies for B\n"
" +--Site B -- proxies for A\n"
"Where A = http://127.0.0.1/\n"
" B = http://baz.com/",
DepictFrameTree(root));

FrameTreeNode* child_node = root->child_at(0);
FrameTreeNode* grandchild_node = child_node->child_at(0);

// This is to make sure that the hit_test_data is clean before running the
// hit_test_data_change_observer.
WaitForHitTestData(child_node->current_frame_host());
WaitForHitTestData(grandchild_node->current_frame_host());

HitTestRegionObserver hit_test_data_change_observer(
root_view->GetRootFrameSinkId());
hit_test_data_change_observer.WaitForHitTestData();

EXPECT_TRUE(ExecuteScript(web_contents(),
"document.getElementById('wrapper').style."
"pointerEvents = 'none';"));

hit_test_data_change_observer.WaitForHitTestDataChange();

MainThreadFrameObserver observer(
root->current_frame_host()->GetRenderWidgetHost());
observer.Wait();

// ------------------------
// root 50px
// ---------------------
// |child 50px |
// 50px| -------------- |
// |50px| grand_child ||
// | | ||
// | |-------------||
// ---------------------

// DispatchMouseEventAndWaitUntilDispatch will make sure the mouse event goes
// to the right frame. Create a listener for the grandchild to verify that it
// does not receive the event. No need to create one for the child because
// root and child are on the same process.
RenderWidgetHostMouseEventMonitor grandchild_frame_monitor(
grandchild_node->current_frame_host()->GetRenderWidgetHost());

// Since child has pointer-events: none, (125, 125) should be claimed by root.
DispatchMouseEventAndWaitUntilDispatch(web_contents(), root_view,
gfx::PointF(125, 125), root_view,
gfx::PointF(125, 125));
EXPECT_FALSE(grandchild_frame_monitor.EventWasReceived());
}

IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestBrowserTest,
PointerEventsNoneWithNestedOOPIF) {
GURL main_url(embedded_test_server()->GetURL(
Expand Down Expand Up @@ -2854,12 +2921,6 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestBrowserTest,
root->current_frame_host()->GetRenderWidgetHost());
observer.Wait();

// Create listeners for mouse events.
RenderWidgetHostMouseEventMonitor main_frame_monitor(
root->current_frame_host()->GetRenderWidgetHost());
RenderWidgetHostMouseEventMonitor child_frame_monitor(
child_node->current_frame_host()->GetRenderWidgetHost());

// ------------------------
// root 50px
// ---------------------
Expand All @@ -2869,30 +2930,17 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestBrowserTest,
// | | ||
// | |-------------||
// ---------------------
//
// Since child has pointer-events: none, (125, 125) should be claimed by root.
blink::WebMouseEvent mouse_event(
blink::WebInputEvent::kMouseDown, blink::WebInputEvent::kNoModifiers,
blink::WebInputEvent::GetStaticTimeStampForTests());
mouse_event.button = blink::WebPointerProperties::Button::kLeft;
SetWebEventPositions(&mouse_event, gfx::Point(125, 125), root_view);
mouse_event.click_count = 1;

main_frame_monitor.ResetEventReceived();
child_frame_monitor.ResetEventReceived();

InputEventAckWaiter waiter(root->current_frame_host()->GetRenderWidgetHost(),
blink::WebInputEvent::kMouseDown);
RenderWidgetHostInputEventRouter* router =
web_contents()->GetInputEventRouter();
router->RouteMouseEvent(root_view, &mouse_event, ui::LatencyInfo());
waiter.Wait();
// DispatchMouseEventAndWaitUntilDispatch will make sure the mouse event goes
// to the right frame. Create a listener for the child to verify that it does
// not receive the event.
RenderWidgetHostMouseEventMonitor child_frame_monitor(
child_node->current_frame_host()->GetRenderWidgetHost());

EXPECT_TRUE(main_frame_monitor.EventWasReceived());
EXPECT_NEAR(125, main_frame_monitor.event().PositionInWidget().x,
kHitTestTolerance);
EXPECT_NEAR(125, main_frame_monitor.event().PositionInWidget().y,
kHitTestTolerance);
// Since child has pointer-events: none, (125, 125) should be claimed by root.
DispatchMouseEventAndWaitUntilDispatch(web_contents(), root_view,
gfx::PointF(125, 125), root_view,
gfx::PointF(125, 125));
EXPECT_FALSE(child_frame_monitor.EventWasReceived());
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<!DOCTYPE html>
<style>
iframe {
top: 50px;
left: 50px;
width: 200px;
height: 200px;
}
</style>
<html>
<body>
<div id="wrapper">
<iframe src="page_with_positioned_frame.html"></iframe>
</div>
</body>
</html>
21 changes: 21 additions & 0 deletions third_party/blink/renderer/core/frame/frame.cc
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,27 @@ void Frame::UpdateInheritedEffectiveTouchActionIfPossible() {
}
}

void Frame::UpdateVisibleToHitTesting() {
bool parent_visible_to_hit_testing = true;
if (auto* parent = Tree().Parent())
parent_visible_to_hit_testing = parent->GetVisibleToHitTesting();

bool self_visible_to_hit_testing = true;
if (auto* local_owner = DynamicTo<HTMLFrameOwnerElement>(owner_.Get())) {
self_visible_to_hit_testing =
local_owner->GetLayoutObject()
? local_owner->GetLayoutObject()->Style()->VisibleToHitTesting()
: true;
}

bool visible_to_hit_testing =
parent_visible_to_hit_testing && self_visible_to_hit_testing;
bool changed = visible_to_hit_testing_ != visible_to_hit_testing;
visible_to_hit_testing_ = visible_to_hit_testing;
if (changed)
DidChangeVisibleToHitTesting();
}

const std::string& Frame::ToTraceValue() {
// token's ToString() is latin1.
if (!trace_value_)
Expand Down
7 changes: 7 additions & 0 deletions third_party/blink/renderer/core/frame/frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,9 @@ class CORE_EXPORT Frame : public GarbageCollected<Frame> {
return *window_agent_factory_;
}

bool GetVisibleToHitTesting() const { return visible_to_hit_testing_; }
void UpdateVisibleToHitTesting();

protected:
// |inheriting_agent_factory| should basically be set to the parent frame or
// opener's WindowAgentFactory. Pass nullptr if the frame is isolated from
Expand All @@ -260,6 +263,8 @@ class CORE_EXPORT Frame : public GarbageCollected<Frame> {
return lifecycle_.GetState() == FrameLifecycle::kDetached;
}

virtual void DidChangeVisibleToHitTesting() = 0;

mutable FrameTree tree_node_;

Member<Page> page_;
Expand All @@ -279,6 +284,8 @@ class CORE_EXPORT Frame : public GarbageCollected<Frame> {

TouchAction inherited_effective_touch_action_ = TouchAction::kTouchActionAuto;

bool visible_to_hit_testing_ = true;

private:
Member<FrameClient> client_;
const Member<WindowProxyManager> window_proxy_manager_;
Expand Down
9 changes: 9 additions & 0 deletions third_party/blink/renderer/core/frame/local_frame.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1787,4 +1787,13 @@ void LocalFrame::EvictFromBackForwardCache() {
Client()->EvictFromBackForwardCache();
}

void LocalFrame::DidChangeVisibleToHitTesting() {
// LayoutEmbeddedContent does not propagate style updates to descendants.
// Need to update the field manually.
for (Frame* child = Tree().FirstChild(); child;
child = child->Tree().NextSibling()) {
child->UpdateVisibleToHitTesting();
}
}

} // namespace blink
2 changes: 2 additions & 0 deletions third_party/blink/renderer/core/frame/local_frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,8 @@ class CORE_EXPORT LocalFrame final : public Frame,
void SetIsCapturingMediaCallback(IsCapturingMediaCallback callback);
bool IsCapturingMedia() const;

void DidChangeVisibleToHitTesting() override;

private:
friend class FrameNavigationDisabler;

Expand Down
12 changes: 8 additions & 4 deletions third_party/blink/renderer/core/frame/remote_frame.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ RemoteFrame::RemoteFrame(RemoteFrameClient* client,
dom_window_ = MakeGarbageCollected<RemoteDOMWindow>(*this);
UpdateInertIfPossible();
UpdateInheritedEffectiveTouchActionIfPossible();
UpdateVisibleToHitTesting();

Initialize();
}
Expand Down Expand Up @@ -214,7 +215,7 @@ RemoteFrameClient* RemoteFrame::Client() const {
return static_cast<RemoteFrameClient*>(Frame::Client());
}

void RemoteFrame::PointerEventsChanged() {
void RemoteFrame::DidChangeVisibleToHitTesting() {
if (!cc_layer_ || !is_surface_layer_)
return;

Expand All @@ -226,9 +227,9 @@ bool RemoteFrame::IsIgnoredForHitTest() const {
HTMLFrameOwnerElement* owner = DeprecatedLocalOwner();
if (!owner || !owner->GetLayoutObject())
return false;

return owner->OwnerType() == FrameOwnerElementType::kPortal ||
(owner->GetLayoutObject()->Style()->PointerEvents() ==
EPointerEvents::kNone);
!visible_to_hit_testing_;
}

void RemoteFrame::SetCcLayer(cc::Layer* cc_layer,
Expand All @@ -243,7 +244,10 @@ void RemoteFrame::SetCcLayer(cc::Layer* cc_layer,
is_surface_layer_ = is_surface_layer;
if (cc_layer_) {
GraphicsLayer::RegisterContentsLayer(cc_layer_);
PointerEventsChanged();
if (is_surface_layer) {
static_cast<cc::SurfaceLayer*>(cc_layer_)->SetHasPointerEventsNone(
IsIgnoredForHitTest());
}
}

To<HTMLFrameOwnerElement>(Owner())->SetNeedsCompositingUpdate();
Expand Down
3 changes: 2 additions & 1 deletion third_party/blink/renderer/core/frame/remote_frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,10 @@ class CORE_EXPORT RemoteFrame final : public Frame {

RemoteFrameClient* Client() const;

void PointerEventsChanged();
bool IsIgnoredForHitTest() const;

void DidChangeVisibleToHitTesting() override;

private:
// Frame protected overrides:
void DetachImpl(FrameDetachType) override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -308,11 +308,6 @@ void HTMLFrameOwnerElement::UpdateContainerPolicy(Vector<String>* messages) {
}
}

void HTMLFrameOwnerElement::PointerEventsChanged() {
if (auto* remote_frame = DynamicTo<RemoteFrame>(ContentFrame()))
remote_frame->PointerEventsChanged();
}

void HTMLFrameOwnerElement::FrameOwnerPropertiesChanged() {
// Don't notify about updates if ContentFrame() is null, for example when
// the subframe hasn't been created yet; or if we are in the middle of
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,6 @@ class CORE_EXPORT HTMLFrameOwnerElement : public HTMLElement,
// For unit tests, manually trigger the UpdateContainerPolicy method.
void UpdateContainerPolicyForTests() { UpdateContainerPolicy(); }

// This function is to notify ChildFrameCompositor of pointer-events changes
// of an OOPIF.
void PointerEventsChanged();

void CancelPendingLazyLoad();

void ParseAttribute(const AttributeModificationParams&) override;
Expand Down
29 changes: 19 additions & 10 deletions third_party/blink/renderer/core/layout/layout_embedded_content.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "third_party/blink/renderer/core/frame/embedded_content_view.h"
#include "third_party/blink/renderer/core/frame/local_frame.h"
#include "third_party/blink/renderer/core/frame/local_frame_view.h"
#include "third_party/blink/renderer/core/frame/remote_frame.h"
#include "third_party/blink/renderer/core/frame/remote_frame_view.h"
#include "third_party/blink/renderer/core/html/html_frame_element_base.h"
#include "third_party/blink/renderer/core/html/html_plugin_element.h"
Expand Down Expand Up @@ -254,20 +255,28 @@ void LayoutEmbeddedContent::StyleDidChange(StyleDifference diff,
const ComputedStyle* old_style) {
LayoutReplaced::StyleDidChange(diff, old_style);

if (!old_style || Style()->PointerEvents() != old_style->PointerEvents()) {
if (auto* frame_owner = DynamicTo<HTMLFrameOwnerElement>(GetNode()))
frame_owner->PointerEventsChanged();
if (EmbeddedContentView* embedded_content_view = GetEmbeddedContentView()) {
if (StyleRef().Visibility() != EVisibility::kVisible) {
embedded_content_view->Hide();
} else {
embedded_content_view->Show();
}
}

EmbeddedContentView* embedded_content_view = GetEmbeddedContentView();
if (!embedded_content_view)
if (old_style &&
StyleRef().VisibleToHitTesting() == old_style->VisibleToHitTesting()) {
return;

if (StyleRef().Visibility() != EVisibility::kVisible) {
embedded_content_view->Hide();
} else {
embedded_content_view->Show();
}

auto* frame_owner = DynamicTo<HTMLFrameOwnerElement>(GetNode());
if (!frame_owner)
return;

auto* frame = frame_owner->ContentFrame();
if (!frame)
return;

frame->UpdateVisibleToHitTesting();
}

void LayoutEmbeddedContent::UpdateLayout() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@
# cc::Layers.
'cc::Layer',
'cc::PictureLayer',
'cc::SurfaceLayer',

# cc::Layer helper data structs.
'cc::ElementId',
Expand Down

0 comments on commit 0bfab86

Please sign in to comment.