Skip to content

Commit

Permalink
Avoid setting frame_element_id on layers from frames with |pointer-ev…
Browse files Browse the repository at this point in the history
…ents: none|

To prevent attributing events to subframes with |pointer-events: none|,
avoid setting this on layers generated from the subframe. Replaces
frame_element_id with visible_frame_element_id.

Bug: 910421
Change-Id: If0f709b210781c13444b71937d2b6225b3494d42
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2353545
Commit-Queue: Andrew Comminos <acomminos@fb.com>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798872}
  • Loading branch information
acomminos authored and Commit Bot committed Aug 17, 2020
1 parent 827c515 commit c97cb87
Show file tree
Hide file tree
Showing 11 changed files with 104 additions and 40 deletions.
2 changes: 1 addition & 1 deletion cc/input/input_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ class CC_EXPORT InputHandler {
const gfx::PointF& mouse_position) = 0;
virtual void MouseLeave() = 0;

// Returns frame_element_id from the layer hit by the given point.
// Returns visible_frame_element_id from the layer hit by the given point.
// If the hit test failed, an invalid element ID is returned.
virtual ElementId FindFrameElementIdAtPoint(
const gfx::PointF& mouse_position) = 0;
Expand Down
2 changes: 1 addition & 1 deletion cc/trees/layer_tree_host_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ class CC_EXPORT LayerTreeHostImpl : public InputHandler,
const gfx::Point& viewport_point) override;
void MouseLeave() override;

// Returns frame_element_id from the layer hit by the given point.
// Returns visible_frame_element_id from the layer hit by the given point.
// If the hit test failed, an invalid element ID is returned.
ElementId FindFrameElementIdAtPoint(
const gfx::PointF& viewport_point) override;
Expand Down
21 changes: 11 additions & 10 deletions cc/trees/layer_tree_host_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17588,7 +17588,7 @@ TEST_F(LayerTreeHostImplTest, FrameElementIdHitTestSimple) {
frame_layer->SetDrawsContent(true);
frame_layer->SetHitTestable(true);
CopyProperties(root_layer(), frame_layer);
CreateTransformNode(frame_layer).frame_element_id = ElementId(0x10);
CreateTransformNode(frame_layer).visible_frame_element_id = ElementId(0x10);

UpdateDrawProperties(host_impl_->active_tree());

Expand All @@ -17604,7 +17604,7 @@ TEST_F(LayerTreeHostImplTest, FrameElementIdHitTestInheritance) {
frame_layer->SetHitTestable(true);
CopyProperties(root_layer(), frame_layer);
CreateTransformNode(frame_layer, root_layer()->transform_tree_index())
.frame_element_id = ElementId(0x20);
.visible_frame_element_id = ElementId(0x20);

// Create a child layer with no associated frame, but with the above frame
// layer as a parent.
Expand Down Expand Up @@ -17636,15 +17636,15 @@ TEST_F(LayerTreeHostImplTest, FrameElementIdHitTestOverlap) {
frame_layer->SetBounds(gfx::Size(50, 50));
frame_layer->SetHitTestable(true);
CopyProperties(root_layer(), frame_layer);
CreateTransformNode(frame_layer).frame_element_id = ElementId(0x10);
CreateTransformNode(frame_layer).visible_frame_element_id = ElementId(0x10);

LayerImpl* occluding_frame_layer = AddLayer();
occluding_frame_layer->SetBounds(gfx::Size(50, 50));
occluding_frame_layer->SetHitTestable(true);
CopyProperties(root_layer(), occluding_frame_layer);
auto& occluding_frame_node = CreateTransformNode(
occluding_frame_layer, frame_layer->transform_tree_index());
occluding_frame_node.frame_element_id = ElementId(0x20);
occluding_frame_node.visible_frame_element_id = ElementId(0x20);
occluding_frame_node.parent_frame_id = frame_layer->transform_tree_index();
occluding_frame_layer->SetOffsetToTransformParent(gfx::Vector2dF(25, 25));

Expand All @@ -17665,13 +17665,14 @@ TEST_F(LayerTreeHostImplTest, FrameElementIdHitTestOverlapSimpleClip) {
frame_layer->SetBounds(gfx::Size(50, 50));
frame_layer->SetHitTestable(true);
CopyProperties(root_layer(), frame_layer);
CreateTransformNode(frame_layer).frame_element_id = ElementId(0x10);
CreateTransformNode(frame_layer).visible_frame_element_id = ElementId(0x10);

LayerImpl* clipped_frame_layer = AddLayer();
clipped_frame_layer->SetBounds(gfx::Size(50, 50));
clipped_frame_layer->SetHitTestable(true);
CopyProperties(root_layer(), clipped_frame_layer);
CreateTransformNode(clipped_frame_layer).frame_element_id = ElementId(0x20);
CreateTransformNode(clipped_frame_layer).visible_frame_element_id =
ElementId(0x20);
clipped_frame_layer->SetOffsetToTransformParent(gfx::Vector2dF(25, 25));

// Create a clip excluding the overlapped region.
Expand All @@ -17692,14 +17693,14 @@ TEST_F(LayerTreeHostImplTest, FrameElementIdHitTestOverlapRoundedCorners) {
frame_layer->SetBounds(gfx::Size(50, 50));
frame_layer->SetHitTestable(true);
CopyProperties(root_layer(), frame_layer);
CreateTransformNode(frame_layer).frame_element_id = ElementId(0x10);
CreateTransformNode(frame_layer).visible_frame_element_id = ElementId(0x10);

LayerImpl* rounded_frame_layer = AddLayer();
rounded_frame_layer->SetBounds(gfx::Size(50, 50));
rounded_frame_layer->SetHitTestable(true);
CopyProperties(root_layer(), rounded_frame_layer);
CreateTransformNode(rounded_frame_layer, frame_layer->transform_tree_index())
.frame_element_id = ElementId(0x20);
.visible_frame_element_id = ElementId(0x20);
rounded_frame_layer->SetOffsetToTransformParent(gfx::Vector2dF(25, 25));

// Add rounded corners to the layer, which are unable to be hit tested by the
Expand All @@ -17722,14 +17723,14 @@ TEST_F(LayerTreeHostImplTest, FrameElementIdHitTestOverlapSibling) {
frame_layer->SetHitTestable(true);
CopyProperties(root_layer(), frame_layer);
CreateTransformNode(frame_layer, root_layer()->transform_tree_index())
.frame_element_id = ElementId(0x20);
.visible_frame_element_id = ElementId(0x20);

LayerImpl* sibling_frame_layer = AddLayer();
sibling_frame_layer->SetBounds(gfx::Size(50, 50));
sibling_frame_layer->SetHitTestable(true);
CopyProperties(root_layer(), sibling_frame_layer);
CreateTransformNode(sibling_frame_layer, root_layer()->transform_tree_index())
.frame_element_id = ElementId(0x30);
.visible_frame_element_id = ElementId(0x30);
sibling_frame_layer->SetOffsetToTransformParent(gfx::Vector2dF(25, 25));

UpdateDrawProperties(host_impl_->active_tree());
Expand Down
16 changes: 8 additions & 8 deletions cc/trees/layer_tree_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2382,17 +2382,17 @@ static ElementId GetFrameElementIdForLayer(const LayerImpl* layer) {
auto& transform_tree =
layer->layer_tree_impl()->property_trees()->transform_tree;
auto* node = transform_tree.Node(layer->transform_tree_index());
while (node && !node->frame_element_id) {
while (node && !node->visible_frame_element_id) {
node = transform_tree.Node(node->parent_frame_id);
}
return node ? node->frame_element_id : ElementId();
return node ? node->visible_frame_element_id : ElementId();
}

static void FindClosestMatchingLayerForAttribution(
const gfx::PointF& screen_space_point,
const LayerImpl* root_layer,
FindClosestMatchingLayerState* state) {
std::unordered_set<ElementId, ElementIdHash> hit_frame_element_ids;
std::unordered_set<ElementId, ElementIdHash> hit_visible_frame_element_ids;
// We want to iterate from front to back when hit testing.
for (auto* layer : base::Reversed(*root_layer->layer_tree_impl())) {
if (!layer->HitTestable())
Expand Down Expand Up @@ -2422,8 +2422,8 @@ static void FindClosestMatchingLayerForAttribution(
state->closest_match = layer;
}

ElementId frame_element_id = GetFrameElementIdForLayer(layer);
hit_frame_element_ids.insert(frame_element_id);
ElementId visible_frame_element_id = GetFrameElementIdForLayer(layer);
hit_visible_frame_element_ids.insert(visible_frame_element_id);
}

// Iterate through the transform tree of the hit layer in order to derive the
Expand All @@ -2439,12 +2439,12 @@ static void FindClosestMatchingLayerForAttribution(
layer->layer_tree_impl()->property_trees()->transform_tree;
for (auto* node = transform_tree.Node(layer->transform_tree_index()); node;
node = transform_tree.Node(node->parent_frame_id)) {
hit_frame_element_ids.erase(node->frame_element_id);
if (hit_frame_element_ids.size() == 0)
hit_visible_frame_element_ids.erase(node->visible_frame_element_id);
if (hit_visible_frame_element_ids.size() == 0)
break;
}

if (hit_frame_element_ids.size() > 0) {
if (hit_visible_frame_element_ids.size() > 0) {
state->closest_distance = 0.f;
state->closest_match = nullptr;
}
Expand Down
2 changes: 1 addition & 1 deletion cc/trees/transform_node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ bool TransformNode::operator==(const TransformNode& other) const {
snap_amount == other.snap_amount &&
maximum_animation_scale == other.maximum_animation_scale &&
starting_animation_scale == other.starting_animation_scale &&
frame_element_id == other.frame_element_id;
visible_frame_element_id == other.visible_frame_element_id;
}

void TransformNode::AsValueInto(base::trace_event::TracedValue* value) const {
Expand Down
4 changes: 2 additions & 2 deletions cc/trees/transform_node.h
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,8 @@ struct CC_EXPORT TransformNode {
float starting_animation_scale;

// Set to the element ID of containing document if this transform node is the
// root of a frame subtree.
ElementId frame_element_id;
// root of a visible frame subtree.
ElementId visible_frame_element_id;

bool operator==(const TransformNode& other) const;

Expand Down
5 changes: 5 additions & 0 deletions third_party/blink/renderer/core/frame/local_frame.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2540,6 +2540,11 @@ void LocalFrame::DidChangeVisibleToHitTesting() {
child = child->Tree().NextSibling()) {
child->UpdateVisibleToHitTesting();
}

// The transform property tree node depends on visibility.
if (auto* view = View()->GetLayoutView()) {
view->SetNeedsPaintPropertyUpdate();
}
}

WebPrescientNetworking* LocalFrame::PrescientNetworking() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1605,19 +1605,20 @@ TEST_P(CompositingSimTest, FrameAttribution) {
ASSERT_TRUE(child_transform_node);

// Iterate the transform tree to gather the parent frame element ID.
cc::ElementId frame_element_id;
cc::ElementId visible_frame_element_id;
auto* current_transform_node = child_transform_node;
while (current_transform_node) {
frame_element_id = current_transform_node->frame_element_id;
if (frame_element_id)
visible_frame_element_id = current_transform_node->visible_frame_element_id;
if (visible_frame_element_id)
break;
current_transform_node =
GetPropertyTrees()->transform_tree.parent(current_transform_node);
}

EXPECT_EQ(frame_element_id, CompositorElementIdFromUniqueObjectId(
DOMNodeIds::IdForNode(&GetDocument()),
CompositorElementIdNamespace::kDOMNodeId));
EXPECT_EQ(visible_frame_element_id,
CompositorElementIdFromUniqueObjectId(
DOMNodeIds::IdForNode(&GetDocument()),
CompositorElementIdNamespace::kDOMNodeId));

// Test that a layerized subframe's frame element ID is that of its
// containing document.
Expand All @@ -1631,10 +1632,62 @@ TEST_P(CompositingSimTest, FrameAttribution) {
auto* iframe_transform_node = GetTransformNode(iframe_layer);
EXPECT_TRUE(iframe_transform_node);

EXPECT_EQ(iframe_transform_node->frame_element_id,
EXPECT_EQ(iframe_transform_node->visible_frame_element_id,
CompositorElementIdFromUniqueObjectId(
DOMNodeIds::IdForNode(iframe_doc),
CompositorElementIdNamespace::kDOMNodeId));
}

TEST_P(CompositingSimTest, VisibleFrameRootLayers) {
base::test::ScopedFeatureList feature_list;
feature_list.InitWithFeatureState(
blink::features::kCompositeCrossOriginIframes, true);

SimRequest main_resource("https://origin-a.com/a.html", "text/html");
SimRequest frame_resource("https://origin-b.com/b.html", "text/html");

LoadURL("https://origin-a.com/a.html");
main_resource.Complete(R"HTML(
<!DOCTYPE html>
<iframe id="iframe" src="https://origin-b.com/b.html"></iframe>
)HTML");
frame_resource.Complete("<!DOCTYPE html>");
Compositor().BeginFrame();

// Ensure that the toplevel is marked as a visible root.
auto* toplevel_layer = CcLayerByOwnerNodeId(&GetDocument());
ASSERT_TRUE(toplevel_layer);
auto* toplevel_transform_node = GetTransformNode(toplevel_layer);
ASSERT_TRUE(toplevel_transform_node);

EXPECT_TRUE(toplevel_transform_node->visible_frame_element_id);

// Ensure that the iframe is marked as a visible root.
Document* iframe_doc =
To<HTMLFrameOwnerElement>(GetElementById("iframe"))->contentDocument();
Node* owner_node = iframe_doc;
if (RuntimeEnabledFeatures::CompositeAfterPaintEnabled())
owner_node = iframe_doc->documentElement();
auto* iframe_layer = CcLayerByOwnerNodeId(owner_node);
ASSERT_TRUE(iframe_layer);
auto* iframe_transform_node = GetTransformNode(iframe_layer);
ASSERT_TRUE(iframe_transform_node);

EXPECT_TRUE(iframe_transform_node->visible_frame_element_id);

// Verify that after adding `pointer-events: none`, the subframe is no longer
// considered a visible root.
GetElementById("iframe")->SetInlineStyleProperty(
CSSPropertyID::kPointerEvents, "none");

UpdateAllLifecyclePhases();

iframe_layer = CcLayerByOwnerNodeId(owner_node);
ASSERT_TRUE(iframe_layer);
iframe_transform_node = GetTransformNode(iframe_layer);
ASSERT_TRUE(iframe_transform_node);

EXPECT_FALSE(iframe_transform_node->visible_frame_element_id);
}

} // namespace blink
Original file line number Diff line number Diff line change
Expand Up @@ -544,10 +544,14 @@ void FragmentPaintPropertyTreeBuilder::UpdatePaintOffsetTranslation(
CompositingReason::kDirectReasonsForPaintOffsetTranslationProperty;
state.rendering_context_id = context_.current.rendering_context_id;
if (IsA<LayoutView>(object_)) {
DCHECK(object_.GetFrame());
state.flags.is_frame_paint_offset_translation = true;
state.frame_element_id = CompositorElementIdFromUniqueObjectId(
DOMNodeIds::IdForNode(&object_.GetDocument()),
CompositorElementIdNamespace::kDOMNodeId);
state.visible_frame_element_id =
object_.GetFrame()->GetVisibleToHitTesting()
? CompositorElementIdFromUniqueObjectId(
DOMNodeIds::IdForNode(&object_.GetDocument()),
CompositorElementIdNamespace::kDOMNodeId)
: cc::ElementId();
}
OnUpdate(properties_->UpdatePaintOffsetTranslation(
*context_.current.transform, std::move(state)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,8 @@ int PropertyTreeManager::EnsureCompositorTransformNode(
}
}

compositor_node.frame_element_id = transform_node.GetFrameElementId();
compositor_node.visible_frame_element_id =
transform_node.GetVisibleFrameElementId();

// Attach the index of the nearest parent node associated with a frame.
int parent_frame_id = kInvalidNodeId;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,9 +173,9 @@ class PLATFORM_EXPORT TransformPaintPropertyNode
CompositingReasons direct_compositing_reasons = CompositingReason::kNone;
CompositorElementId compositor_element_id;
std::unique_ptr<CompositorStickyConstraint> sticky_constraint;
// If a frame is rooted at this node, this represents the element ID of the
// containing document.
CompositorElementId frame_element_id;
// If a visible frame is rooted at this node, this represents the element
// ID of the containing document.
CompositorElementId visible_frame_element_id;

PaintPropertyChangeType ComputeChange(
const State& other,
Expand All @@ -198,7 +198,7 @@ class PLATFORM_EXPORT TransformPaintPropertyNode
rendering_context_id != other.rendering_context_id ||
compositor_element_id != other.compositor_element_id ||
scroll != other.scroll || !StickyConstraintEquals(other) ||
frame_element_id != other.frame_element_id) {
visible_frame_element_id != other.visible_frame_element_id) {
return PaintPropertyChangeType::kChangedOnlyValues;
}

Expand Down Expand Up @@ -427,8 +427,8 @@ class PLATFORM_EXPORT TransformPaintPropertyNode
return state_.compositor_element_id;
}

const CompositorElementId& GetFrameElementId() const {
return state_.frame_element_id;
const CompositorElementId& GetVisibleFrameElementId() const {
return state_.visible_frame_element_id;
}

bool IsFramePaintOffsetTranslation() const {
Expand Down

0 comments on commit c97cb87

Please sign in to comment.