1441136 - Add a fast way to enumerate Shadow Roots in a document.

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement)

Description

[Emilio Cobos Álvarez (:emilio)]

This is needed to handle style changes efficiently, see bug 1425759.

Comment 1

[ExE Boss]

This is also probably needed for WebExtensions and efficiently implementing the `>>>` selector.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

This will help nothing with the piercing combinator implementation if we were to implement it. But actually I don't think we should implement the piercing combinator at all given the current state of affairs.

We could expose ShadowRoots to WebExtensions somehow, and this would help for that though someone needs to request an API for that.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1441136: Cleanup a bit the responsive image content setup.

Also, clear it in DestroyElementMaps, as the rest of the code.

Review commit: https://reviewboard.mozilla.org/r/232538/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/232538/

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1441136: Add a fast way to enumerate ShadowRoots in a document.

Review commit: https://reviewboard.mozilla.org/r/232540/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/232540/

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8963656 [details]
Bug 1441136: Cleanup a bit the responsive image content setup.

https://reviewboard.mozilla.org/r/232538/#review237980

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1441136: Some other minor cleanup.

Now that XBL and Shadow DOM are not entangled we don't really need to do this.

Review commit: https://reviewboard.mozilla.org/r/232594/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/232594/

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8963656 [details]
Bug 1441136: Cleanup a bit the responsive image content setup.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/232538/diff/1-2/

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8963657 [details]
Bug 1441136: Add a fast way to enumerate ShadowRoots in a document.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/232540/diff/1-2/

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8963657 [details]
Bug 1441136: Add a fast way to enumerate ShadowRoots in a document.

https://reviewboard.mozilla.org/r/232540/#review238044

::: dom/base/ShadowRoot.cpp:84
(Diff revision 2)
>    if (auto* host = GetHost()) {
>      // mHost may have been unlinked.
>      host->RemoveMutationObserver(this);
>    }
>  
> +  MOZ_DIAGNOSTIC_ASSERT(!OwnerDoc()->IsComposedDocShadowRoot(*this));

Actually I think I cannot guarantee this:

  https://treeherder.mozilla.org/#/jobs?repo=try&revision=d8c5cc78bb2b43fb8049146e77b9f680ed5c5bf0
  
I guess it can happen when we're on a cycle and we're killed before our host or something?

I guess I can do this and / or SetIsComposedDocParticipant(false) on unlink? What would you prefer me to do?

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8963657 [details]
Bug 1441136: Add a fast way to enumerate ShadowRoots in a document.

https://reviewboard.mozilla.org/r/232540/#review238044

> Actually I think I cannot guarantee this:
> 
>   https://treeherder.mozilla.org/#/jobs?repo=try&revision=d8c5cc78bb2b43fb8049146e77b9f680ed5c5bf0
>   
> I guess it can happen when we're on a cycle and we're killed before our host or something?
> 
> I guess I can do this and / or SetIsComposedDocParticipant(false) on unlink? What would you prefer me to do?

Err, "do this" is supposed to be something like:

```
if (IsComposedDocParticipant()) {
  OwnerDoc()->Remove...(*this);
}
```

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8963657 [details]
Bug 1441136: Add a fast way to enumerate ShadowRoots in a document.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/232540/diff/2-3/

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8963709 [details]
Bug 1441136: Some other minor cleanup.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/232594/diff/1-2/

Comment 13

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8963709 [details]
Bug 1441136: Some other minor cleanup.

https://reviewboard.mozilla.org/r/232594/#review238848

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8963657 [details]
Bug 1441136: Add a fast way to enumerate ShadowRoots in a document.

https://reviewboard.mozilla.org/r/232540/#review238854

::: layout/style/ServoStyleSet.cpp:179
(Diff revision 3)
>  {
>    if (!aDoc.IsShadowDOMEnabled()) {
>      return;
>    }
>  
> -  EnumerateShadowRootsInSubtree(aDoc, aCb);
> +  auto& shadowRoots = aDoc.ComposedShadowRoots();

Totally unclear here what kind of type shadowRoots is. Please don't use auto.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ba3355b94497
Cleanup a bit the responsive image content setup. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/cefe395c3c87
Add a fast way to enumerate ShadowRoots in a document. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/acfd61b3f820
Some other minor cleanup. r=smaug

Comment 16

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/ba3355b94497
https://hg.mozilla.org/mozilla-central/rev/cefe395c3c87
https://hg.mozilla.org/mozilla-central/rev/acfd61b3f820