691215 - crash nsXMLPrettyPrinter::Unhook , I believe document.replaceChild (in bookmarklet) crashes FF 7.x

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P1)

Description

[J Del Monte]

Attachment: FF_fail.txt

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0.1) Gecko/20100101 Firefox/7.0.1
Build ID: 20110928134238

Steps to reproduce:

Tried using a bookmarklet that worked fine in FF 6.x


Actual results:

FF crashes (tried in both W2K and 64 bit Win 7)


Expected results:

Should have worked - returns a link from within the XML document.   I think it fails at: document.replaceChild(f,document.firstChild)

Comment 1

[Tim (fmdeveloper)]

Please provide a crash id or stacktrace -> https://developer.mozilla.org/en/How_to_get_a_stacktrace_for_a_bug_report

If you could reduce the test case to the bare minimum that would also help

Comment 2

[J Del Monte]

(In reply to Tim (fmdeveloper) from comment #1)
> Please provide a crash id or stacktrace ->
> https://developer.mozilla.org/en/How_to_get_a_stacktrace_for_a_bug_report
> 
> If you could reduce the test case to the bare minimum that would also help

Hopefully this is what you are looking for:
http://crash-stats.mozilla.com/report/index/bp-e5f85efa-050b-402e-ba2b-513b82111002

By test case do you mean the XML document or the bookmarklet?   The XML sample is what the portable ding returns so it is a true test case.   I didn't write the bookmarlket from heck so I cannot really trim it, sorry.

Comment 3

[Alice0775 White]

bp-30247a53-b238-421d-b562-680b12111002


Regression Window(cached m-c hourly),
Works:
http://hg.mozilla.org/mozilla-central/rev/5c6d107ede5a
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0a1) Gecko/20110601 Firefox/7.0a1 ID:20110601030746
Crashes:
http://hg.mozilla.org/mozilla-central/rev/840644b2b6a2
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0a1) Gecko/20110601 Firefox/7.0a1 ID:20110601011530
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5c6d107ede5a&tochange=840644b2b6a2

Triggered by:
Bug 657353 - Stop sending BeginUpdate/EndUpdate on content state changes

Comment 4

[Alice0775 White]

Attachment: xml

Comment 5

[Alice0775 White]

[str]
1．open attachment 564100 [details]
2.the following bookmarklet should be bookmarked,
3.open the bookmarklet.

[actual]
browser crashes

javascript:(function(){function%20j(b,a,c){typeof%20c==="undefined"&&(c=document);typeof%20a==="undefined"&&(a=0);b=c.getElementsByTagName(b);return%20b.length<a+1?"":b[a].firstChild.data}function%20h(b,a,c){return{attrName:b,attrVal:a,text:c}}function%20g(b){var%20a=document.createElementNS("http://www.w3.org/1999/xhtml",b),c;arguments.length>1&&(c=arguments[1],c.attrName.length&&c.attrVal.length&&a.setAttribute(c.attrName,c.attrVal),c.text.length&&a.appendChild(document.createTextNode(c.text)));return%20a}function%20i(b){for(var%20a=%20b.length-1;a>0;a--)b[a-1].appendChild(b[a])}(function(){for(var%20b=void%200,a=void%200,c=void%200,b=b?b:a?a.ownerDocument:document,f=[],a=b.evaluate("//headline",a?a:b,c?c:null,XPathResult.ORDERED_NODE_SNAPSHOT_TYPE,null),b=0;b<a.snapshotLength;b++)f[b]=a.snapshotItem(b);var%20d,e,k,l;if(f.length===0)alert("No%20messages!");else{e=window.location.toString().replace("getMessages","createSession");window.XMLHttpRequest?(xhttp=new%20XMLHttpRequest,xhttp.open("GET",e,!1),xhttp.send(""),e=xhttp.responseXML):(alert("Sorry,%20your%20browswer%20can't%20handle%20this!"),%20e=!1);j("status",0,e)!=="0"?alert("Unable%20to%20create%20session;%20verify%20installId%20and%20cksum"):d=j("disc",0,e);b="&disc="+d;e=g("ul",h("style","margin:2em;",""),"");for(d=0;d<f.length;d++)a=j("activationText",0,f[d]).replace(/(<([^>]+)>)/ig,""),c=j("activationUrl",0,f[d]),c.length>0?(j("type",0,f[d])==="offer"?(c+=b,k=j("activationNewDelta",0,f[d])*1,l=g("span",h("style","margin-left:2em;%20font-size:80%;","Active%20in%20"+(k/60).toFixed(0)+":"+((k%%2060)/100).toFixed(2)*100))):k=0,i([e,g("li"),g("a",h("href",c,%20a))]),k>0&&i([e.lastChild,l])):i([e,g("li",h("","",a))])}f=g("div");i([f,e]);l=g("html:div",h("style","margin:%202.5em%205em;%20color:%20red;%20font-size:%2090%;%20font-style:%20italic;",""));e=g("p",h("style","padding:%205px;",""));d=g("span",h("style","border:%201px%20solid%20black;%20padding:2px;","Caution:%20You%20are%20using%20Beta%20v0.1%20of%20this%20script.%20Be%20sure%20to%20"));d.appendChild(g("a",h("href","http://www.flyertalk.com/forum/16382603-post8.html","check%20periodically")));d.appendChild(document.createTextNode("%20for%20an%20updated%20version."));%20i([l,e,d]);i([f,l]);i([f,g("p",h("style","font-size:80%;%20margin:2em;",document.lastModified))]);document.replaceChild(f,document.firstChild)})()})();

Comment 6

[Alice0775 White]

Sorry the above bookmarklet fails to run due to cut and paste problem or something.

Pls use bookmarklet described in attachment 564094 [details]

Comment 7

[Boris Zbarsky [:bzbarsky]]

So mDocument is null in nsXMLPrettyPrinter::Unhook.

What happens is that the replaceChild call triggers a remove and an insert which both call MaybeUnhook.  So we get two runnables to unhook posted, and then the second one of those to run loses.

And the reason we get two runnables posted is because the check is like so:

    if (!aContent || !aContent->GetBindingParent() && !mUnhookPending) {

which is clearly bogus; it tests true any time !aContent, which is the case here because aContent is the aContainer from ContentInserted/ContentRemoved, which is null for replacement of the document element.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix stupid logic error in the XML prettyprinter that can cause a null-dereference crash.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Thank you all for the clear steps to reproduce!

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 564109 [details] [diff] [review]
Fix stupid logic error in the XML prettyprinter that can cause a null-dereference crash.

Thanks!

Comment 11

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5a26e1aa86e5

Comment 12

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 564109 [details] [diff] [review]
Fix stupid logic error in the XML prettyprinter that can cause a null-dereference crash.

It may be worth taking this on aurora.  This is a very safe change that fixes a null-deref crash that web content can actually trigger pretty easily (see the mochitest).

On the other hand, I expect our frequency for this crash is pretty low...

Comment 13

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/mozilla-central/rev/5a26e1aa86e5

Comment 14

[LegNeato]

Comment on attachment 564109 [details] [diff] [review]
Fix stupid logic error in the XML prettyprinter that can cause a null-dereference crash.

Crash level is very, very low. We'll let this come up in the normal process.