722101 - Firefox 12 crashes due to some activity in chrome nodes created with an instance of nsIDOMParser

Status: RESOLVED WORKSFORME

(Core :: JavaScript Engine, defect)

Description

[Dimitri Saltanov]

User Agent: Mozilla/5.0 (Windows NT 5.1; rv:9.0.1) Gecko/20100101 Firefox/9.0.1
Build ID: 20111220165912

Steps to reproduce:

Mozilla/5.0 (Windows NT 5.1; rv:12.0a1) Gecko/20120128 Firefox/12.0a1 ID:20120128031108

Firefox 12 will crash almost immediately after nodes are adopted and inserted into chrome that were created with an instance of the xmlextras nsIDOMParser, where it worked fine in earlier versions of Firefox.

A simple test case:

1) use the following code for a restartless addon:

/* begin bootstrap.js */
Components.utils.import('resource://gre/modules/Services.jsm');
const { classes: Cc, interfaces: Ci, results: Cr }= Components;
function startup()
  {
    let parser= Cc['@mozilla.org/xmlextras/domparser;1'].createInstance(Ci.nsIDOMParser);
    parser.init(null, null, null);
    let xul= '<label xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" value="BANG!"/>';
    let node= parser.parseFromString(xul, 'text/xml').documentElement;
    let document= Services.wm.getMostRecentWindow('navigator:browser').document;
    document.querySelector('#toolbar-menubar').appendChild(document.adoptNode(node));
  };
function shutdown() undefined;
/* end bootstrap.js */

2) manually enable the test add-on, making 'BANG!' appear next to Firefox menu in the browser window (just make sure to enable add-on explicitly from Addons Manager, since the simple code above may fail to execute as intended when Firefox starts up, due lack of any browser windows)

3) if Firefox is still running, move mouse around the browser window a little bit, it should bring it down for sure (note that the nodes in question do get inserted and displayed properly in chrome, but then crash due to some DOM activity)

This behaviour is regular and the bug is also reported by users of my chromeless addons that have no such problem with Firefox 4 through 11.

I was unsure whether to file this bug in XML or DOM.

Comment 1

[Scoobidiver (away)]

Can you provide the crash ID (bp-...) from about:crashes?

Comment 2

[Dimitri Saltanov]

(In reply to Scoobidiver from comment #1)
> Can you provide the crash ID (bp-...) from about:crashes?

bp-6cd840d4-6755-4d26-90ba-4b66e2120129

Comment 3

[Scoobidiver (away)]

Can you do your test with a new profile (see http://support.mozilla.org/en-US/kb/Managing-profiles) because I see many extensions in you crash report?

Comment 4

[Dimitri Saltanov]

(In reply to Scoobidiver from comment #3)
> Can you do your test with a new profile (see
> http://support.mozilla.org/en-US/kb/Managing-profiles) because I see many
> extensions in you crash report?

Sure can, here you are:
bp-06125e8c-4c22-4213-915f-a8c492120129

FYI, seeing that the crash is always coming from garbage collector, I just tried importNode instead of adoptNode out of curiosity so the node is not deleted from the donor document, and it seems to solve the crash.

Comment 5

[Dimitri Saltanov]

FYI also, when using parsers created with a built-in DOMParser() constructor located in a chrome window, e.g.

parser= new (Services.wm.getMostRecentWindow('navigator:browser').DOMParser)(); 

instead of one directly instantiated from XPCOM as in the test case, then the crash does not seem to happen after adoptNode (even if the target document is located in another window or dialog). I wonder what's the difference.

Comment 6

[Dimitri Saltanov]

[undoing accidental change, sorry]

Comment 7

[Boris Zbarsky [:bzbarsky]]

This sounds like a wrapper reparenting issue.  I wonder what compartment stuff is created in when the DOMParser is created via contractid... Presumably the component's compartment, right?  And then we have to reparent into the window's compartment and whatnot?

Dimitri, can you find when this problem first appeared in nightlies, perhaps?

Comment 8

[Dimitri Saltanov]

(In reply to Boris Zbarsky (:bz) from comment #7)

Not later than Jan 13 according to records, but AFAICR it was much earlier, though not necessarily in the very initial builds of FF12. FF11 never showed this problem.

Comment 9

[Dimitri Saltanov]

PS Compatibility reports for one of my addons also show a user problem with FF12 on Jan 8, but without any detail I'm not sure it would be reliable to relate it to this bug.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Dimitri, I was thinking more like using http://harthur.github.com/mozregression/ to pin down the exact changesets of the last nightly without the problem and first one with the problem.

Comment 11

[Dimitri Saltanov]

(In reply to Boris Zbarsky (:bz) from comment #10)

2012-01-06 was good
2012-01-07 was bad

Comment 12

[Boris Zbarsky [:bzbarsky]]

What are the hg changeset urls for those in about:buildconfig?

Comment 13

[Scoobidiver (away)]

Here is the regression window:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8ae16e346bd0&tochange=5a446202be5f

Comment 14

[Boris Zbarsky [:bzbarsky]]

Hmm.  No obvious dom/xpconnect changes in there.  Some jseng changes, though...

Comment 15

[Alex Keybl [:akeybl]]

Tracking for FF12 because this crash may spike once we have more testing coverage (since add-ons can be implicated).

Comment 16

[David Mandelin [:dmandelin]]

(In reply to Boris Zbarsky (:bz) from comment #14)
> Hmm.  No obvious dom/xpconnect changes in there.  Some jseng changes,
> though...

Bug 712714 is in there, which was a big one. Dimitri, are you available to run some test builds that we can prepare to narrow down the problem?

Comment 17

[David Mandelin [:dmandelin]]

(In reply to David Mandelin from comment #16)
> (In reply to Boris Zbarsky (:bz) from comment #14)
> > Hmm.  No obvious dom/xpconnect changes in there.  Some jseng changes,
> > though...
> 
> Bug 712714 is in there, which was a big one. Dimitri, are you available to
> run some test builds that we can prepare to narrow down the problem?

Or, alternatively, could you provide me the addon you used with comment 0 to test the bug, so I can bisect further?

Comment 18

[Dimitri Saltanov]

(In reply to David Mandelin from comment #17)

Hmm... Just tested it in 11.0 and 13.0a1, and the bug seems to be gone! I'll attach a bug-test addon nevertheless.

Comment 19

[Dimitri Saltanov]

Attachment: bug test addon

Comment 20

[David Mandelin [:dmandelin]]

The regressing changeset:

changeset:   83883:faf5f8842fec
user:        Brian Hackett <[e-mail]>
date:        Thu Jan 05 06:38:46 2012 -0800
summary:     Don't modify dictionary shapes in place, bug 703157. r=luke

Comment 21

[David Mandelin [:dmandelin]]

Fixed by:

changeset:   87049:75eb6956410b
user:        Brian Hackett <[e-mail]>
date:        Thu Feb 16 15:15:16 2012 -0800
summary:     Fix dictionary state when swapping objects, bug 727330. r=luke

I verified that this changeset is present on mozilla-central, mozilla-aurora, and mozilla-beta. On beta it is 935e8d973a75.