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

RESOLVED FIXED in Firefox 61

Status

()

enhancement
RESOLVED FIXED
Last year
Last year

People

(Reporter: emilio, Assigned: emilio)

Tracking

unspecified
mozilla61
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox61 fixed)

Details

Attachments

(3 attachments)

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

Comment 1

Last year
This is also probably needed for WebExtensions and efficiently implementing the `>>>` selector.
Blocks: 1439153, 1117572
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.
No longer blocks: 1117572
Assignee: nobody → emilio
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)

Comment 5

Last year
mozreview-review
Comment on attachment 8963656 [details]
Bug 1441136: Cleanup a bit the responsive image content setup.

https://reviewboard.mozilla.org/r/232538/#review237980
Attachment #8963656 - Flags: review?(bugs) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Assignee

Comment 9

Last year
mozreview-review
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?
Assignee

Comment 10

Last year
mozreview-review-reply
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 hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 13

Last year
mozreview-review
Comment on attachment 8963709 [details]
Bug 1441136: Some other minor cleanup.

https://reviewboard.mozilla.org/r/232594/#review238848
Attachment #8963709 - Flags: review?(bugs) → review+

Comment 14

Last year
mozreview-review
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.
Attachment #8963657 - Flags: review?(bugs) → review+

Comment 15

Last year
Pushed by ecoal95@gmail.com:
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
You need to log in before you can comment on or make changes to this bug.