The default bug view has changed. See this FAQ.

crash in nsStyleSet::ReparentStyleContext when clicking / hovering NoScript icon in Add-on Toolbar

RESOLVED FIXED in mozilla16

Status

()

Core
Layout
--
critical
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: dholbert, Assigned: bz)

Tracking

({crash, regression, topcrash})

16 Branch
mozilla16
crash, regression, topcrash
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox16-)

Details

(Whiteboard: [workaround: View|Toolbars|Customize, then move NoScript icon to navbar], crash signature)

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
STR:
 1. Install NoScript from http://noscript.net/getit.
       (Restart Firefox to complete installation.)

 2. "View | Toolbars | Add-on Bar" to make Add-on Bar visible.
 3. "View | Toolbars | Customize"
 4. Drag NoScript icon from URLbar area to Add-on Bar; then, close "Customize" dialog.
 5. Click NoScript icon in Add-on Bar.

Actual results: CRASH

This is new in today's m-c nightly.

m-c mozregression range:
http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2012-06-12&enddate=2012-06-13

m-i mozregression range:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=f08886a8cf22&tochange=4a4519b018e9

This bug was filed from the Socorro interface and is 
report bp-6114342e-2eb8-4591-a0f9-850542120613 .
=============================================================
(Reporter)

Comment 1

5 years ago
At first glance, this cset for bug 763424 looks like the most related-looking layout change in the m-i regression range:
  http://hg.mozilla.org/integration/mozilla-inbound/rev/f4ff27e63666
(touches nsFrameManager::ReResolveStyleContext, which occurs multiple times in the crash stack)

Tentatively blaming that, to start with.
(Reporter)

Updated

5 years ago
Blocks: 763424
(Reporter)

Comment 2

5 years ago
Quoting Scoobidiver from bug 724235 comment #1:
> There's a spike in crashes (about 50 crashes per hour!) starting in
> 16.0a1/20120613.

Updated

5 years ago
Keywords: topcrash
OS: Linux → All
(Reporter)

Comment 3

5 years ago
OK, I think bug 763424 is innocent -- I backed it out locally, and I'm still crashing.
No longer blocks: 763424
(Reporter)

Comment 4

5 years ago
In fact, it's a regression from another bug in that same push -- bug 736695.

I backed out its 3 csets (the top 3 in http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=f4ff27e63666 ) and that fixes this crash.
Blocks: 736695
(Reporter)

Comment 5

5 years ago
(I backed them out locally, I mean)
(Reporter)

Comment 6

5 years ago
To be more specific: this is fixed by a local backout of the final cset from bug 736695:
 http://hg.mozilla.org/integration/mozilla-inbound/rev/ca716959ca2b
Yeah, the final cset is the dangerous one.  The previous ones were basically unobservable (until the final cset) refactoring.

I'll look into this.

Updated

5 years ago
Blocks: 724235

Updated

5 years ago
Crash Signature: [@ nsStyleSet::ReparentStyleContext] → [@ nsStyleSet::ReparentStyleContext(nsStyleContext*, nsStyleContext*, mozilla::dom::Element*) ] [@ nsStyleSet::ReparentStyleContext]
tracking-firefox16: --- → ?
Keywords: regression
Version: Trunk → 16 Branch

Comment 8

5 years ago
Side note for NoScript users coming here after a crash: as a work-around you can just use the contextual "Customize" menu on the add-on bar and move the NoScript icon to the navigation bar.

Updated

5 years ago
Crash Signature: [@ nsStyleSet::ReparentStyleContext(nsStyleContext*, nsStyleContext*, mozilla::dom::Element*) ] [@ nsStyleSet::ReparentStyleContext] → [@ nsStyleSet::ReparentStyleContext(nsStyleContext*, nsStyleContext*, mozilla::dom::Element*)] [@ nsStyleSet::ReparentStyleContext]

Comment 9

5 years ago
(In reply to Giorgio Maone from comment #8)
> Side note for NoScript users coming here after a crash: as a work-around you
> can just use the contextual "Customize" menu on the add-on bar and move the
> NoScript icon to the navigation bar.

hmm. somehow i got this crash today while looking at noscript's context-menu.
crashreport: b76a4e04-a32f-4596-93bb-a37f92120614
(Reporter)

Updated

5 years ago
Whiteboard: [workaround: View|Toolbars|Customize, then move NoScript icon to navbar]

Comment 10

5 years ago
A crash also occurs when using the right-click menu (Right click -> NoScript).

Comment 11

5 years ago
(In reply to Daniel Holbert [:dholbert] from comment #4)
> In fact, it's a regression from another bug in that same push -- bug 736695.
This bug is ten times crashier than bug 736695. Can someone backout the culprit changeset if nobody has a quick fix?
I triggered this several times hovering the NoScript add-on bar icon, but I had the same crash twice without going close to the add-on bar tonight, trying to load the Facebook app My Calendar - Birthdays:

http://crash-stats.mozilla.com/report/index/bp-7b83c4cf-0484-4b91-829d-572b42120615
http://crash-stats.mozilla.com/report/index/bp-2430093d-c330-4603-8dcf-32c842120615

Thanks for the workaround, Giorgio.
For what it's worth, I haven't gotten this to crash yet....  I do see some assertions, but no crash.  I guess I'll aim to fix the assertions and hope that fixes the crash for people.
So the first thing that goes wrong is that a timer fires ["chrome://noscript/content/noscriptOverlay.js":98] and calls some script ["chrome://noscript/content/noscriptOverlay.js":38] which calls openPopup on a popup boxobject.  This fires a popupshowing event, which runs more script ["chrome://noscript/content/noscriptOverlay.js":76] which inserts a <xul:menuitem> ["chrome://noscript/content/noscriptOverlay.js":912].

We go to construct frames for this <xul:menuitem>, find an XBL-bound kid under it, but that kid thinks it's not in a document.  That's the first assertion...
Then there are other asserts about primary frames being confused, etc....
Fwiw, I would dearly love a smaller testcase that still shows the problem.  That would make this a heck of a lot easier to debug...  Right now I'm getting dozens of calls to the code involved every time I hover over the button.  :(
Aha!  Looking at the noscript code links from comment 14, there's a document fragment involved!
So I bet that's it.  The removals from the document fragment happen under the insertion scriptblocker, so the binding teardowns happen async _after_ all those nodes are inserted into the document, which is bad.
Created attachment 633625 [details] [diff] [review]
Handle removals of kids of document fragments being inserted under a separate scriptblocker, just like we do with the single-node insert case, so their XBL binding teardown happens before insert.

Well, this fixes the assertions for me.  Can someone who is seeing crashes give this a shot?  There should be try builds at https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/bzbarsky@mozilla.com-cced24ceafe2/ at some point later today...
Attachment #633625 - Flags: review?(bugs)
Assignee: nobody → bzbarsky
Whiteboard: [workaround: View|Toolbars|Customize, then move NoScript icon to navbar] → [need review][workaround: View|Toolbars|Customize, then move NoScript icon to navbar]
(Reporter)

Comment 20

5 years ago
(In reply to Boris Zbarsky (:bz) from comment #19)
> Well, this fixes the assertions for me.  Can someone who is seeing crashes
> give this a shot?

Nice! This fixes all assertions & crashes in my local debug build.  Tested w/ icon in NavBar &  Add-on bar, and also tested the NoScript submenu in page-rightclick-contextmenu.  No assertions or crashes.
Comment on attachment 633625 [details] [diff] [review]
Handle removals of kids of document fragments being inserted under a separate scriptblocker, just like we do with the single-node insert case, so their XBL binding teardown happens before insert.


>@@ -4348,63 +4450,42 @@ nsINode::ReplaceOrInsertBefore(bool aRep
>   }
> 
>   /*
>    * Check if we're inserting a document fragment. If we are, we need
>    * to remove the children of the document fragment and add them
>    * individually (i.e. we don't add the actual document fragment).
Fix this comment, since the children have been removed already.
Attachment #633625 - Flags: review?(bugs) → review+
http://hg.mozilla.org/integration/mozilla-inbound/rev/d8bc9c5cdc4a with that comment fixed.
Flags: in-testsuite?
Whiteboard: [need review][workaround: View|Toolbars|Customize, then move NoScript icon to navbar] → [workaround: View|Toolbars|Customize, then move NoScript icon to navbar]
Target Milestone: --- → mozilla16

Comment 23

5 years ago
Just for your amusement, this is roughly 50% of all crashes on Nightly yesterday.
Which just goes to show that we should remove support for XBL destructors, preferably yesterday...
I can reproduce today using Windows 7 and Nightly. The workaround didn't work for me.

Crash report:
https://crash-stats.mozilla.com/report/index/bp-01343147-3d5d-4601-8230-1d9492120615
https://hg.mozilla.org/mozilla-central/rev/d8bc9c5cdc4a
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Reporter)

Updated

5 years ago
Duplicate of this bug: 765502
(Reporter)

Updated

5 years ago
Summary: crash in nsStyleSet::ReparentStyleContext when clicking NoScript icon in Add-on Toolbar → crash in nsStyleSet::ReparentStyleContext when clicking / hovering NoScript icon in Add-on Toolbar

Updated

5 years ago
Duplicate of this bug: 765502

Updated

5 years ago
tracking-firefox16: ? → -
You need to log in before you can comment on or make changes to this bug.