1361041 - Crash in nsIFrame::SetStyleContextWithoutNotification

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Marcia Knous [:marcia]]

This bug was filed from the Socorro interface and is 
report bp-a951f694-a2b5-4a3b-a565-2e3880170424.
=============================================================

Seen while looking at nightly crash stats - crashes started using 20170426030329: http://bit.ly/2pOwnHh. Crashes also present on ESR52

Possible regression range based on Build ID: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a30dc237c3a600a5231f2974fc2b85dfb5513414&tochange=0f5ba06c4c5959030a05cb852656d854065e2226

Comment 1

[Andrew McCreight [:mccr8]]

Any ideas, Emilio? You have a number of patches in that regression range. (This signature has persisted past when bug 1359995 landed.)

Comment 2

[Emilio Cobos Álvarez (:emilio)]

So at a glance it seems it could come from bug 1357142.

The call that's causing the crash seems to be [1], that is, we're trying to recreate frames for the root element, which should always have a frame created at the initialization of the PresShell.

So the function I removed in that bug had something like the following:

  if (!mDidInitialize) {
    // Nothing to do here.  In fact, if we proceed and aContent is the
    // root we will crash.
    return NS_OK;
  }

Which PostRecreateFramesFor doesn't guard against (because I thought it was not needed, per tryserver results).

Sounds a lot like we do need that check, though I'd like to see a testcase where it happens :(

Will post a patch soon.

[1]: http://searchfox.org/mozilla-central/rev/3dc6ceb42746ab40f1441e1e659ffb8f62ae78e3/layout/base/nsCSSFrameConstructor.cpp#2420

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1361041: Avoid posting ReconstructFrame hints to an uninitialized PresShell.

The call that's causing the crash seems to be [1], that is, we're trying to
recreate frames for the root element, which should always have a frame created
at the initialization of the PresShell.

So the function I removed in that bug had something like the following:

  if (!mDidInitialize) {
    // Nothing to do here.  In fact, if we proceed and aContent is the
    // root we will crash.
    return NS_OK;
  }

Which PostRecreateFramesFor doesn't guard against (because I thought it was not
needed, per tryserver results).

Sounds a lot like we do need that check, though I'd like to have a testcase
where it happens :(

[1]: http://searchfox.org/mozilla-central/rev/3dc6ceb42746ab40f1441e1e659ffb8f62ae78e3/layout/base/nsCSSFrameConstructor.cpp#2420

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

Comment 4

[Andrew McCreight [:mccr8]]

[Tracking Requested - why for this release]: new crash

Comment 5

[Emilio Cobos Álvarez (:emilio)]

The commit message needs a s/Recreate/Reconstruct/, but that should do it.

Thanks for the heads-up Andrew and Marcia :)

Comment 6

[Emilio Cobos Álvarez (:emilio)]

err, collision

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8863401 [details]
Bug 1361041: Avoid posting ReconstructFrame hints to an uninitialized PresShell.

https://reviewboard.mozilla.org/r/135172/#review138126

r=me

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8863401 [details]
Bug 1361041: Avoid posting ReconstructFrame hints to an uninitialized PresShell.

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

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b87e63c76691
Avoid posting ReconstructFrame hints to an uninitialized PresShell. r=bz

Comment 10

[Marcia Knous [:marcia]]

Tracking 55+ for this crash.

Comment 11

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

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