1370968 - Crash in nsLayoutUtils::FindNearestCommonAncestorFrame

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Calixte Denizet (:calixte)]

This bug was filed from the Socorro interface and is 
report bp-489abd7b-dce1-4fed-a01e-d1ca20170607.
=============================================================

There is 2 crashes (from the same installation) in nightly 55 with buildid  	20170607030206. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1359318.

[1] https://hg.mozilla.org/mozilla-central/rev?node=279a08ffe577ec62a6b9d5ca76fe25bd117dd2b3

Comment 1

[Tobias Schneider [:tobytailor]]

Attachment: Crashtest

Comment 2

[Tobias Schneider [:tobytailor]]

Attachment: Always make sure we have a scroll root frame in case of being used in a XUL page

Comment 3

[Tobias Schneider [:tobytailor]]

Attachment: Guard against possible NULL values when IntersecionObserver API is used in XUL pages

Comment 4

[Tobias Schneider [:tobytailor]]

Problem here was that a target within an XUL page, loaded via an iframe, was observed and the code to calculate the intersection expected the target having a root scroll frame. We should guard properly against any cases of NULL when using this API within a XUL context.

Comment 5

[Markus Stange [:mstange]]

Comment on attachment 8875477 [details] [diff] [review]
Guard against possible NULL values when IntersecionObserver API is used in XUL pages

Review of attachment 8875477 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/DOMIntersectionObserver.cpp
@@ +417,5 @@
> +        if (presContext) {
> +          nsCOMPtr<nsIPresShell> presShell = presContext->PresShell();
> +          if (presShell) {
> +            nsIFrame* rootScrollFrame = presShell->GetRootScrollFrame();
> +            if (rootScrollFrame) {

The rootScrollFrame check should be the only one that's needed. A frame always has a PresContext and a PresContext always has a PresShell. (Except maybe during frame / prescontext destruction.)

The general rule is: Getters that start with the word "Get" can return null, those that don't can not.

Comment 6

[Tobias Schneider [:tobytailor]]

Attachment: Guard against possible NULL values when IntersecionObserver API is used in XUL pages

Addressed review comment.

Comment 7

[Tobias Schneider [:tobytailor]]

Attachment: Crashtest

Comment 8

[Tobias Schneider [:tobytailor]]

Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9ee18f866530165799b9c2be93d744fe1eb96985

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/217c7653e6ec
Guard against possible NULL values when IntersecionObserver API is used in XUL pages. r=mstange
https://hg.mozilla.org/integration/mozilla-inbound/rev/5c606fafb195
Test for crash in nsLayoutUtils::FindNearestCommonAncestorFrame. r=mstange

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/217c7653e6ec
https://hg.mozilla.org/mozilla-central/rev/5c606fafb195