1137723 - crash in nsIFrame::SetParent(nsContainerFrame*)

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

This bug was filed from the Socorro interface and is 
report bp-6cfa7f95-68ac-493b-a109-855bb2150227.
=============================================================

Firefox 36.0 and Firefox Nightly 39.0a1 20150227 on Windows 8.1 64 bit.

Steps to reproduce:
1. Open http://www.stuttgarter-zeitung.de/inhalt.griechenland-hilfe-bundestag-stimmt-verlaengerung-zu.daa3baae-e17d-47e8-ad28-62eece0b4cfa.html
2. Go to menu File > Print Preview.
Actual result: Crash

about:support info from a new 36.0 profile:

Application Basics
------------------

Name: Firefox
Version: 36.0
User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:36.0) Gecko/20100101 Firefox/36.0
Multiprocess Windows: 0/1

Extensions
----------

Graphics
--------

Adapter Description: Intel(R) HD Graphics 4600
Adapter Description (GPU #2): NVIDIA GeForce GTX 765M
Adapter Drivers: igdumdim64 igd10iumd64 igd10iumd64 igdumdim32 igd10iumd32 igd10iumd32
Adapter Drivers (GPU #2): nvd3dumx,nvwgf2umx,nvwgf2umx nvd3dum,nvwgf2um,nvwgf2um
Adapter RAM: Unknown
Adapter RAM (GPU #2): 2048
Device ID: 0x0416
Device ID (GPU #2): 0x11e2
Direct2D Enabled: true
DirectWrite Enabled: true (6.3.9600.17415)
Driver Date: 9-29-2014
Driver Date (GPU #2): 2-5-2015
Driver Version: 10.18.10.3960
Driver Version (GPU #2): 9.18.13.4752
GPU #2 Active: false
GPU Accelerated Windows: 1/1 Direct3D 11 (OMTC)
Subsys ID: 35371558
Subsys ID (GPU #2): 35371558
Vendor ID: 0x8086
Vendor ID (GPU #2): 0x10de
WebGL Renderer: Google Inc. -- ANGLE (Intel(R) HD Graphics 4600 Direct3D9Ex vs_3_0 ps_3_0)
windowLayerManagerRemote: true
AzureCanvasBackend: direct2d
AzureContentBackend: direct2d
AzureFallbackCanvasBackend: cairo
AzureSkiaAccelerated: 0

Important Modified Preferences
------------------------------

browser.cache.disk.capacity: 296960
browser.cache.disk.smart_size.first_run: false
browser.cache.frecency_experiment: 4
browser.places.smartBookmarksVersion: 7
browser.startup.homepage_override.buildID: 20150222232811
browser.startup.homepage_override.mstone: 36.0
extensions.lastAppVersion: 36.0
gfx.direct3d.last_used_feature_level_idx: 0
media.gmp-gmpopenh264.lastUpdate: 1425050242
media.gmp-gmpopenh264.version: 1.3
media.gmp-manager.lastCheck: 1425050242
network.cookie.prefsMigrated: true
places.history.expiration.transient_current_max_pages: 30683
plugin.disable_full_page_plugin_for_types: application/pdf
plugin.importedState: true
privacy.sanitize.migrateFx3Prefs: true

Important Locked Preferences
----------------------------

JavaScript
----------

Incremental GC: true

Accessibility
-------------

Activated: false
Prevent Accessibility: 0

Library Versions
----------------

NSPR
Expected minimum version: 4.10.7
Version in use: 4.10.7

NSS
Expected minimum version: 3.17.4 Basic ECC
Version in use: 3.17.4 Basic ECC

NSSSMIME
Expected minimum version: 3.17.4 Basic ECC
Version in use: 3.17.4 Basic ECC

NSSSSL
Expected minimum version: 3.17.4 Basic ECC
Version in use: 3.17.4 Basic ECC

NSSUTIL
Expected minimum version: 3.17.4
Version in use: 3.17.4

Experimental Features
---------------------

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Is this reproducible with Fx 35?

Comment 2

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Yes, also crashes with 35.0.1.

Comment 3

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: testcase.html, v1

This testcase of 3kB still can reproduce the issue. Removing nodes or style rules often stops the crash. Also shortening the url at the bottom (the Facebook one) will end the crashes.

Comment 4

[Mats Palmgren (inactive)]

Attachment: Testcase #2

The first testcase didn't crash for me (on Linux) but this does.
Using "US Letter" and Portrait mode in Print Preview.

Comment 5

[Mats Palmgren (inactive)]

(gdb) bt
#0  0x00007ffff0a7c428 in nsIFrame::IsBoxFrame
#1  0x00007ffff0b678d7 in IsBoxWrapped
#2  0x00007ffff0b81c90 in nsIFrame::SetParent
#3  0x00007ffff0bc10b6 in nsInlineFrame::DrainSelfOverflowListInternal
#4  0x00007ffff0bc0f04 in nsInlineFrame::DestroyFrom
...
(gdb) up
#1  0x00007ffff0b678d7 in IsBoxWrapped (aFrame=0x7fffb6ee3f10) at layout/generic/nsFrame.cpp:177
177              aFrame->GetParent()->IsBoxFrame() &&
(gdb) list
173     static bool
174     IsBoxWrapped(const nsIFrame* aFrame)
175     {
176       return aFrame->GetParent() &&
177              aFrame->GetParent()->IsBoxFrame() &&
178              !aFrame->IsBoxFrame();
179     }
(gdb) up
#2  0x00007ffff0b81c90 in nsIFrame::SetParent (this=0x7fffb6ee3f10, aParent=0x7fffb837f008) at layout/generic/nsFrame.cpp:8714
8714      bool wasBoxWrapped = ::IsBoxWrapped(this);
(gdb) list
8711    void
8712    nsIFrame::SetParent(nsContainerFrame* aParent)
8713    {
8714      bool wasBoxWrapped = ::IsBoxWrapped(this);
8715      mParent = aParent;
8716      if (!wasBoxWrapped && ::IsBoxWrapped(this)) {
8717        ::InitBoxMetrics(this, true);
8718      } else if (wasBoxWrapped && !::IsBoxWrapped(this)) {

nsIFrame::SetParent is trying to figure out if we were IsBoxWrapped
with the old parent, and if we will be with the new and then init
or delete the BoxMetricsProperty accordingly.  The problem is that,
due to our inline lazy re-parenting optimization, the current mParent
(on line 8714) might be stale at this point so IsBoxWrapped() crashed
trying to use it.

Comment 6

[Mats Palmgren (inactive)]

Attachment: fix+test

(see the reason for the crash explained in the comment above)

I don't think this code needs to be this smart.  We can call InitBoxMetrics
with the *new* parent if we're IsBoxWrapped, otherwise just ignore that we
may have an existing BoxMetricsProperty, because that should be very rare
(unless we're going to or from mParent=null, it might not happen at all).

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e6011fb5bada
https://treeherder.mozilla.org/#/jobs?repo=try&revision=27be84b59c09

The testcases still generate some known Style System assertions,
bugs are filed.

Comment 7

[Mats Palmgren (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/acc733decf7a

Comment 8

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

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

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Verified fixed with Firefox Nightly 39.0a1 20150306030229 on Windows 8.1