Closed Bug 1431282 Opened 2 years ago Closed 2 years ago

Reland the fix for bug 1402766 on beta 58

Categories

(Core :: Layout, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox57 --- unaffected
firefox58 --- fixed
firefox59 --- fixed

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

We've found another way to violate the invariants that bug 1402766 wallpapered over.  That wallpaper was reverted in bug 1405443, but we should reinstate it for now until we fix those invariant violations...
MozReview-Commit-ID: 3ggJI0qmOJV
Attachment #8943467 - Flags: review?(emilio)
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Comment on attachment 8943467 [details] [diff] [review]
Work around layout violating its own invariants and causing stylo code to crash, _again_

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

::: layout/generic/nsInlineFrame.cpp
@@ +988,5 @@
>      nsIFrame* nextInline = blockFrame->GetProperty(nsIFrame::IBSplitSibling());
> +
> +    // This check is here due to bug 1405443.  Please remove it once
> +    // that bug is fixed.
> +    if (!nextInline) {

nit: maybe MOZ_UNLIKELY?
Attachment #8943467 - Flags: review?(emilio) → review+
Attachment #8943467 - Attachment is obsolete: true
We're planning to build 58RC2 tomorrow, so you'll need to make sure Gerry is in the loop here.
Flags: needinfo?(gchang)
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/09d0f66b1eee
Work around layout violating its own invariants and causing stylo code to crash, _again_.  r=emilio
Comment on attachment 8943468 [details] [diff] [review]
Work around layout violating its own invariants and causing stylo code to crash, _again_

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1405443, effectively
[User impact if declined]: At the very least, ability to trigger null-deref
   crashes.  I _think_ it's not any worse than that.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: We're shipping code including this fix
  in 57.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: We're shipping this exact null-check in 57.
[String changes made/needed]:
Attachment #8943468 - Flags: approval-mozilla-beta?
Comment on attachment 8943468 [details] [diff] [review]
Work around layout violating its own invariants and causing stylo code to crash, _again_

Take this in 58 as we need this check here for bug 1431232.
Flags: needinfo?(gchang)
Attachment #8943468 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8943468 - Flags: approval-mozilla-release+
https://hg.mozilla.org/mozilla-central/rev/09d0f66b1eee
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Blocks: 1431232
You need to log in before you can comment on or make changes to this bug.