Remove UpdateDistributionForUnknownReasons() [chromium/src : main]

0 views
Skip to first unread message

Mason Freed (Gerrit)

unread,
Sep 14, 2021, 7:18:56 PM9/14/21
to Rune Lillesveen, abigailbk...@google.com, aboxhal...@chromium.org, aleventhal...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dmazzon...@chromium.org, dtseng...@chromium.org, hirokisa...@chromium.org, josiah...@chromium.org, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org

Attention is currently required from: Rune Lillesveen.

Mason Freed would like Rune Lillesveen to review this change.

View Change

Remove UpdateDistributionForUnknownReasons()

This is a leftover from the Shadow DOM v0 codebase, and was
only called in a small handful of places. Most of those were
moved into StyleEngine::UpdateActiveStyle(), and all were
converted to a call to RecalcSlotAssignments(). This CL
also contains other related cleanup, removing "distribution"
as a concept.

This should have no effect on behavior.

Bug: 937746
Change-Id: Iaa6ae72e383444d839ecf1c346e87a83504ffa7a
---
M third_party/blink/renderer/core/css/style_engine.cc
M third_party/blink/renderer/core/dom/README.md
M third_party/blink/renderer/core/dom/document.cc
M third_party/blink/renderer/core/dom/node.cc
M third_party/blink/renderer/core/dom/node.h
M third_party/blink/renderer/core/html/html_element.cc
M third_party/blink/renderer/core/inspector/inspector_css_agent.cc
M third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.cc
M third_party/blink/web_tests/SlowTests
R third_party/blink/web_tests/http/tests/devtools/elements/shadow/shadow-slot-assignment-expected.txt
R third_party/blink/web_tests/http/tests/devtools/elements/shadow/shadow-slot-assignment.js
11 files changed, 22 insertions(+), 53 deletions(-)


To view, visit change 3160780. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Iaa6ae72e383444d839ecf1c346e87a83504ffa7a
Gerrit-Change-Number: 3160780
Gerrit-PatchSet: 2
Gerrit-Owner: Mason Freed <mas...@chromium.org>
Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
Gerrit-Reviewer: Rune Lillesveen <fut...@chromium.org>
Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
Gerrit-CC: Alexis Menard <alexis...@intel.com>
Gerrit-CC: Alice Boxhall <abox...@chromium.org>
Gerrit-CC: Dominic Mazzoni <dmaz...@chromium.org>
Gerrit-CC: Jeongeun Kim <je_jul...@chromium.org>
Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
Gerrit-CC: Nektarios Paisios <nek...@chromium.org>
Gerrit-Attention: Rune Lillesveen <fut...@chromium.org>
Gerrit-MessageType: newchange

Mason Freed (Gerrit)

unread,
Sep 14, 2021, 7:19:01 PM9/14/21
to abigailbk...@google.com, aboxhal...@chromium.org, aleventhal...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dmazzon...@chromium.org, dtseng...@chromium.org, hirokisa...@chromium.org, josiah...@chromium.org, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org, Rune Lillesveen, Chromium LUCI CQ, Alice Boxhall, Alexis Menard, chromium...@chromium.org, devtools...@chromium.org, Jeongeun Kim, Kevin Babbitt, Nektarios Paisios

Attention is currently required from: Rune Lillesveen.

View Change

1 comment:

To view, visit change 3160780. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Iaa6ae72e383444d839ecf1c346e87a83504ffa7a
Gerrit-Change-Number: 3160780
Gerrit-PatchSet: 2
Gerrit-Owner: Mason Freed <mas...@chromium.org>
Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
Gerrit-Reviewer: Rune Lillesveen <fut...@chromium.org>
Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
Gerrit-CC: Alexis Menard <alexis...@intel.com>
Gerrit-CC: Alice Boxhall <abox...@chromium.org>
Gerrit-CC: Dominic Mazzoni <dmaz...@chromium.org>
Gerrit-CC: Jeongeun Kim <je_jul...@chromium.org>
Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
Gerrit-CC: Nektarios Paisios <nek...@chromium.org>
Gerrit-Attention: Rune Lillesveen <fut...@chromium.org>
Gerrit-Comment-Date: Tue, 14 Sep 2021 23:18:52 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Mason Freed (Gerrit)

unread,
Sep 14, 2021, 7:19:47 PM9/14/21
to Alice Boxhall, abigailbk...@google.com, aboxhal...@chromium.org, aleventhal...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dmazzon...@chromium.org, dtseng...@chromium.org, hirokisa...@chromium.org, josiah...@chromium.org, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org, Rune Lillesveen

Attention is currently required from: Rune Lillesveen, Alice Boxhall.

Mason Freed would like Alice Boxhall to review this change.

View Change

Remove UpdateDistributionForUnknownReasons()

This is a leftover from the Shadow DOM v0 codebase, and was
only called in a small handful of places. Most of those were
moved into StyleEngine::UpdateActiveStyle(), and all were
converted to a call to RecalcSlotAssignments(). This CL
also contains other related cleanup, removing "distribution"
as a concept.

This should have no effect on behavior.

Bug: 937746
Change-Id: Iaa6ae72e383444d839ecf1c346e87a83504ffa7a
---
M third_party/blink/renderer/core/css/style_engine.cc
M third_party/blink/renderer/core/dom/README.md
M third_party/blink/renderer/core/dom/document.cc
M third_party/blink/renderer/core/dom/node.cc
M third_party/blink/renderer/core/dom/node.h
M third_party/blink/renderer/core/html/html_element.cc
M third_party/blink/renderer/core/inspector/inspector_css_agent.cc
M third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.cc
M third_party/blink/web_tests/SlowTests
R third_party/blink/web_tests/http/tests/devtools/elements/shadow/shadow-slot-assignment-expected.txt
R third_party/blink/web_tests/http/tests/devtools/elements/shadow/shadow-slot-assignment.js
11 files changed, 22 insertions(+), 53 deletions(-)


To view, visit change 3160780. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Iaa6ae72e383444d839ecf1c346e87a83504ffa7a
Gerrit-Change-Number: 3160780
Gerrit-PatchSet: 2
Gerrit-Owner: Mason Freed <mas...@chromium.org>
Gerrit-Reviewer: Alice Boxhall <abox...@chromium.org>
Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
Gerrit-Reviewer: Rune Lillesveen <fut...@chromium.org>
Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
Gerrit-CC: Alexis Menard <alexis...@intel.com>
Gerrit-CC: Dominic Mazzoni <dmaz...@chromium.org>
Gerrit-CC: Jeongeun Kim <je_jul...@chromium.org>
Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
Gerrit-CC: Nektarios Paisios <nek...@chromium.org>
Gerrit-Attention: Rune Lillesveen <fut...@chromium.org>
Gerrit-Attention: Alice Boxhall <abox...@chromium.org>
Gerrit-MessageType: newchange

Mason Freed (Gerrit)

unread,
Sep 14, 2021, 7:19:53 PM9/14/21
to abigailbk...@google.com, aboxhal...@chromium.org, aleventhal...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dmazzon...@chromium.org, dtseng...@chromium.org, hirokisa...@chromium.org, josiah...@chromium.org, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org, Alice Boxhall, Rune Lillesveen, Chromium LUCI CQ, Alexis Menard, chromium...@chromium.org, devtools...@chromium.org, Jeongeun Kim, Kevin Babbitt, Nektarios Paisios

Attention is currently required from: Rune Lillesveen, Alice Boxhall.

View Change

1 comment:

  • Patchset:

    • Patch Set #2:

      +aboxhall@ for ax_object_cache_impl.cc. Just a comment there. Thanks!

To view, visit change 3160780. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Iaa6ae72e383444d839ecf1c346e87a83504ffa7a
Gerrit-Change-Number: 3160780
Gerrit-PatchSet: 2
Gerrit-Owner: Mason Freed <mas...@chromium.org>
Gerrit-Reviewer: Alice Boxhall <abox...@chromium.org>
Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
Gerrit-Reviewer: Rune Lillesveen <fut...@chromium.org>
Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
Gerrit-CC: Alexis Menard <alexis...@intel.com>
Gerrit-CC: Dominic Mazzoni <dmaz...@chromium.org>
Gerrit-CC: Jeongeun Kim <je_jul...@chromium.org>
Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
Gerrit-CC: Nektarios Paisios <nek...@chromium.org>
Gerrit-Attention: Rune Lillesveen <fut...@chromium.org>
Gerrit-Attention: Alice Boxhall <abox...@chromium.org>
Gerrit-Comment-Date: Tue, 14 Sep 2021 23:19:43 +0000

Rune Lillesveen (Gerrit)

unread,
Sep 15, 2021, 4:16:48 AM9/15/21
to Mason Freed, abigailbk...@google.com, aboxhal...@chromium.org, aleventhal...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dmazzon...@chromium.org, dtseng...@chromium.org, hirokisa...@chromium.org, josiah...@chromium.org, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org, Alice Boxhall, Rune Lillesveen, Chromium LUCI CQ, Alexis Menard, chromium...@chromium.org, devtools...@chromium.org, Jeongeun Kim, Kevin Babbitt, Nektarios Paisios

Attention is currently required from: Alice Boxhall, Mason Freed.

View Change

2 comments:

To view, visit change 3160780. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Iaa6ae72e383444d839ecf1c346e87a83504ffa7a
Gerrit-Change-Number: 3160780
Gerrit-PatchSet: 2
Gerrit-Owner: Mason Freed <mas...@chromium.org>
Gerrit-Reviewer: Alice Boxhall <abox...@chromium.org>
Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
Gerrit-Reviewer: Rune Lillesveen <fut...@chromium.org>
Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
Gerrit-CC: Alexis Menard <alexis...@intel.com>
Gerrit-CC: Dominic Mazzoni <dmaz...@chromium.org>
Gerrit-CC: Jeongeun Kim <je_jul...@chromium.org>
Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
Gerrit-CC: Nektarios Paisios <nek...@chromium.org>
Gerrit-Attention: Alice Boxhall <abox...@chromium.org>
Gerrit-Attention: Mason Freed <mas...@chromium.org>
Gerrit-Comment-Date: Wed, 15 Sep 2021 08:16:29 +0000

Nektarios Paisios (Gerrit)

unread,
Sep 15, 2021, 5:24:10 AM9/15/21
to Mason Freed, abigailbk...@google.com, aboxhal...@chromium.org, aleventhal...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dmazzon...@chromium.org, dtseng...@chromium.org, hirokisa...@chromium.org, josiah...@chromium.org, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org, Alice Boxhall, Rune Lillesveen, Chromium LUCI CQ, Alexis Menard, chromium...@chromium.org, devtools...@chromium.org, Jeongeun Kim, Kevin Babbitt

Attention is currently required from: Alice Boxhall, Mason Freed.

Patch set 2:Code-Review +1

View Change

    To view, visit change 3160780. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Iaa6ae72e383444d839ecf1c346e87a83504ffa7a
    Gerrit-Change-Number: 3160780
    Gerrit-PatchSet: 2
    Gerrit-Owner: Mason Freed <mas...@chromium.org>
    Gerrit-Reviewer: Alice Boxhall <abox...@chromium.org>
    Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
    Gerrit-Reviewer: Nektarios Paisios <nek...@chromium.org>
    Gerrit-Reviewer: Rune Lillesveen <fut...@chromium.org>
    Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
    Gerrit-CC: Alexis Menard <alexis...@intel.com>
    Gerrit-CC: Dominic Mazzoni <dmaz...@chromium.org>
    Gerrit-CC: Jeongeun Kim <je_jul...@chromium.org>
    Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
    Gerrit-Attention: Alice Boxhall <abox...@chromium.org>
    Gerrit-Attention: Mason Freed <mas...@chromium.org>
    Gerrit-Comment-Date: Wed, 15 Sep 2021 09:23:55 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: Yes
    Gerrit-MessageType: comment

    Mason Freed (Gerrit)

    unread,
    Sep 15, 2021, 6:21:54 PM9/15/21
    to abigailbk...@google.com, aboxhal...@chromium.org, aleventhal...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dmazzon...@chromium.org, dtseng...@chromium.org, hirokisa...@chromium.org, josiah...@chromium.org, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org, Nektarios Paisios, Alice Boxhall, Rune Lillesveen, Chromium LUCI CQ, Alexis Menard, chromium...@chromium.org, devtools...@chromium.org, Jeongeun Kim, Kevin Babbitt

    Attention is currently required from: Rune Lillesveen, Alice Boxhall.

    View Change

    2 comments:

    • File third_party/blink/renderer/core/css/style_engine.cc:

      • There are a few places we call UpdateActiveStyle() and didn't update slot assignments without this C […]

        Hmm interesting. What are the rules for <style> tags inside shadow dom? I'm wondering, e.g., what happens if there's a style tag inside slot fallback content. So modifying your example:

          <div id="host"><template shadowroot=open>
        <slot>
        Fallback content
        <style>
        * {font-family: ...;}
        </style>
        </slot>
        </template>slotted content</div>

        Should the <style> element get loaded there? If it depends on whether content is slotted, then perhaps we might need the slot assignment update? Just thinking out loud.


        I'm generally happy putting back the explicit calls to RecalcSlotAssignments() before UpdateActiveStyle(), if you think that's better. I was mainly seeing if I could clean up some of the previous calls, since they sounded like it was unknown whether/why they were needed.

    • File third_party/blink/renderer/core/dom/document.cc:

      • I wonder why we needed this here. […]

        This was one that I hoped to eliminate by pushing it into UpdateActiveStyle(). I can try just removing it entirely (after I remove RecalcSlotAssignments() from UpdateActiveStyle()) and see what happens...?

    To view, visit change 3160780. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Iaa6ae72e383444d839ecf1c346e87a83504ffa7a
    Gerrit-Change-Number: 3160780
    Gerrit-PatchSet: 2
    Gerrit-Owner: Mason Freed <mas...@chromium.org>
    Gerrit-Reviewer: Alice Boxhall <abox...@chromium.org>
    Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
    Gerrit-Reviewer: Nektarios Paisios <nek...@chromium.org>
    Gerrit-Reviewer: Rune Lillesveen <fut...@chromium.org>
    Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
    Gerrit-CC: Alexis Menard <alexis...@intel.com>
    Gerrit-CC: Dominic Mazzoni <dmaz...@chromium.org>
    Gerrit-CC: Jeongeun Kim <je_jul...@chromium.org>
    Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
    Gerrit-Attention: Rune Lillesveen <fut...@chromium.org>
    Gerrit-Attention: Alice Boxhall <abox...@chromium.org>
    Gerrit-Comment-Date: Wed, 15 Sep 2021 22:21:44 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Rune Lillesveen <fut...@chromium.org>
    Gerrit-MessageType: comment

    Mason Freed (Gerrit)

    unread,
    Sep 15, 2021, 7:19:12 PM9/15/21
    to abigailbk...@google.com, aboxhal...@chromium.org, aleventhal...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dmazzon...@chromium.org, dtseng...@chromium.org, hirokisa...@chromium.org, josiah...@chromium.org, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org, Nektarios Paisios, Alice Boxhall, Rune Lillesveen, Chromium LUCI CQ, Alexis Menard, chromium...@chromium.org, devtools...@chromium.org, Jeongeun Kim, Kevin Babbitt

    Attention is currently required from: Rune Lillesveen, Alice Boxhall.

    View Change

    1 comment:

    • File third_party/blink/renderer/core/dom/document.cc:

      • Patch Set #3, Line 2574:

        // TODO(masonf): remove me
        //GetSlotAssignmentEngine().RecalcSlotAssignments()

        We'll see what happens on this patchset.

    To view, visit change 3160780. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Iaa6ae72e383444d839ecf1c346e87a83504ffa7a
    Gerrit-Change-Number: 3160780
    Gerrit-PatchSet: 3
    Gerrit-Owner: Mason Freed <mas...@chromium.org>
    Gerrit-Reviewer: Alice Boxhall <abox...@chromium.org>
    Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
    Gerrit-Reviewer: Nektarios Paisios <nek...@chromium.org>
    Gerrit-Reviewer: Rune Lillesveen <fut...@chromium.org>
    Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
    Gerrit-CC: Alexis Menard <alexis...@intel.com>
    Gerrit-CC: Dominic Mazzoni <dmaz...@chromium.org>
    Gerrit-CC: Jeongeun Kim <je_jul...@chromium.org>
    Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
    Gerrit-Attention: Rune Lillesveen <fut...@chromium.org>
    Gerrit-Attention: Alice Boxhall <abox...@chromium.org>
    Gerrit-Comment-Date: Wed, 15 Sep 2021 23:19:00 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Gerrit-MessageType: comment

    Rune Lillesveen (Gerrit)

    unread,
    Sep 16, 2021, 3:29:38 AM9/16/21
    to Mason Freed, abigailbk...@google.com, aboxhal...@chromium.org, aleventhal...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dmazzon...@chromium.org, dtseng...@chromium.org, hirokisa...@chromium.org, josiah...@chromium.org, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org, Nektarios Paisios, Alice Boxhall, Rune Lillesveen, Chromium LUCI CQ, Alexis Menard, chromium...@chromium.org, devtools...@chromium.org, Jeongeun Kim, Kevin Babbitt

    Attention is currently required from: Alice Boxhall, Mason Freed.

    View Change

    4 comments:

    • File third_party/blink/renderer/core/css/style_engine.cc:

      • Hmm interesting. What are the rules for <style> tags inside shadow dom? I'm wondering, e.g., what happens if there's a style tag inside slot fallback content. So modifying your example:

      •   <div id="host"><template shadowroot=open>
        <slot>
        Fallback content
        <style>
        * {font-family: ...;}
        </style>
        </slot>
        </template>slotted content</div>

        Should the <style> element get loaded there? If it depends on whether content is slotted, then perhaps we might need the slot assignment update? Just thinking out loud.

      • The html spec only says it needs to be connected, so I believe it should be parsed/applied.

      • I'm generally happy putting back the explicit calls to RecalcSlotAssignments() before UpdateActiveStyle(), if you think that's better. I was mainly seeing if I could clean up some of the previous calls, since they sounded like it was unknown whether/why they were needed.

      • Yes, I think we should move it out of UpdateActiveStyle().

    • File third_party/blink/renderer/core/dom/document.cc:

      • This was one that I hoped to eliminate by pushing it into UpdateActiveStyle(). […]

        Yep

    • File third_party/blink/renderer/core/dom/document.cc:

      • Patch Set #3, Line 2574:

        // TODO(masonf): remove me
        //GetSlotAssignmentEngine().RecalcSlotAssignments()

        We'll see what happens on this patchset.

      • Looks green to me.

    • File third_party/blink/renderer/core/inspector/inspector_css_agent.cc:

      • Patch Set #4, Line 2273: element->GetDocument().GetSlotAssignmentEngine().RecalcSlotAssignments();

        I assume this is necessary for PseudoCSSRulesForElement() below. For style recalc, we typically call RecalcSlotAssignments for the document before traversing the DOM for style recalc, not every time we call StyleResolver::StyleForElement(), but this is a one-off for a single element. Might be better to do RecalcSlotAssignments() in PseudoCSSRulesForElement() unless there are cases where we call it for many elements traversing over the tree.

        Browsing the source a bit, we UpdateStyleAndLayoutTreeForNode(), which will update the slot assignments in this case:

        https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/inspector/inspector_style_resolver.cc;l=25-35;drc=2507e0d8b56f6c1b1fb53280d9453a93cee69cfa;bpv=1;bpt=1

        So, the APIs for inspector probably needs a closer look.

        You may leave RecalcSlotAssignments() here for now with a comment saying it's needed for matching rules on an up-to-date flat tree in PseudoCSSRulesForElement(). Or, move it into PseudoCSSRulesForElement(). Your choice.

    To view, visit change 3160780. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Iaa6ae72e383444d839ecf1c346e87a83504ffa7a
    Gerrit-Change-Number: 3160780
    Gerrit-PatchSet: 4
    Gerrit-Owner: Mason Freed <mas...@chromium.org>
    Gerrit-Reviewer: Alice Boxhall <abox...@chromium.org>
    Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
    Gerrit-Reviewer: Nektarios Paisios <nek...@chromium.org>
    Gerrit-Reviewer: Rune Lillesveen <fut...@chromium.org>
    Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
    Gerrit-CC: Alexis Menard <alexis...@intel.com>
    Gerrit-CC: Dominic Mazzoni <dmaz...@chromium.org>
    Gerrit-CC: Jeongeun Kim <je_jul...@chromium.org>
    Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
    Gerrit-Attention: Alice Boxhall <abox...@chromium.org>
    Gerrit-Attention: Mason Freed <mas...@chromium.org>
    Gerrit-Comment-Date: Thu, 16 Sep 2021 07:29:23 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Rune Lillesveen <fut...@chromium.org>
    Comment-In-Reply-To: Mason Freed <mas...@chromium.org>
    Gerrit-MessageType: comment

    Aaron Leventhal (Gerrit)

    unread,
    Sep 16, 2021, 11:30:48 AM9/16/21
    to Mason Freed, abigailbk...@google.com, aboxhal...@chromium.org, aleventhal...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dmazzon...@chromium.org, dtseng...@chromium.org, hirokisa...@chromium.org, josiah...@chromium.org, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org, Nektarios Paisios, Alice Boxhall, Rune Lillesveen, Chromium LUCI CQ, Alexis Menard, chromium...@chromium.org, devtools...@chromium.org, Jeongeun Kim, Kevin Babbitt

    Attention is currently required from: Alice Boxhall, Mason Freed.

    View Change

    1 comment:

    To view, visit change 3160780. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Iaa6ae72e383444d839ecf1c346e87a83504ffa7a
    Gerrit-Change-Number: 3160780
    Gerrit-PatchSet: 4
    Gerrit-Owner: Mason Freed <mas...@chromium.org>
    Gerrit-Reviewer: Alice Boxhall <abox...@chromium.org>
    Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
    Gerrit-Reviewer: Nektarios Paisios <nek...@chromium.org>
    Gerrit-Reviewer: Rune Lillesveen <fut...@chromium.org>
    Gerrit-CC: Aaron Leventhal <aleve...@chromium.org>
    Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
    Gerrit-CC: Alexis Menard <alexis...@intel.com>
    Gerrit-CC: Dominic Mazzoni <dmaz...@chromium.org>
    Gerrit-CC: Jeongeun Kim <je_jul...@chromium.org>
    Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
    Gerrit-Attention: Alice Boxhall <abox...@chromium.org>
    Gerrit-Attention: Mason Freed <mas...@chromium.org>
    Gerrit-Comment-Date: Thu, 16 Sep 2021 15:30:32 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Gerrit-MessageType: comment

    Mason Freed (Gerrit)

    unread,
    Sep 16, 2021, 12:17:12 PM9/16/21
    to abigailbk...@google.com, aboxhal...@chromium.org, aleventhal...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dmazzon...@chromium.org, dtseng...@chromium.org, hirokisa...@chromium.org, josiah...@chromium.org, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org, Aaron Leventhal, Nektarios Paisios, Alice Boxhall, Rune Lillesveen, Chromium LUCI CQ, Alexis Menard, chromium...@chromium.org, devtools...@chromium.org, Jeongeun Kim, Kevin Babbitt

    Attention is currently required from: Rune Lillesveen, Alice Boxhall, Aaron Leventhal.

    View Change

    5 comments:

    • Patchset:

      • Yep, thanks for the heads-up. nektar@ +1'd this CL for me.

        We really should add an OOO notification to aboxhall@'s Gerrit account, to avoid this problem going forward. Though that'll be hard without her help.

    • File third_party/blink/renderer/core/css/style_engine.cc:

      • > Hmm interesting. What are the rules for <style> tags inside shadow dom? I'm wondering, e.g. […]

        Done

    • File third_party/blink/renderer/core/dom/document.cc:

      • Yep

        Done

    • File third_party/blink/renderer/core/dom/document.cc:

      • Looks green to me.

        Great! I removed the comment and the call.

    • File third_party/blink/renderer/core/inspector/inspector_css_agent.cc:

    To view, visit change 3160780. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Iaa6ae72e383444d839ecf1c346e87a83504ffa7a
    Gerrit-Change-Number: 3160780
    Gerrit-PatchSet: 5
    Gerrit-Owner: Mason Freed <mas...@chromium.org>
    Gerrit-Reviewer: Alice Boxhall <abox...@chromium.org>
    Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
    Gerrit-Reviewer: Nektarios Paisios <nek...@chromium.org>
    Gerrit-Reviewer: Rune Lillesveen <fut...@chromium.org>
    Gerrit-CC: Aaron Leventhal <aleve...@chromium.org>
    Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
    Gerrit-CC: Alexis Menard <alexis...@intel.com>
    Gerrit-CC: Dominic Mazzoni <dmaz...@chromium.org>
    Gerrit-CC: Jeongeun Kim <je_jul...@chromium.org>
    Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
    Gerrit-Attention: Rune Lillesveen <fut...@chromium.org>
    Gerrit-Attention: Alice Boxhall <abox...@chromium.org>
    Gerrit-Attention: Aaron Leventhal <aleve...@chromium.org>
    Gerrit-Comment-Date: Thu, 16 Sep 2021 16:17:01 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Rune Lillesveen <fut...@chromium.org>
    Comment-In-Reply-To: Aaron Leventhal <aleve...@chromium.org>

    Mason Freed (Gerrit)

    unread,
    Sep 16, 2021, 12:18:57 PM9/16/21
    to abigailbk...@google.com, aboxhal...@chromium.org, aleventhal...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dmazzon...@chromium.org, dtseng...@chromium.org, hirokisa...@chromium.org, josiah...@chromium.org, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org

    Attention is currently required from: Rune Lillesveen, Alice Boxhall, Aaron Leventhal.

    Mason Freed uploaded patch set #6 to this change.

    View Change

    Remove UpdateDistributionForUnknownReasons()

    This is a leftover from the Shadow DOM v0 codebase, and was
    only called in a small handful of places. Most of those were
    replaced by calls to RecalcSlotAssignments(), but a few were
    removed entirely. This CL also contains other related cleanup,

    removing "distribution" as a concept.

    This should have no effect on behavior.

    Bug: 937746
    Change-Id: Iaa6ae72e383444d839ecf1c346e87a83504ffa7a
    ---
    M third_party/blink/renderer/core/dom/README.md
    M third_party/blink/renderer/core/dom/document.cc
    M third_party/blink/renderer/core/dom/node.cc
    M third_party/blink/renderer/core/dom/node.h
    M third_party/blink/renderer/core/html/html_element.cc
    M third_party/blink/renderer/core/inspector/inspector_css_agent.cc
    M third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.cc
    M third_party/blink/web_tests/SlowTests
    R third_party/blink/web_tests/http/tests/devtools/elements/shadow/shadow-slot-assignment-expected.txt
    R third_party/blink/web_tests/http/tests/devtools/elements/shadow/shadow-slot-assignment.js
    10 files changed, 29 insertions(+), 54 deletions(-)

    To view, visit change 3160780. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Iaa6ae72e383444d839ecf1c346e87a83504ffa7a
    Gerrit-Change-Number: 3160780
    Gerrit-PatchSet: 6
    Gerrit-Owner: Mason Freed <mas...@chromium.org>
    Gerrit-Reviewer: Alice Boxhall <abox...@chromium.org>
    Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
    Gerrit-Reviewer: Nektarios Paisios <nek...@chromium.org>
    Gerrit-Reviewer: Rune Lillesveen <fut...@chromium.org>
    Gerrit-CC: Aaron Leventhal <aleve...@chromium.org>
    Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
    Gerrit-CC: Alexis Menard <alexis...@intel.com>
    Gerrit-CC: Dominic Mazzoni <dmaz...@chromium.org>
    Gerrit-CC: Jeongeun Kim <je_jul...@chromium.org>
    Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
    Gerrit-Attention: Rune Lillesveen <fut...@chromium.org>
    Gerrit-Attention: Alice Boxhall <abox...@chromium.org>
    Gerrit-Attention: Aaron Leventhal <aleve...@chromium.org>
    Gerrit-MessageType: newpatchset

    Rune Lillesveen (Gerrit)

    unread,
    Sep 16, 2021, 1:50:51 PM9/16/21
    to Mason Freed, abigailbk...@google.com, aboxhal...@chromium.org, aleventhal...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dmazzon...@chromium.org, dtseng...@chromium.org, hirokisa...@chromium.org, josiah...@chromium.org, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org, Rune Lillesveen, Aaron Leventhal, Nektarios Paisios, Alice Boxhall, Chromium LUCI CQ, Alexis Menard, chromium...@chromium.org, devtools...@chromium.org, Jeongeun Kim, Kevin Babbitt

    Attention is currently required from: Alice Boxhall, Mason Freed, Aaron Leventhal.

    Patch set 6:Code-Review +1

    View Change

    2 comments:

    • Patchset:

    • File third_party/blink/renderer/core/inspector/inspector_css_agent.cc:

      • Thanks for pointing that out. […]

        Yes, wfm.

    To view, visit change 3160780. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Iaa6ae72e383444d839ecf1c346e87a83504ffa7a
    Gerrit-Change-Number: 3160780
    Gerrit-PatchSet: 6
    Gerrit-Owner: Mason Freed <mas...@chromium.org>
    Gerrit-Reviewer: Alice Boxhall <abox...@chromium.org>
    Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
    Gerrit-Reviewer: Nektarios Paisios <nek...@chromium.org>
    Gerrit-Reviewer: Rune Lillesveen <fut...@chromium.org>
    Gerrit-CC: Aaron Leventhal <aleve...@chromium.org>
    Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
    Gerrit-CC: Alexis Menard <alexis...@intel.com>
    Gerrit-CC: Dominic Mazzoni <dmaz...@chromium.org>
    Gerrit-CC: Jeongeun Kim <je_jul...@chromium.org>
    Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
    Gerrit-Attention: Alice Boxhall <abox...@chromium.org>
    Gerrit-Attention: Mason Freed <mas...@chromium.org>
    Gerrit-Attention: Aaron Leventhal <aleve...@chromium.org>
    Gerrit-Comment-Date: Thu, 16 Sep 2021 17:50:08 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    Comment-In-Reply-To: Rune Lillesveen <fut...@chromium.org>

    Mason Freed (Gerrit)

    unread,
    Sep 16, 2021, 1:59:10 PM9/16/21
    to abigailbk...@google.com, aboxhal...@chromium.org, aleventhal...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dmazzon...@chromium.org, dtseng...@chromium.org, hirokisa...@chromium.org, josiah...@chromium.org, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org, Rune Lillesveen, Aaron Leventhal, Nektarios Paisios, Alice Boxhall, Chromium LUCI CQ, Alexis Menard, chromium...@chromium.org, devtools...@chromium.org, Jeongeun Kim, Kevin Babbitt

    Attention is currently required from: Alice Boxhall, Aaron Leventhal.

    Patch set 6:Commit-Queue +2

    View Change

    1 comment:

    To view, visit change 3160780. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Iaa6ae72e383444d839ecf1c346e87a83504ffa7a
    Gerrit-Change-Number: 3160780
    Gerrit-PatchSet: 6
    Gerrit-Owner: Mason Freed <mas...@chromium.org>
    Gerrit-Reviewer: Alice Boxhall <abox...@chromium.org>
    Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
    Gerrit-Reviewer: Nektarios Paisios <nek...@chromium.org>
    Gerrit-Reviewer: Rune Lillesveen <fut...@chromium.org>
    Gerrit-CC: Aaron Leventhal <aleve...@chromium.org>
    Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
    Gerrit-CC: Alexis Menard <alexis...@intel.com>
    Gerrit-CC: Dominic Mazzoni <dmaz...@chromium.org>
    Gerrit-CC: Jeongeun Kim <je_jul...@chromium.org>
    Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
    Gerrit-Attention: Alice Boxhall <abox...@chromium.org>
    Gerrit-Attention: Aaron Leventhal <aleve...@chromium.org>
    Gerrit-Comment-Date: Thu, 16 Sep 2021 17:59:02 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    Gerrit-MessageType: comment

    Chromium LUCI CQ (Gerrit)

    unread,
    Sep 16, 2021, 2:29:37 PM9/16/21
    to Mason Freed, abigailbk...@google.com, aboxhal...@chromium.org, aleventhal...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, devtools-re...@chromium.org, dmazzon...@chromium.org, dtseng...@chromium.org, hirokisa...@chromium.org, josiah...@chromium.org, nektar...@chromium.org, sarakat...@chromium.org, yuzo+...@chromium.org, Rune Lillesveen, Aaron Leventhal, Nektarios Paisios, Alice Boxhall, Alexis Menard, chromium...@chromium.org, devtools...@chromium.org, Jeongeun Kim, Kevin Babbitt

    Chromium LUCI CQ submitted this change.

    View Change


    Approvals: Nektarios Paisios: Looks good to me Rune Lillesveen: Looks good to me Mason Freed: Commit
    Remove UpdateDistributionForUnknownReasons()

    This is a leftover from the Shadow DOM v0 codebase, and was
    only called in a small handful of places. Most of those were
    replaced by calls to RecalcSlotAssignments(), but a few were
    removed entirely. This CL also contains other related cleanup,
    removing "distribution" as a concept.

    This should have no effect on behavior.

    Bug: 937746
    Change-Id: Iaa6ae72e383444d839ecf1c346e87a83504ffa7a
    Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3160780
    Reviewed-by: Rune Lillesveen <fut...@chromium.org>
    Reviewed-by: Nektarios Paisios <nek...@chromium.org>
    Commit-Queue: Mason Freed <mas...@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#922231}

    ---
    M third_party/blink/renderer/core/dom/README.md
    M third_party/blink/renderer/core/dom/document.cc
    M third_party/blink/renderer/core/dom/node.cc
    M third_party/blink/renderer/core/dom/node.h
    M third_party/blink/renderer/core/html/html_element.cc
    M third_party/blink/renderer/core/inspector/inspector_css_agent.cc
    M third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.cc
    M third_party/blink/web_tests/SlowTests
    R third_party/blink/web_tests/http/tests/devtools/elements/shadow/shadow-slot-assignment-expected.txt
    R third_party/blink/web_tests/http/tests/devtools/elements/shadow/shadow-slot-assignment.js
    10 files changed, 29 insertions(+), 54 deletions(-)


    To view, visit change 3160780. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: Iaa6ae72e383444d839ecf1c346e87a83504ffa7a
    Gerrit-Change-Number: 3160780
    Gerrit-PatchSet: 7
    Gerrit-Owner: Mason Freed <mas...@chromium.org>
    Gerrit-Reviewer: Alice Boxhall <abox...@chromium.org>
    Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
    Gerrit-Reviewer: Mason Freed <mas...@chromium.org>
    Gerrit-Reviewer: Nektarios Paisios <nek...@chromium.org>
    Gerrit-Reviewer: Rune Lillesveen <fut...@chromium.org>
    Gerrit-CC: Aaron Leventhal <aleve...@chromium.org>
    Gerrit-CC: Akihiro Ota <akihi...@chromium.org>
    Gerrit-CC: Alexis Menard <alexis...@intel.com>
    Gerrit-CC: Dominic Mazzoni <dmaz...@chromium.org>
    Gerrit-CC: Jeongeun Kim <je_jul...@chromium.org>
    Gerrit-CC: Kevin Babbitt <kbab...@microsoft.com>
    Gerrit-MessageType: merged
    Reply all
    Reply to author
    Forward
    0 new messages