1353529 - Crash when using IntersectionObserver in XUL pages

Status: RESOLVED FIXED

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

Description

[Tobias Schneider [:tobytailor]]

Using the IntersectionObserver API in XUL pages (e.g. in Add-Ons) causes crashes when trying to get to the root scrolling element.

STR:

Install the "Brief" feed reading extension from the Add-on marketplace.
Open the Mozilla Blog feed.
As of today, clicking on the feed entry with the title "A Public-Private Partnership for Gigabit Innovation and Internet Health" reproducibly causes a crash.

Crash reports:

https://crash-stats.mozilla.com/signature/?version=55.0a1&signature=mozilla%3A%3Adom%3A%3ADOMIntersectionObserver%3A%3AUpdate&date=%3E%3D2017-03-27T22%3A16%3A00.000Z&date=%3C2017-04-03T22%3A16%3A00.000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&_columns=install_time&_sort=-date&page=1#reports

Comment 1

[Tobias Schneider [:tobytailor]]

Attachment: Bug 1353529 - Crash when using IntersectionObserver in XUL pages.

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

Comment 2

[Markus Stange [:mstange]]

Comment on attachment 8854682 [details]
Bug 1353529 - Crash when using IntersectionObserver in XUL pages.

https://reviewboard.mozilla.org/r/126640/#review129192

::: dom/base/DOMIntersectionObserver.cpp:284
(Diff revision 1)
>        }
>      }
>    } else {
>      nsCOMPtr<nsIPresShell> presShell = aDocument->GetShell();
>      if (presShell) {
>        rootFrame = presShell->GetRootScrollFrame();

Couldn't this also return null?

Comment 3

[Tobias Schneider [:tobytailor]]

It would if directly used in a XUL only document. We don't support that and would pass the intersection test by checking for null value later in the file. Our issue here was that an HTML document was embedded in XUL via an iframe, so that call returned the iframe's root scroll element but crashed when trying to walk up the presContext chain.

Comment 4

[Carsten Book [:Tomcat]]

can't land this via autolander because of one open issue it seems. Can you take a look, thanks!

Comment 5

[Markus Stange [:mstange]]

I've dropped the issue, can you try again? Thanks.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a628de1a8a06
Crash when using IntersectionObserver in XUL pages. r=mstange

Comment 7

[Carsten Book [:Tomcat]]

(In reply to Markus Stange [:mstange] from comment #5)
> I've dropped the issue, can you try again? Thanks.

done

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/a628de1a8a06

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Please request Aurora approval on this when you get a chance. Might as well have it already landed by the time you're looking to re-enable there :)

Comment 10

[Tobias Schneider [:tobytailor]]

Comment on attachment 8854682 [details]
Bug 1353529 - Crash when using IntersectionObserver in XUL pages.

Approval Request Comment
[Feature/Bug causing the regression]: 1243846
[User impact if declined]: Crashes in popular RSS reader add-ons
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[Is the change risky?]: No

Comment 11

[Gerry Chang [:gchang]]

Comment on attachment 8854682 [details]
Bug 1353529 - Crash when using IntersectionObserver in XUL pages.

Fix a crash when using IntersectionObserver. Beta54+. Should be in 54 beta 6.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/3d786fe34df6