Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep track of all pending slot assignments, and recalc all before UpdateStyle phase #10174

Merged
merged 1 commit into from Mar 28, 2018

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Mar 26, 2018

For the purpose of rendering, Blink must calculate all pending slot assignments
before UpdateStyle phase. Unless that, a recursive RecalcStyle() may not
traverse a node which should be re-attached.

e.g. Suppose the following tree, where #d1 is assigned to a slot.

host
├──/shadow-root
│ └── slot name=s1
└── div id=d1 slot=s1

Then, we change #d1's slot attribute to s2, like:

document.querySelector('#d1').setAttribute('slot', 's2');

host
├──/shadow-root
│ └── slot name=s1
└── div id=d1 slot=s2

In this case, #d1 should be removed from a LayoutTree. In other words, Blink
has to reattach #d1 somehow. However, if we don't recalc a slot assignment for
the shadow tree before UpdateStyle, a recursive RecalcStyle never traverses the
sub-tree because child-needs-style-recalc flag is not set for the node (and its
ancestor nodes). A flag should be set as a result of slot assignment recalc.

Thanks to the Incremental Shadow DOM, a slot assignemt recalc now becomes a
local operatoin on each shadow tree, rather than one global operation for every
shadow trees. We don't need to traverse a composed tree to recalc all. We can
recalc a slot assignment for each shadow tree directly, without traversing a
composed tree.

For a shadow tree which is not connected, we don't need to recalc its slot
assignment before UpdateStyle because such a shadow tree shouldn't have any
effect on rendering. Lazy slot assignment recalc is enough for such a shadow
tree.

This CL doesn't introduce any optimization to minimize the number of
to-be-reattached nodes. I'll optimize that as a next task. I'll use a sort of
dynamic programming there, as I did at
https://chromium-review.googlesource.com/c/chromium/src/+/535493 for
non-Incremental Shadow DOM.

Except for the performance, this CL should be the last part of Incremental
Shadow DOM from the external behavior's perspective. Style and Layout should
work correctly after this CL, even with Incremental Shadow DOM.

BUG=776656

Change-Id: Id18e87ff59d92863c68c571e7db09253c08aa91f
Reviewed-on: https://chromium-review.googlesource.com/964062
Reviewed-by: Takayoshi Kochi kochi@chromium.org
Reviewed-by: Rune Lillesveen futhark@chromium.org
Commit-Queue: Hayato Ito hayato@chromium.org
Cr-Commit-Position: refs/heads/master@{#546458}

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already reviewed downstream.

@w3c-bots
Copy link

w3c-bots commented Mar 26, 2018

Build PASSED

Started: 2018-03-28 12:08:55
Finished: 2018-03-28 12:13:51

View more information about this build on:

@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-964062 branch 2 times, most recently from 0592468 to 4e6357f Compare March 28, 2018 08:26
…ateStyle phase

For the purpose of rendering, Blink must calculate all pending slot assignments
before UpdateStyle phase. Unless that, a recursive RecalcStyle() may not
traverse a node which should be re-attached.

e.g. Suppose the following tree, where #d1 is assigned to a slot.

host
├──/shadow-root
│   └── slot name=s1
└── div id=d1 slot=s1

Then, we change #d1's slot attribute to s2, like:

> document.querySelector('#d1').setAttribute('slot', 's2');

host
├──/shadow-root
│   └── slot name=s1
└── div id=d1 slot=s2

In this case, #d1 should be removed from a LayoutTree. In other words, Blink
has to reattach #d1 somehow. However, if we don't recalc a slot assignment for
the shadow tree before UpdateStyle, a recursive RecalcStyle never traverses the
sub-tree because child-needs-style-recalc flag is not set for the node (and its
ancestor nodes). A flag should be set as a result of slot assignment recalc.

Thanks to the Incremental Shadow DOM, a slot assignemt recalc now becomes a
local operatoin on each shadow tree, rather than one global operation for every
shadow trees. We don't need to traverse a composed tree to recalc all. We can
recalc a slot assignment for each shadow tree directly, without traversing a
composed tree.

For a shadow tree which is not connected, we don't need to recalc its slot
assignment before UpdateStyle because such a shadow tree shouldn't have any
effect on rendering. Lazy slot assignment recalc is enough for such a shadow
tree.

This CL doesn't introduce any optimization to minimize the number of
to-be-reattached nodes. I'll optimize that as a next task. I'll use a sort of
dynamic programming there, as I did at
https://chromium-review.googlesource.com/c/chromium/src/+/535493 for
non-Incremental Shadow DOM.

Except for the performance, this CL should be the last part of Incremental
Shadow DOM from the external behavior's perspective. Style and Layout should
work correctly after this CL, even with Incremental Shadow DOM.

BUG=776656

Change-Id: Id18e87ff59d92863c68c571e7db09253c08aa91f
Reviewed-on: https://chromium-review.googlesource.com/964062
Reviewed-by: Takayoshi Kochi <kochi@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Hayato Ito <hayato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546458}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants