Unbounded Element: Clipping bypass and visibility [8/N] [chromium/src : main]

0 views
Skip to first unread message

Vladimir Levin (Gerrit)

unread,
May 25, 2026, 3:57:36 PM (7 days ago) May 25
to Mason Freed, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, Daniel Cheng, Dirk Schulze, Stephen Chenney, alexmo...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, cc-...@chromium.org, creis...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, kinuko...@chromium.org, navigation...@chromium.org, zol...@webkit.org
Attention needed from Mason Freed

Vladimir Levin added 9 comments

File cc/layers/render_surface_impl.cc
Line 664, Patchset 22 (Latest): if (OwningEffectNode() && OwningEffectNode()->render_surface_reason ==
Vladimir Levin . resolved

I think we added IsUnbounded to RenderSurfaceImpl in one of the previous patches (which also checks trees in viz)

File cc/trees/draw_property_utils.cc
Line 701, Patchset 22 (Latest): return gfx::Rect(layer->bounds());
Vladimir Levin . resolved

I think this says if any of my ancestor effect nodes are unbounded, then *my* layer rect is the size of that unbounded ancestor.

I don't really know the consequences here, but presumably what we want to say is if any of effects that only apply to me cause me to be unbounded or something like that. I guess this is to prevent any of the layers from clipping, maybe it's ok.

Line 703, Patchset 22 (Latest): if (node.id == kContentsRootPropertyNodeId) {
Vladimir Levin . unresolved

Should this be before the render surface check? I'm not sure if root can be unbounded

File third_party/blink/renderer/core/css/resolver/style_adjuster.cc
Line 653, Patchset 22 (Latest): builder.SetVisibility(EVisibility::kVisible);
Vladimir Levin . unresolved

Why this change? I'm not sure how that's different from something like display: none in the ancestor or opacity 0 or something

File third_party/blink/renderer/core/html/html_element.cc
Line 1590, Patchset 22 (Latest): GetDocument().UpdateStyleAndLayoutForNode(this,
Vladimir Levin . unresolved

this reminds me of content-visibility: auto ancestors. Also reminds me of maybe requiring that the element is at least partially in the viewport like position anchor

File third_party/blink/renderer/core/layout/layout_box.cc
Line 563, Patchset 22 (Latest): if (auto* html_element = DynamicTo<HTMLElement>(GetNode());
Vladimir Levin . unresolved

You probably want this to be stacked (updating IsStacked) since I don't think we want to support any element interleaving, which will give you the layer type for free

File third_party/blink/renderer/core/paint/cull_rect_updater.cc
Line 100, Patchset 22 (Latest): if (object.IsActiveUnboundedElementOrDescendant()) {
Vladimir Levin . unresolved

Does the "OrDescendant" part mean this object is a descendant of an unbounded element? (another a bit more awkward reading is that an unbounded element is a descendant of this object)

File third_party/blink/renderer/core/paint/paint_layer_clipper.cc
Line 68, Patchset 22 (Latest): for (const LayoutObject* curr = &layer.GetLayoutObject(); curr;
Vladimir Levin . unresolved

This is fine but you can probably want the parents of the paint layer (PaintLayer::Parent) tree since unbounded will be a paint layer

File third_party/blink/renderer/core/paint/pre_paint_tree_walk.cc
Line 650, Patchset 22 (Latest): context.inside_active_unbounded);
Vladimir Levin . unresolved

Presumably this needs some invalidation when the ancestor is no longer unbounded since I think we can prune the tree walk... but I'm guessing we can just force the subtree update if the value changes. Not sure if that happens somewhere already

Open in Gerrit

Related details

Attention is currently required from:
  • Mason Freed
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Iaa2c8b93485b9c44398aac429a13ccade3a6ec16
Gerrit-Change-Number: 7849906
Gerrit-PatchSet: 22
Gerrit-Owner: Mason Freed <mas...@chromium.org>
Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
Gerrit-Reviewer: Vladimir Levin <vmp...@chromium.org>
Gerrit-CC: Daniel Cheng <dch...@chromium.org>
Gerrit-CC: Dirk Schulze <dsch...@chromium.org>
Gerrit-CC: Menard, Alexis <alexis...@intel.com>
Gerrit-CC: Stephen Chenney <sche...@chromium.org>
Gerrit-Attention: Mason Freed <mas...@chromium.org>
Gerrit-Comment-Date: Mon, 25 May 2026 19:57:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Mason Freed (Gerrit)

unread,
May 26, 2026, 7:43:39 PM (6 days ago) May 26
to Vladimir Levin, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, Daniel Cheng, Dirk Schulze, Stephen Chenney, alexmo...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, cc-...@chromium.org, creis...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, kinuko...@chromium.org, navigation...@chromium.org, zol...@webkit.org
Attention needed from Vladimir Levin

Mason Freed added 9 comments

File cc/layers/render_surface_impl.cc
Line 664, Patchset 22: if (OwningEffectNode() && OwningEffectNode()->render_surface_reason ==
Vladimir Levin . resolved

I think we added IsUnbounded to RenderSurfaceImpl in one of the previous patches (which also checks trees in viz)

Mason Freed

Good catch - done.

File cc/trees/draw_property_utils.cc
Line 701, Patchset 22: return gfx::Rect(layer->bounds());
Vladimir Levin . resolved

I think this says if any of my ancestor effect nodes are unbounded, then *my* layer rect is the size of that unbounded ancestor.

I don't really know the consequences here, but presumably what we want to say is if any of effects that only apply to me cause me to be unbounded or something like that. I guess this is to prevent any of the layers from clipping, maybe it's ok.

Mason Freed

Yeah, I think you're right - this would break clipped contained content. I've tweaked this to remove the early return, and instead redirect the ancestor clip target to the unbounded elements effect node.

Line 703, Patchset 22: if (node.id == kContentsRootPropertyNodeId) {
Vladimir Levin . resolved

Should this be before the render surface check? I'm not sure if root can be unbounded

Mason Freed

Done.

File third_party/blink/renderer/core/css/resolver/style_adjuster.cc
Line 653, Patchset 22: builder.SetVisibility(EVisibility::kVisible);
Vladimir Levin . unresolved

Why this change? I'm not sure how that's different from something like display: none in the ancestor or opacity 0 or something

Mason Freed

This counteracts the [UA stylesheet rule](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/resources/html.css;l=981;drc=cea189ccafc8b0e6be51005148649525ad2cc9fb). I used visibility rather than `display:none` so that the layout information is already computed. I debated that though. But this seems like it'll be faster and might make transitions easier?

File third_party/blink/renderer/core/html/html_element.cc
Line 1590, Patchset 22: GetDocument().UpdateStyleAndLayoutForNode(this,
Vladimir Levin . unresolved

this reminds me of content-visibility: auto ancestors. Also reminds me of maybe requiring that the element is at least partially in the viewport like position anchor

Mason Freed

Help me understand what the first sentence is trying to tell me. You're worried what happens if the unbounded element sits in an as-yet unrendered content-visibility:auto tree? If so, yeah you're probably right that this is a problem. Ok for me to fix this as a followup?

On the second sentence, why do you think that should be a requirement? I could see use cases where the very bottom of an input is at the edge of the viewport, but you still want a picker below it (totally outside the viewport). Perhaps we should discuss this on the explainer - I've added an open question.

File third_party/blink/renderer/core/layout/layout_box.cc
Line 563, Patchset 22: if (auto* html_element = DynamicTo<HTMLElement>(GetNode());
Vladimir Levin . resolved

You probably want this to be stacked (updating IsStacked) since I don't think we want to support any element interleaving, which will give you the layer type for free

Mason Freed

Thanks for the pointer! Done.

File third_party/blink/renderer/core/paint/cull_rect_updater.cc
Line 100, Patchset 22: if (object.IsActiveUnboundedElementOrDescendant()) {
Vladimir Levin . resolved

Does the "OrDescendant" part mean this object is a descendant of an unbounded element? (another a bit more awkward reading is that an unbounded element is a descendant of this object)

Mason Freed

I renamed the method - LMK if that makes it more clear. I agree this was confusing.

File third_party/blink/renderer/core/paint/paint_layer_clipper.cc
Line 68, Patchset 22: for (const LayoutObject* curr = &layer.GetLayoutObject(); curr;
Vladimir Levin . resolved

This is fine but you can probably want the parents of the paint layer (PaintLayer::Parent) tree since unbounded will be a paint layer

Mason Freed

Done

File third_party/blink/renderer/core/paint/pre_paint_tree_walk.cc
Line 650, Patchset 22: context.inside_active_unbounded);
Vladimir Levin . resolved

Presumably this needs some invalidation when the ancestor is no longer unbounded since I think we can prune the tree walk... but I'm guessing we can just force the subtree update if the value changes. Not sure if that happens somewhere already

Mason Freed

See HTMLElement::SetUnboundedElementActive(), where I've changed to call SubtreePaintPropertyUpdateReason(). I think based on the docs for that, that it will do the right thing?

Open in Gerrit

Related details

Attention is currently required from:
  • Vladimir Levin
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Iaa2c8b93485b9c44398aac429a13ccade3a6ec16
Gerrit-Change-Number: 7849906
Gerrit-PatchSet: 23
Gerrit-Owner: Mason Freed <mas...@chromium.org>
Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
Gerrit-Reviewer: Vladimir Levin <vmp...@chromium.org>
Gerrit-CC: Daniel Cheng <dch...@chromium.org>
Gerrit-CC: Dirk Schulze <dsch...@chromium.org>
Gerrit-CC: Menard, Alexis <alexis...@intel.com>
Gerrit-CC: Stephen Chenney <sche...@chromium.org>
Gerrit-Attention: Vladimir Levin <vmp...@chromium.org>
Gerrit-Comment-Date: Tue, 26 May 2026 23:43:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Vladimir Levin <vmp...@chromium.org>
satisfied_requirement
unsatisfied_requirement
open
diffy

Rune Lillesveen (Gerrit)

unread,
May 27, 2026, 2:49:06 PM (5 days ago) May 27
to Mason Freed, Rune Lillesveen, Vladimir Levin, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, Daniel Cheng, Dirk Schulze, Stephen Chenney, alexmo...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, cc-...@chromium.org, creis...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, kinuko...@chromium.org, navigation...@chromium.org, zol...@webkit.org
Attention needed from Mason Freed and Vladimir Levin

Rune Lillesveen added 1 comment

File third_party/blink/renderer/core/css/resolver/style_adjuster.cc
Line 653, Patchset 22: builder.SetVisibility(EVisibility::kVisible);
Vladimir Levin . unresolved

Why this change? I'm not sure how that's different from something like display: none in the ancestor or opacity 0 or something

Mason Freed

This counteracts the [UA stylesheet rule](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/resources/html.css;l=981;drc=cea189ccafc8b0e6be51005148649525ad2cc9fb). I used visibility rather than `display:none` so that the layout information is already computed. I debated that though. But this seems like it'll be faster and might make transitions easier?

Rune Lillesveen

Drive-by:

Why is this done via StyleAdjuster?

Could this be a UA rule instead?

Open in Gerrit

Related details

Attention is currently required from:
  • Mason Freed
  • Vladimir Levin
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Iaa2c8b93485b9c44398aac429a13ccade3a6ec16
Gerrit-Change-Number: 7849906
Gerrit-PatchSet: 25
Gerrit-Owner: Mason Freed <mas...@chromium.org>
Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
Gerrit-Reviewer: Vladimir Levin <vmp...@chromium.org>
Gerrit-CC: Daniel Cheng <dch...@chromium.org>
Gerrit-CC: Dirk Schulze <dsch...@chromium.org>
Gerrit-CC: Menard, Alexis <alexis...@intel.com>
Gerrit-CC: Rune Lillesveen <fut...@chromium.org>
Gerrit-CC: Stephen Chenney <sche...@chromium.org>
Gerrit-Attention: Mason Freed <mas...@chromium.org>
Gerrit-Attention: Vladimir Levin <vmp...@chromium.org>
Gerrit-Comment-Date: Wed, 27 May 2026 18:48:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Mason Freed <mas...@chromium.org>
Comment-In-Reply-To: Vladimir Levin <vmp...@chromium.org>
satisfied_requirement
unsatisfied_requirement
open
diffy

Mason Freed (Gerrit)

unread,
May 27, 2026, 4:25:48 PM (5 days ago) May 27
to Arthur Sonzogni, Rune Lillesveen, Vladimir Levin, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, Daniel Cheng, Dirk Schulze, Stephen Chenney, alexmo...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, cc-...@chromium.org, creis...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, kinuko...@chromium.org, navigation...@chromium.org, zol...@webkit.org
Attention needed from Arthur Sonzogni and Vladimir Levin

Mason Freed added 1 comment

Patchset-level comments
File-level comment, Patchset 25 (Latest):
Mason Freed . resolved

+arthursonzogni for just unbounded_element_browsertest.cc

Open in Gerrit

Related details

Attention is currently required from:
  • Arthur Sonzogni
  • Vladimir Levin
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Iaa2c8b93485b9c44398aac429a13ccade3a6ec16
Gerrit-Change-Number: 7849906
Gerrit-PatchSet: 25
Gerrit-Owner: Mason Freed <mas...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
Gerrit-Reviewer: Vladimir Levin <vmp...@chromium.org>
Gerrit-CC: Daniel Cheng <dch...@chromium.org>
Gerrit-CC: Dirk Schulze <dsch...@chromium.org>
Gerrit-CC: Menard, Alexis <alexis...@intel.com>
Gerrit-CC: Rune Lillesveen <fut...@chromium.org>
Gerrit-CC: Stephen Chenney <sche...@chromium.org>
Gerrit-Attention: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Attention: Vladimir Levin <vmp...@chromium.org>
Gerrit-Comment-Date: Wed, 27 May 2026 20:25:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Mason Freed (Gerrit)

unread,
May 27, 2026, 5:19:58 PM (5 days ago) May 27
to Arthur Sonzogni, Rune Lillesveen, Vladimir Levin, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, Daniel Cheng, Dirk Schulze, Stephen Chenney, alexmo...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, cc-...@chromium.org, creis...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, kinuko...@chromium.org, navigation...@chromium.org, zol...@webkit.org
Attention needed from Arthur Sonzogni, Rune Lillesveen and Vladimir Levin

Mason Freed added 1 comment

File third_party/blink/renderer/core/css/resolver/style_adjuster.cc
Line 653, Patchset 22: builder.SetVisibility(EVisibility::kVisible);
Vladimir Levin . unresolved

Why this change? I'm not sure how that's different from something like display: none in the ancestor or opacity 0 or something

Mason Freed

This counteracts the [UA stylesheet rule](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/resources/html.css;l=981;drc=cea189ccafc8b0e6be51005148649525ad2cc9fb). I used visibility rather than `display:none` so that the layout information is already computed. I debated that though. But this seems like it'll be faster and might make transitions easier?

Rune Lillesveen

Drive-by:

Why is this done via StyleAdjuster?

Could this be a UA rule instead?

Mason Freed

You mean via an internal pseudo class? I could do that, if you'd prefer it. (It just takes more work - that's the only reason this is here.)

Open in Gerrit

Related details

Attention is currently required from:
  • Arthur Sonzogni
  • Rune Lillesveen
  • Vladimir Levin
Gerrit-Attention: Rune Lillesveen <fut...@chromium.org>
Gerrit-Attention: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Attention: Vladimir Levin <vmp...@chromium.org>
Gerrit-Comment-Date: Wed, 27 May 2026 21:19:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Mason Freed <mas...@chromium.org>
Comment-In-Reply-To: Rune Lillesveen <fut...@chromium.org>
Comment-In-Reply-To: Vladimir Levin <vmp...@chromium.org>
satisfied_requirement
unsatisfied_requirement
open
diffy

Rune Lillesveen (Gerrit)

unread,
May 27, 2026, 6:09:39 PM (5 days ago) May 27
to Mason Freed, Arthur Sonzogni, Rune Lillesveen, Vladimir Levin, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, Daniel Cheng, Dirk Schulze, Stephen Chenney, alexmo...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, cc-...@chromium.org, creis...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, kinuko...@chromium.org, navigation...@chromium.org, zol...@webkit.org
Attention needed from Arthur Sonzogni, Mason Freed and Vladimir Levin

Rune Lillesveen added 2 comments

File third_party/blink/renderer/core/css/resolver/style_adjuster.cc
Line 653, Patchset 22: builder.SetVisibility(EVisibility::kVisible);
Vladimir Levin . unresolved

Why this change? I'm not sure how that's different from something like display: none in the ancestor or opacity 0 or something

Mason Freed

This counteracts the [UA stylesheet rule](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/resources/html.css;l=981;drc=cea189ccafc8b0e6be51005148649525ad2cc9fb). I used visibility rather than `display:none` so that the layout information is already computed. I debated that though. But this seems like it'll be faster and might make transitions easier?

Rune Lillesveen

Drive-by:

Why is this done via StyleAdjuster?

Could this be a UA rule instead?

Mason Freed

You mean via an internal pseudo class? I could do that, if you'd prefer it. (It just takes more work - that's the only reason this is here.)

Rune Lillesveen

StyleAdjuster was never meant to be a convenient way to implement UA rules, and after sesse@ made it possible to cache ComputedStyles in the MPC with style adjustments applied, the exception you're adding to StyleAdjuster::GetElementTypeCacheKey() takes a slight performance hit. Nothing that in itself is noticeable, but papercuts ...

I won't block this CL, but UA rule should be considered.

Line 1548, Patchset 25 (Latest): if (auto* html_element = DynamicTo<HTMLElement>(element)) {
Rune Lillesveen . unresolved

Add a comment saying this is necessary because it does an element-dependent adjustment of visibility.

Open in Gerrit

Related details

Attention is currently required from:
  • Arthur Sonzogni
  • Mason Freed
  • Vladimir Levin
Gerrit-Attention: Mason Freed <mas...@chromium.org>
Gerrit-Attention: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Attention: Vladimir Levin <vmp...@chromium.org>
Gerrit-Comment-Date: Wed, 27 May 2026 22:09:20 +0000
satisfied_requirement
unsatisfied_requirement
open
diffy

Mason Freed (Gerrit)

unread,
May 27, 2026, 6:30:40 PM (5 days ago) May 27
to Arthur Sonzogni, Rune Lillesveen, Vladimir Levin, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, Daniel Cheng, Dirk Schulze, Stephen Chenney, alexmo...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, cc-...@chromium.org, creis...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, kinuko...@chromium.org, navigation...@chromium.org, zol...@webkit.org
Attention needed from Arthur Sonzogni, Rune Lillesveen and Vladimir Levin

Mason Freed added 1 comment

File third_party/blink/renderer/core/css/resolver/style_adjuster.cc
Line 653, Patchset 22: builder.SetVisibility(EVisibility::kVisible);
Vladimir Levin . unresolved

Why this change? I'm not sure how that's different from something like display: none in the ancestor or opacity 0 or something

Mason Freed

This counteracts the [UA stylesheet rule](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/resources/html.css;l=981;drc=cea189ccafc8b0e6be51005148649525ad2cc9fb). I used visibility rather than `display:none` so that the layout information is already computed. I debated that though. But this seems like it'll be faster and might make transitions easier?

Rune Lillesveen

Drive-by:

Why is this done via StyleAdjuster?

Could this be a UA rule instead?

Mason Freed

You mean via an internal pseudo class? I could do that, if you'd prefer it. (It just takes more work - that's the only reason this is here.)

Rune Lillesveen

StyleAdjuster was never meant to be a convenient way to implement UA rules, and after sesse@ made it possible to cache ComputedStyles in the MPC with style adjustments applied, the exception you're adding to StyleAdjuster::GetElementTypeCacheKey() takes a slight performance hit. Nothing that in itself is noticeable, but papercuts ...

I won't block this CL, but UA rule should be considered.

Mason Freed

Ok, fair enough! I'll switch this to a UA rule via an internal pseudo class. Thanks for the feedback! I'll comment again when done.

Open in Gerrit

Related details

Attention is currently required from:
  • Arthur Sonzogni
  • Rune Lillesveen
  • Vladimir Levin
Gerrit-Attention: Rune Lillesveen <fut...@chromium.org>
Gerrit-Attention: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Attention: Vladimir Levin <vmp...@chromium.org>
Gerrit-Comment-Date: Wed, 27 May 2026 22:30:31 +0000
satisfied_requirement
unsatisfied_requirement
open
diffy

Arthur Sonzogni (Gerrit)

unread,
May 28, 2026, 5:34:27 AM (4 days ago) May 28
to Mason Freed, Rune Lillesveen, Vladimir Levin, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, Daniel Cheng, Dirk Schulze, Stephen Chenney, alexmo...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, cc-...@chromium.org, creis...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, kinuko...@chromium.org, navigation...@chromium.org, zol...@webkit.org
Attention needed from Mason Freed and Vladimir Levin

Arthur Sonzogni voted and added 6 comments

Votes added by Arthur Sonzogni

Code-Review+1

6 comments

Patchset-level comments
File-level comment, Patchset 26 (Latest):
Arthur Sonzogni . resolved

STAMP (and defering to vmpstr)

File content/browser/renderer_host/unbounded_element_browsertest.cc
Line 1, Patchset 26 (Latest):// Copyright 2026 The Chromium Authors
Arthur Sonzogni . unresolved

LGTM, but I only read this test file. I will defer to @vmp...@chromium.org for the whole review.

Line 48, Patchset 26 (Latest): content::WaitForHitTestData(primary_main_frame_host());
Arthur Sonzogni . unresolved

nit: Omit content::

Line 73, Patchset 26 (Latest):// TODO(crbug.com/508672616): Not yet implemented on Android.
#if BUILDFLAG(IS_ANDROID)
#define MAYBE_AncestorClipping DISABLED_AncestorClipping
#define MAYBE_InputEventRouting DISABLED_InputEventRouting
#define MAYBE_PopoverInsideUnbounded DISABLED_PopoverInsideUnbounded
#define MAYBE_CompositorPopupAllocation DISABLED_CompositorPopupAllocation
#define MAYBE_CompositorPopupAllocationHighDPI \
DISABLED_CompositorPopupAllocationHighDPI
#else
#define MAYBE_AncestorClipping AncestorClipping
#define MAYBE_InputEventRouting InputEventRouting
#define MAYBE_PopoverInsideUnbounded PopoverInsideUnbounded
#define MAYBE_CompositorPopupAllocation CompositorPopupAllocation
#define MAYBE_CompositorPopupAllocationHighDPI CompositorPopupAllocationHighDPI
#endif
Arthur Sonzogni . unresolved

Optional.
This could be in the test fixture constructor and/or setup function;

```
// TODO(crbug.com/508672616): Not yet implemented on Android.
if (BUILDFLAG(IS_ANDROID)) {
GTEST_SKIP();
}
```


(Or individual test cases if partially supported)

Line 93, Patchset 26 (Latest): std::string setup_script = R"(
(async () => {
document.body.innerHTML = `
<div id="container" style="width:50px; height:50px; overflow:hidden; position:relative;">
<div id="child" style="width:100px; height:100px; position:absolute; top:0; left:0;" unbounded></div>
</div>
`;
const child = document.getElementById('child');
child.addEventListener('mousedown', () => { window.__clicked = true; });
await child.showUnboundedElement();
return true;
})();
)";
Arthur Sonzogni . unresolved

nit:

  • Wrap to fit 80 columns.
  • EvalJs automatically wait if the expression evaluates to a promise.


https://paste.googleplex.com/5021859476275200?raw

(Sorry, I can't copy-paste due to Chrome DLP bugs, freezing my tab)

Line 121, Patchset 26 (Latest): (async () => {
document.body.innerHTML = '<div id="child" style="width:100px; height:100px;" unbounded></div>';
const div = document.getElementById('child');
div.addEventListener('mousemove', (e) => {
window.__mouse_x = e.clientX;
window.__mouse_y = e.clientY;
});
await div.showUnboundedElement();
return true;
})();
)";
Arthur Sonzogni . unresolved

ditto

Open in Gerrit

Related details

Attention is currently required from:
  • Mason Freed
  • Vladimir Levin
Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement satisfiedReview-Enforcement
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Iaa2c8b93485b9c44398aac429a13ccade3a6ec16
    Gerrit-Change-Number: 7849906
    Gerrit-PatchSet: 26
    Gerrit-Owner: Mason Freed <mas...@chromium.org>
    Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
    Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
    Gerrit-Reviewer: Vladimir Levin <vmp...@chromium.org>
    Gerrit-CC: Daniel Cheng <dch...@chromium.org>
    Gerrit-CC: Dirk Schulze <dsch...@chromium.org>
    Gerrit-CC: Menard, Alexis <alexis...@intel.com>
    Gerrit-CC: Rune Lillesveen <fut...@chromium.org>
    Gerrit-CC: Stephen Chenney <sche...@chromium.org>
    Gerrit-Attention: Mason Freed <mas...@chromium.org>
    Gerrit-Attention: Vladimir Levin <vmp...@chromium.org>
    Gerrit-Comment-Date: Thu, 28 May 2026 09:34:12 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Mason Freed (Gerrit)

    unread,
    May 28, 2026, 5:56:51 PM (4 days ago) May 28
    to Arthur Sonzogni, Rune Lillesveen, Vladimir Levin, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, Daniel Cheng, Dirk Schulze, Stephen Chenney, alexmo...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, cc-...@chromium.org, creis...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, kinuko...@chromium.org, navigation...@chromium.org, zol...@webkit.org
    Attention needed from Arthur Sonzogni, Rune Lillesveen and Vladimir Levin

    Mason Freed added 8 comments

    File content/browser/renderer_host/unbounded_element_browsertest.cc
    Line 1, Patchset 26:// Copyright 2026 The Chromium Authors
    Arthur Sonzogni . resolved

    LGTM, but I only read this test file. I will defer to @vmp...@chromium.org for the whole review.

    Mason Freed

    Thanks! I will need another +1 due to file changes though.

    Line 48, Patchset 26: content::WaitForHitTestData(primary_main_frame_host());
    Arthur Sonzogni . resolved

    nit: Omit content::

    Mason Freed

    Done

    Line 73, Patchset 26:// TODO(crbug.com/508672616): Not yet implemented on Android.

    #if BUILDFLAG(IS_ANDROID)
    #define MAYBE_AncestorClipping DISABLED_AncestorClipping
    #define MAYBE_InputEventRouting DISABLED_InputEventRouting
    #define MAYBE_PopoverInsideUnbounded DISABLED_PopoverInsideUnbounded
    #define MAYBE_CompositorPopupAllocation DISABLED_CompositorPopupAllocation
    #define MAYBE_CompositorPopupAllocationHighDPI \
    DISABLED_CompositorPopupAllocationHighDPI
    #else
    #define MAYBE_AncestorClipping AncestorClipping
    #define MAYBE_InputEventRouting InputEventRouting
    #define MAYBE_PopoverInsideUnbounded PopoverInsideUnbounded
    #define MAYBE_CompositorPopupAllocation CompositorPopupAllocation
    #define MAYBE_CompositorPopupAllocationHighDPI CompositorPopupAllocationHighDPI
    #endif
    Arthur Sonzogni . resolved

    Optional.
    This could be in the test fixture constructor and/or setup function;

    ```
    // TODO(crbug.com/508672616): Not yet implemented on Android.
    if (BUILDFLAG(IS_ANDROID)) {
    GTEST_SKIP();
    }
    ```


    (Or individual test cases if partially supported)

    Mason Freed

    So much better! Thanks! Done.

    Line 93, Patchset 26: std::string setup_script = R"(

    (async () => {
    document.body.innerHTML = `
    <div id="container" style="width:50px; height:50px; overflow:hidden; position:relative;">
    <div id="child" style="width:100px; height:100px; position:absolute; top:0; left:0;" unbounded></div>
    </div>
    `;
    const child = document.getElementById('child');
    child.addEventListener('mousedown', () => { window.__clicked = true; });
    await child.showUnboundedElement();
    return true;
    })();
    )";
    Arthur Sonzogni . resolved

    nit:

    • Wrap to fit 80 columns.
    • EvalJs automatically wait if the expression evaluates to a promise.


    https://paste.googleplex.com/5021859476275200?raw

    (Sorry, I can't copy-paste due to Chrome DLP bugs, freezing my tab)

    Mason Freed

    Ahh nice - thanks!

    Line 121, Patchset 26: (async () => {

    document.body.innerHTML = '<div id="child" style="width:100px; height:100px;" unbounded></div>';
    const div = document.getElementById('child');
    div.addEventListener('mousemove', (e) => {
    window.__mouse_x = e.clientX;
    window.__mouse_y = e.clientY;
    });
    await div.showUnboundedElement();
    return true;
    })();
    )";
    Arthur Sonzogni . resolved

    ditto

    Mason Freed

    Done

    File third_party/blink/renderer/core/css/resolver/style_adjuster.cc
    Line 653, Patchset 22: builder.SetVisibility(EVisibility::kVisible);
    Vladimir Levin . resolved

    Why this change? I'm not sure how that's different from something like display: none in the ancestor or opacity 0 or something

    Mason Freed

    This counteracts the [UA stylesheet rule](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/resources/html.css;l=981;drc=cea189ccafc8b0e6be51005148649525ad2cc9fb). I used visibility rather than `display:none` so that the layout information is already computed. I debated that though. But this seems like it'll be faster and might make transitions easier?

    Rune Lillesveen

    Drive-by:

    Why is this done via StyleAdjuster?

    Could this be a UA rule instead?

    Mason Freed

    You mean via an internal pseudo class? I could do that, if you'd prefer it. (It just takes more work - that's the only reason this is here.)

    Rune Lillesveen

    StyleAdjuster was never meant to be a convenient way to implement UA rules, and after sesse@ made it possible to cache ComputedStyles in the MPC with style adjustments applied, the exception you're adding to StyleAdjuster::GetElementTypeCacheKey() takes a slight performance hit. Nothing that in itself is noticeable, but papercuts ...

    I won't block this CL, but UA rule should be considered.

    Mason Freed

    Ok, fair enough! I'll switch this to a UA rule via an internal pseudo class. Thanks for the feedback! I'll comment again when done.

    Mason Freed

    Done

    Line 1548, Patchset 25: if (auto* html_element = DynamicTo<HTMLElement>(element)) {
    Rune Lillesveen . resolved

    Add a comment saying this is necessary because it does an element-dependent adjustment of visibility.

    Mason Freed

    Turns out we don't need this at all, with the pseudo class.

    File third_party/blink/renderer/core/html/html_element.cc
    Line 1590, Patchset 22: GetDocument().UpdateStyleAndLayoutForNode(this,
    Vladimir Levin . resolved

    this reminds me of content-visibility: auto ancestors. Also reminds me of maybe requiring that the element is at least partially in the viewport like position anchor

    Mason Freed

    Help me understand what the first sentence is trying to tell me. You're worried what happens if the unbounded element sits in an as-yet unrendered content-visibility:auto tree? If so, yeah you're probably right that this is a problem. Ok for me to fix this as a followup?

    On the second sentence, why do you think that should be a requirement? I could see use cases where the very bottom of an input is at the edge of the viewport, but you still want a picker below it (totally outside the viewport). Perhaps we should discuss this on the explainer - I've added an open question.

    Mason Freed

    I'm going to fix this in a followup.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Arthur Sonzogni
    • Rune Lillesveen
    • Vladimir Levin
    Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement is not satisfiedCode-Owners
      • requirement is not satisfiedCode-Review
      • requirement is not satisfiedReview-Enforcement
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: Iaa2c8b93485b9c44398aac429a13ccade3a6ec16
      Gerrit-Change-Number: 7849906
      Gerrit-PatchSet: 28
      Gerrit-Owner: Mason Freed <mas...@chromium.org>
      Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
      Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
      Gerrit-Reviewer: Vladimir Levin <vmp...@chromium.org>
      Gerrit-CC: Daniel Cheng <dch...@chromium.org>
      Gerrit-CC: Dirk Schulze <dsch...@chromium.org>
      Gerrit-CC: Menard, Alexis <alexis...@intel.com>
      Gerrit-CC: Rune Lillesveen <fut...@chromium.org>
      Gerrit-CC: Stephen Chenney <sche...@chromium.org>
      Gerrit-Attention: Rune Lillesveen <fut...@chromium.org>
      Gerrit-Attention: Arthur Sonzogni <arthurs...@chromium.org>
      Gerrit-Attention: Vladimir Levin <vmp...@chromium.org>
      Gerrit-Comment-Date: Thu, 28 May 2026 21:56:36 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      Comment-In-Reply-To: Mason Freed <mas...@chromium.org>
      Comment-In-Reply-To: Rune Lillesveen <fut...@chromium.org>
      Comment-In-Reply-To: Arthur Sonzogni <arthurs...@chromium.org>
      Comment-In-Reply-To: Vladimir Levin <vmp...@chromium.org>
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      Rune Lillesveen (Gerrit)

      unread,
      May 29, 2026, 3:33:32 AM (4 days ago) May 29
      to Mason Freed, Rune Lillesveen, Arthur Sonzogni, Vladimir Levin, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, Daniel Cheng, Dirk Schulze, Stephen Chenney, alexmo...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, cc-...@chromium.org, creis...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, kinuko...@chromium.org, navigation...@chromium.org, zol...@webkit.org
      Attention needed from Arthur Sonzogni, Mason Freed and Vladimir Levin

      Rune Lillesveen voted and added 1 comment

      Votes added by Rune Lillesveen

      Code-Review+1

      1 comment

      Patchset-level comments
      File-level comment, Patchset 30 (Latest):
      Rune Lillesveen . resolved

      core/css lgtm

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Arthur Sonzogni
      • Mason Freed
      • Vladimir Levin
      Submit Requirements:
        • requirement satisfiedCode-Coverage
        • requirement is not satisfiedCode-Owners
        • requirement satisfiedCode-Review
        • requirement satisfiedReview-Enforcement
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: comment
        Gerrit-Project: chromium/src
        Gerrit-Branch: main
        Gerrit-Change-Id: Iaa2c8b93485b9c44398aac429a13ccade3a6ec16
        Gerrit-Change-Number: 7849906
        Gerrit-PatchSet: 30
        Gerrit-Owner: Mason Freed <mas...@chromium.org>
        Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
        Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
        Gerrit-Reviewer: Rune Lillesveen <fut...@chromium.org>
        Gerrit-Reviewer: Vladimir Levin <vmp...@chromium.org>
        Gerrit-CC: Daniel Cheng <dch...@chromium.org>
        Gerrit-CC: Dirk Schulze <dsch...@chromium.org>
        Gerrit-CC: Menard, Alexis <alexis...@intel.com>
        Gerrit-CC: Stephen Chenney <sche...@chromium.org>
        Gerrit-Attention: Mason Freed <mas...@chromium.org>
        Gerrit-Attention: Arthur Sonzogni <arthurs...@chromium.org>
        Gerrit-Attention: Vladimir Levin <vmp...@chromium.org>
        Gerrit-Comment-Date: Fri, 29 May 2026 07:33:11 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: Yes
        satisfied_requirement
        unsatisfied_requirement
        open
        diffy

        Arthur Sonzogni (Gerrit)

        unread,
        May 29, 2026, 5:05:37 AM (3 days ago) May 29
        to Mason Freed, Rune Lillesveen, Vladimir Levin, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, Daniel Cheng, Dirk Schulze, Stephen Chenney, alexmo...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, cc-...@chromium.org, creis...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, kinuko...@chromium.org, navigation...@chromium.org, zol...@webkit.org
        Attention needed from Mason Freed and Vladimir Levin

        Arthur Sonzogni voted and added 1 comment

        Votes added by Arthur Sonzogni

        Code-Review+1

        1 comment

        File content/browser/renderer_host/unbounded_element_browsertest.cc
        Line 73, Patchset 26:// TODO(crbug.com/508672616): Not yet implemented on Android.
        #if BUILDFLAG(IS_ANDROID)
        #define MAYBE_AncestorClipping DISABLED_AncestorClipping
        #define MAYBE_InputEventRouting DISABLED_InputEventRouting
        #define MAYBE_PopoverInsideUnbounded DISABLED_PopoverInsideUnbounded
        #define MAYBE_CompositorPopupAllocation DISABLED_CompositorPopupAllocation
        #define MAYBE_CompositorPopupAllocationHighDPI \
        DISABLED_CompositorPopupAllocationHighDPI
        #else
        #define MAYBE_AncestorClipping AncestorClipping
        #define MAYBE_InputEventRouting InputEventRouting
        #define MAYBE_PopoverInsideUnbounded PopoverInsideUnbounded
        #define MAYBE_CompositorPopupAllocation CompositorPopupAllocation
        #define MAYBE_CompositorPopupAllocationHighDPI CompositorPopupAllocationHighDPI
        #endif
        Arthur Sonzogni . resolved

        Optional.
        This could be in the test fixture constructor and/or setup function;

        ```
        // TODO(crbug.com/508672616): Not yet implemented on Android.
        if (BUILDFLAG(IS_ANDROID)) {
        GTEST_SKIP();
        }
        ```


        (Or individual test cases if partially supported)

        Mason Freed

        So much better! Thanks! Done.

        Arthur Sonzogni

        You are welcome!

        Open in Gerrit

        Related details

        Attention is currently required from:
        • Mason Freed
        • Vladimir Levin
        Gerrit-Attention: Vladimir Levin <vmp...@chromium.org>
        Gerrit-Comment-Date: Fri, 29 May 2026 09:05:18 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: Yes
        Comment-In-Reply-To: Mason Freed <mas...@chromium.org>
        Comment-In-Reply-To: Arthur Sonzogni <arthurs...@chromium.org>
        satisfied_requirement
        unsatisfied_requirement
        open
        diffy

        Vladimir Levin (Gerrit)

        unread,
        May 29, 2026, 3:15:21 PM (3 days ago) May 29
        to Mason Freed, Arthur Sonzogni, Rune Lillesveen, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, Daniel Cheng, Dirk Schulze, Stephen Chenney, alexmo...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, cc-...@chromium.org, creis...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, kinuko...@chromium.org, navigation...@chromium.org, zol...@webkit.org
        Attention needed from Mason Freed

        Vladimir Levin voted and added 5 comments

        Votes added by Vladimir Levin

        Code-Review+1

        5 comments

        Patchset-level comments
        Vladimir Levin . resolved

        lgtm, with some minor nits. Generally, I only commented in one spot to leave a comment but some of the more subtle hunks could also use comments optionally

        File cc/layers/render_surface_impl.cc
        Line 664, Patchset 30 (Latest): if (IsUnbounded()) {
        Vladimir Levin . unresolved

        minor nit, just add another conjunct below?

        File third_party/blink/renderer/core/html/html_element.cc
        Line 1590, Patchset 22: GetDocument().UpdateStyleAndLayoutForNode(this,
        Vladimir Levin . resolved

        this reminds me of content-visibility: auto ancestors. Also reminds me of maybe requiring that the element is at least partially in the viewport like position anchor

        Mason Freed

        Help me understand what the first sentence is trying to tell me. You're worried what happens if the unbounded element sits in an as-yet unrendered content-visibility:auto tree? If so, yeah you're probably right that this is a problem. Ok for me to fix this as a followup?

        On the second sentence, why do you think that should be a requirement? I could see use cases where the very bottom of an input is at the edge of the viewport, but you still want a picker below it (totally outside the viewport). Perhaps we should discuss this on the explainer - I've added an open question.

        Mason Freed

        I'm going to fix this in a followup.

        Vladimir Levin

        None of these were meant to be requirements. Follow up is fine :)

        the content visibility comment is that these types of calls are usually also need changes in display lock context to ensure that we keep things unlocked while it's unbounded or something along those lines. consider a context menu in a content-visibility: auto element that is offscreen. The offscreenness of the element means that we don't update rendering, but unbounded is likely to escape strict containment and paint on screen anyway (like a top layer element). This should mean that we keep the content visibility auto unlocked for the duration of unboundedness, similar to https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/display_lock/display_lock_context.cc;l=1147-1167;drc=fcc8e2e643f4beab78d87a7a8566f0ed50332666

        which notifies if there is a top layer element in the subtree, which subsequently causes the context to stay unlocked:

        https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/display_lock/display_lock_context.cc;l=1317;drc=fcc8e2e643f4beab78d87a7a8566f0ed50332666

        File third_party/blink/renderer/core/layout/layout_object.h
        Line 3336, Patchset 30 (Latest): bool IsInclusiveDescendantOfUnboundedElement() const {
        Vladimir Levin . resolved

        Thanks for the rename, this is much clearer imho

        File third_party/blink/renderer/core/paint/paint_property_tree_builder.cc
        Line 4013, Patchset 30 (Latest): if (auto* html_element = DynamicTo<HTMLElement>(object_.GetNode());
        Vladimir Levin . unresolved

        minor nit: comment

        Open in Gerrit

        Related details

        Attention is currently required from:
        • Mason Freed
        Submit Requirements:
        • requirement satisfiedCode-Coverage
        • requirement satisfiedCode-Owners
        • requirement satisfiedCode-Review
        • requirement is not satisfiedNo-Unresolved-Comments
        • requirement satisfiedReview-Enforcement
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: comment
        Gerrit-Project: chromium/src
        Gerrit-Branch: main
        Gerrit-Change-Id: Iaa2c8b93485b9c44398aac429a13ccade3a6ec16
        Gerrit-Change-Number: 7849906
        Gerrit-PatchSet: 30
        Gerrit-Owner: Mason Freed <mas...@chromium.org>
        Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
        Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
        Gerrit-Reviewer: Rune Lillesveen <fut...@chromium.org>
        Gerrit-Reviewer: Vladimir Levin <vmp...@chromium.org>
        Gerrit-CC: Daniel Cheng <dch...@chromium.org>
        Gerrit-CC: Dirk Schulze <dsch...@chromium.org>
        Gerrit-CC: Menard, Alexis <alexis...@intel.com>
        Gerrit-CC: Stephen Chenney <sche...@chromium.org>
        Gerrit-Attention: Mason Freed <mas...@chromium.org>
        Gerrit-Comment-Date: Fri, 29 May 2026 19:15:08 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: Yes
        Comment-In-Reply-To: Mason Freed <mas...@chromium.org>
        Comment-In-Reply-To: Vladimir Levin <vmp...@chromium.org>
        satisfied_requirement
        unsatisfied_requirement
        open
        diffy

        Mason Freed (Gerrit)

        unread,
        1:24 PM (2 hours ago) 1:24 PM
        to Vladimir Levin, Arthur Sonzogni, Rune Lillesveen, Chromium LUCI CQ, Menard, Alexis, chromium...@chromium.org, Daniel Cheng, Dirk Schulze, Stephen Chenney, alexmo...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, cc-...@chromium.org, creis...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, kinuko...@chromium.org, navigation...@chromium.org, zol...@webkit.org

        Mason Freed voted and added 6 comments

        Votes added by Mason Freed

        Commit-Queue+2

        6 comments

        Patchset-level comments
        Vladimir Levin . resolved

        lgtm, with some minor nits. Generally, I only commented in one spot to leave a comment but some of the more subtle hunks could also use comments optionally

        Mason Freed

        Thanks. I added more comments where it looks like they would help. Feel free to point out any that I missed and I'll add comments as a followup.

        File-level comment, Patchset 31:
        Mason Freed . resolved

        Thanks for the reviews!

        File cc/layers/render_surface_impl.cc
        Line 664, Patchset 30: if (IsUnbounded()) {
        Vladimir Levin . resolved

        minor nit, just add another conjunct below?

        Mason Freed

        That's not minor, I totally agree. Done.

        File third_party/blink/renderer/core/html/html_element.cc
        Line 1590, Patchset 22: GetDocument().UpdateStyleAndLayoutForNode(this,
        Vladimir Levin . resolved

        this reminds me of content-visibility: auto ancestors. Also reminds me of maybe requiring that the element is at least partially in the viewport like position anchor

        Mason Freed

        Help me understand what the first sentence is trying to tell me. You're worried what happens if the unbounded element sits in an as-yet unrendered content-visibility:auto tree? If so, yeah you're probably right that this is a problem. Ok for me to fix this as a followup?

        On the second sentence, why do you think that should be a requirement? I could see use cases where the very bottom of an input is at the edge of the viewport, but you still want a picker below it (totally outside the viewport). Perhaps we should discuss this on the explainer - I've added an open question.

        Mason Freed

        I'm going to fix this in a followup.

        Vladimir Levin

        None of these were meant to be requirements. Follow up is fine :)

        the content visibility comment is that these types of calls are usually also need changes in display lock context to ensure that we keep things unlocked while it's unbounded or something along those lines. consider a context menu in a content-visibility: auto element that is offscreen. The offscreenness of the element means that we don't update rendering, but unbounded is likely to escape strict containment and paint on screen anyway (like a top layer element). This should mean that we keep the content visibility auto unlocked for the duration of unboundedness, similar to https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/display_lock/display_lock_context.cc;l=1147-1167;drc=fcc8e2e643f4beab78d87a7a8566f0ed50332666

        which notifies if there is a top layer element in the subtree, which subsequently causes the context to stay unlocked:

        https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/display_lock/display_lock_context.cc;l=1317;drc=fcc8e2e643f4beab78d87a7a8566f0ed50332666

        Mason Freed

        Thanks for this! Will address as a followup.

        File third_party/blink/renderer/core/layout/layout_object.h
        Line 3336, Patchset 30: bool IsInclusiveDescendantOfUnboundedElement() const {
        Vladimir Levin . resolved

        Thanks for the rename, this is much clearer imho

        Mason Freed

        😊

        File third_party/blink/renderer/core/paint/paint_property_tree_builder.cc
        Line 4013, Patchset 30: if (auto* html_element = DynamicTo<HTMLElement>(object_.GetNode());
        Vladimir Levin . resolved

        minor nit: comment

        Mason Freed

        Done

        Open in Gerrit

        Related details

        Attention set is empty
        Submit Requirements:
          • requirement satisfiedCode-Coverage
          • requirement satisfiedCode-Owners
          • requirement satisfiedCode-Review
          • requirement satisfiedReview-Enforcement
          Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
          Gerrit-MessageType: comment
          Gerrit-Project: chromium/src
          Gerrit-Branch: main
          Gerrit-Change-Id: Iaa2c8b93485b9c44398aac429a13ccade3a6ec16
          Gerrit-Change-Number: 7849906
          Gerrit-PatchSet: 32
          Gerrit-Owner: Mason Freed <mas...@chromium.org>
          Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
          Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
          Gerrit-Reviewer: Rune Lillesveen <fut...@chromium.org>
          Gerrit-Reviewer: Vladimir Levin <vmp...@chromium.org>
          Gerrit-CC: Daniel Cheng <dch...@chromium.org>
          Gerrit-CC: Dirk Schulze <dsch...@chromium.org>
          Gerrit-CC: Menard, Alexis <alexis...@intel.com>
          Gerrit-CC: Stephen Chenney <sche...@chromium.org>
          Gerrit-Comment-Date: Mon, 01 Jun 2026 17:24:27 +0000
          satisfied_requirement
          open
          diffy

          Chromium LUCI CQ (Gerrit)

          unread,
          3:11 PM (23 minutes ago) 3:11 PM
          to Mason Freed, Vladimir Levin, Arthur Sonzogni, Rune Lillesveen, Menard, Alexis, chromium...@chromium.org, Daniel Cheng, Dirk Schulze, Stephen Chenney, alexmo...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-reviews-p...@chromium.org, blink-...@chromium.org, cc-...@chromium.org, creis...@chromium.org, drott+bl...@chromium.org, fmalit...@chromium.org, fserb...@chromium.org, kinuko...@chromium.org, navigation...@chromium.org, zol...@webkit.org

          Chromium LUCI CQ submitted the change with unreviewed changes

          Unreviewed changes

          30 is the latest approved patch-set.
          The change was submitted with unreviewed changes in the following files:

          ```
          The name of the file: third_party/blink/renderer/core/paint/paint_property_tree_builder.cc
          Insertions: 2, Deletions: 0.

          The diff is too large to show. Please review the diff.
          ```
          ```
          The name of the file: third_party/blink/renderer/core/paint/paint_layer_clipper.cc
          Insertions: 3, Deletions: 0.

          The diff is too large to show. Please review the diff.
          ```
          ```
          The name of the file: third_party/blink/renderer/core/layout/layout_object.h
          Insertions: 2, Deletions: 0.

          The diff is too large to show. Please review the diff.
          ```
          ```
          The name of the file: third_party/blink/renderer/core/paint/paint_layer_painter.cc
          Insertions: 2, Deletions: 0.

          The diff is too large to show. Please review the diff.
          ```
          ```
          The name of the file: cc/layers/render_surface_impl.cc
          Insertions: 1, Deletions: 4.

          The diff is too large to show. Please review the diff.
          ```

          Change information

          Commit message:
          Unbounded Element: Clipping bypass and visibility [8/N]

          This change implements the core rendering bypass for active unbounded
          elements and their descendants. It introduces a bitfield on LayoutObject
          to track the unbounded hierarchy and overrides ancestor clips during
          paint property tree construction. Visibility is managed via
          ComputedStyle adjustment during style resolution.

          In the paint property tree, active unbounded elements are attached
          directly to the root clip node. Background clip rect calculations
          are updated to use local border box properties, bypassing ancestor
          clips. Additionally, CullRect::Infinite() is forced during painting
          to prevent culling. The compositor is updated to skip clipping on
          RenderSurfaceImpl and LayerVisibleRect for unbounded elements.
          Finally, active elements are deactivated when the unbounded attribute
          is removed, and browser tests are added to verify clipping bypass,
          input event routing, and dynamic bounds synchronization.

          See go/unbounded-element for the design doc.
          Bug: 508672616
          Change-Id: Iaa2c8b93485b9c44398aac429a13ccade3a6ec16
          Reviewed-by: Rune Lillesveen <fut...@chromium.org>
          Reviewed-by: Arthur Sonzogni <arthurs...@chromium.org>
          Reviewed-by: Vladimir Levin <vmp...@chromium.org>
          Commit-Queue: Mason Freed <mas...@chromium.org>
          Cr-Commit-Position: refs/heads/main@{#1639570}
          Files:
          • M cc/layers/render_surface_impl.cc
          • M cc/trees/draw_property_utils.cc
          • M content/browser/renderer_host/unbounded_element_browsertest.cc
          • M third_party/blink/renderer/core/css/css_selector.cc
          • M third_party/blink/renderer/core/css/css_selector.h
          • M third_party/blink/renderer/core/css/invalidation/rule_invalidation_data_visitor.cc
          • M third_party/blink/renderer/core/css/selector_checker.cc
          • M third_party/blink/renderer/core/html/html_element.cc
          • M third_party/blink/renderer/core/html/html_element.h
          • M third_party/blink/renderer/core/html/resources/html.css
          • M third_party/blink/renderer/core/layout/layout_object.h
          • M third_party/blink/renderer/core/paint/box_fragment_painter.cc
          • M third_party/blink/renderer/core/paint/compositing/compositing_reason_finder.cc
          • M third_party/blink/renderer/core/paint/cull_rect_updater.cc
          • M third_party/blink/renderer/core/paint/paint_layer_clipper.cc
          • M third_party/blink/renderer/core/paint/paint_layer_painter.cc
          • M third_party/blink/renderer/core/paint/paint_property_tree_builder.cc
          • M third_party/blink/renderer/core/paint/pre_paint_tree_walk.cc
          • M third_party/blink/renderer/core/paint/pre_paint_tree_walk.h
          • M third_party/blink/renderer/platform/graphics/compositing_reasons.h
          Change size: L
          Delta: 20 files changed, 312 insertions(+), 68 deletions(-)
          Branch: refs/heads/main
          Submit Requirements:
          • requirement satisfiedCode-Review: +1 by Rune Lillesveen, +1 by Vladimir Levin, +1 by Arthur Sonzogni
          Open in Gerrit
          Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
          Gerrit-MessageType: merged
          Gerrit-Project: chromium/src
          Gerrit-Branch: main
          Gerrit-Change-Id: Iaa2c8b93485b9c44398aac429a13ccade3a6ec16
          Gerrit-Change-Number: 7849906
          Gerrit-PatchSet: 33
          Gerrit-Owner: Mason Freed <mas...@chromium.org>
          Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
          Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
          Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
          Gerrit-Reviewer: Rune Lillesveen <fut...@chromium.org>
          Gerrit-Reviewer: Vladimir Levin <vmp...@chromium.org>
          open
          diffy
          satisfied_requirement
          Reply all
          Reply to author
          Forward
          0 new messages