Closed Bug 1370968 Opened 2 years ago Closed 2 years ago

Crash in nsLayoutUtils::FindNearestCommonAncestorFrame


(Core :: Layout, defect, critical)

55 Branch
Windows 10
Not set



Tracking Status
firefox-esr52 --- unaffected
firefox53 --- unaffected
firefox54 --- unaffected
firefox55 --- fixed


(Reporter: calixte, Assigned: tschneider)


(Blocks 1 open bug)


(Keywords: crash, regression, Whiteboard: [clouseau])

Crash Data


(2 files, 3 obsolete files)

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.

Flags: needinfo?(tschneider)
Attached patch Crashtest (obsolete) — Splinter Review
Flags: needinfo?(tschneider)
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.
Attachment #8875477 - Flags: review?(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.
Addressed review comment.
Attachment #8875477 - Attachment is obsolete: true
Attachment #8875477 - Flags: review?(mstange)
Attachment #8875492 - Flags: review?(mstange)
Attachment #8875492 - Flags: review?(mstange) → review+
Attached patch CrashtestSplinter Review
Attachment #8875435 - Attachment is obsolete: true
Keywords: checkin-needed
Assignee: nobody → tschneider
Flags: in-testsuite+
Pushed by
Guard against possible NULL values when IntersecionObserver API is used in XUL pages. r=mstange
Test for crash in nsLayoutUtils::FindNearestCommonAncestorFrame. r=mstange
Keywords: checkin-needed
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.