1433336 - Don't collect all descendants documents for intersection observer in nsRefreshDriver

Status: RESOLVED FIXED

(Core :: Layout, enhancement)

Description

[Hiroyuki Ikezoe (:hiro)]

We are enumerating all descendants but we just need documents that are observed by IntersectionObserver.

https://hg.mozilla.org/mozilla-central/file/b5b38db26ed7/layout/base/nsRefreshDriver.cpp#l1582

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c54c77f25ab193b8aa9958a642451463d685fa99

Comment 1

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1433336 - Drop unused SubDocEnumArgs in nsDocument.cpp.

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

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1433336 - Add nsIDocument::CollectDescendantDocument().

In contrast to nsIDocument::EnumerateSubDocuments(), this new function is
supposed to work with the callback function which does not mutate any state
in the document, and also this function checks all descendant documents even
if there is a descendant that the callback function returns false.

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

Comment 3

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1433336 - Collect only documents which are observerd by IntersectionObserver in nsRefreshDriver::Tick().

We don't need to collect whole descendants documents.

This patch intentionally leaves animation events handling which is another
caller of CollectDocuments in nsRefreshDriver since the animation part will
be fixed in a different way in bug 1415780.

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

Comment 4

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

Comment on attachment 8945670 [details]
Bug 1433336 - Drop unused SubDocEnumArgs in nsDocument.cpp.

https://reviewboard.mozilla.org/r/215806/#review221610

Comment 5

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

Comment on attachment 8945671 [details]
Bug 1433336 - Add nsIDocument::CollectDescendantDocument().

https://reviewboard.mozilla.org/r/215808/#review221612

::: dom/base/nsIDocument.h:1920
(Diff revision 1)
>    typedef bool (*nsSubDocEnumFunc)(nsIDocument *aDocument, void *aData);
>    virtual void EnumerateSubDocuments(nsSubDocEnumFunc aCallback,
>                                       void *aData) = 0;
>  
>    /**
> +   * Collect descendant documents that the |aCalback| returns true for the

Maybe: Collect all the descendant documents for which |aCallback| returns true.

Comment 6

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

Comment on attachment 8945672 [details]
Bug 1433336 - Collect only documents which are observerd by IntersectionObserver in nsRefreshDriver::Tick().

https://reviewboard.mozilla.org/r/215810/#review221618

Comment 7

[Hiroyuki Ikezoe (:hiro)]

(In reply to Olli Pettay [:smaug] from comment #5)
> Comment on attachment 8945671 [details]
> Bug 1433336 - Add nsIDocument::CollectDescendantDocument().
> 
> https://reviewboard.mozilla.org/r/215808/#review221612
> 
> ::: dom/base/nsIDocument.h:1920
> (Diff revision 1)
> >    typedef bool (*nsSubDocEnumFunc)(nsIDocument *aDocument, void *aData);
> >    virtual void EnumerateSubDocuments(nsSubDocEnumFunc aCallback,
> >                                       void *aData) = 0;
> >  
> >    /**
> > +   * Collect descendant documents that the |aCalback| returns true for the
> 
> Maybe: Collect all the descendant documents for which |aCallback| returns
> true.

Thanks. :)

Comment 8

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8945670 [details]
Bug 1433336 - Drop unused SubDocEnumArgs in nsDocument.cpp.

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

Comment 9

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8945671 [details]
Bug 1433336 - Add nsIDocument::CollectDescendantDocument().

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

Comment 10

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8945672 [details]
Bug 1433336 - Collect only documents which are observerd by IntersectionObserver in nsRefreshDriver::Tick().

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

Comment 11

[Hiroyuki Ikezoe (:hiro)]

A final try;
https://treeherder.mozilla.org/#/jobs?repo=try&revision=08ac90c040abdb6963a948929be742531b98dc58

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2284c3fb5ccb
Drop unused SubDocEnumArgs in nsDocument.cpp. r=smaug
https://hg.mozilla.org/integration/autoland/rev/10a71ed46d4d
Add nsIDocument::CollectDescendantDocument(). r=smaug
https://hg.mozilla.org/integration/autoland/rev/a7b31b61e096
Collect only documents which are observerd by IntersectionObserver in nsRefreshDriver::Tick(). r=smaug

Comment 13

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/2284c3fb5ccb
https://hg.mozilla.org/mozilla-central/rev/10a71ed46d4d
https://hg.mozilla.org/mozilla-central/rev/a7b31b61e096