552663 - Crash [@ nsContentUtils::ComparePosition]

Status: RESOLVED FIXED

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

Description

[timeless]

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000000
0x000000010068db2f in nsContentUtils::ComparePosition (aNode1=0x132491d00, aNode2=0x0) at /Users/timeless/devel/mozilla.org/mozilla-central/content/base/src/nsContentUtils.cpp:1575
1575	  if (aNode2->IsNodeOfType(nsINode::eATTRIBUTE)) {
(gdb) bt 10
#0 nsContentUtils::ComparePosition (aNode1=0x132491d00, aNode2=0x0) at nsContentUtils.cpp:1575
#1 nsContentUtils::PositionIsBefore (aNode1=0x132491d00, aNode2=0x0) at nsContentUtils.h:284
#2 nsContentList::ContentAppended (this=0x12efad510, aDocument=0x138994e00, aContainer=0x131664830, aNewIndexInContainer=27) at nsContentList.cpp:672
#3 nsNodeUtils::ContentAppended (aContainer=0x131664830, aNewIndexInContainer=27) at nsNodeUtils.cpp:135
#4 nsGenericElement::doInsertChildAt (aKid=0x12f047140, aIndex=27, aNotify=1, aParent=0x131664830, aDocument=0x138994e00, aChildArray=@0x131664870) at nsGenericElement.cpp:3304
#5 nsGenericElement::InsertChildAt (this=0x131664830, aKid=0x12f047140, aIndex=27, aNotify=1) at nsGenericElement.cpp:3235
#6 nsGenericElement::doReplaceOrInsertBefore (aReplace=0, aNewChild=0x12f047188, aRefChild=0x0, aParent=0x131664830, aDocument=0x138994e00, aReturn=0x7fff5fbfbcc0) at nsGenericElement.cpp:3993
#7 nsGenericElement::InsertBefore (this=0x131664830, aNewChild=0x12f047188, aRefChild=0x0, aReturn=0x7fff5fbfbcc0) at nsGenericElement.cpp:3529
#8 nsHTMLHeadElement::InsertBefore (this=0x131664830, newChild=0x12f047188, refChild=0x0, _retval=0x7fff5fbfbcc0) at nsHTMLHeadElement.cpp:56
#9 nsGenericElement::AppendChild (this=0x131664830, aNewChild=0x12f047188, aReturn=0x7fff5fbfbcc0) at nsGenericElement.h:503

Comment 1

[timeless]

so. i have the following extensions:

DOM Inspector
Google Gears [incompatible, disabled]
NoScript
Nightly Tester Tools
Evernote Web Clipper [incompatible?, disabled]
JavaScript Debugger

javascript debugger is set to debug chrome, and trace exceptions. I suspect the exception it's hitting comes from noscript, because smaug can't trigger this w/ just venkman.

all i do is:
open Minefield, open Venkman, restore my session (maps.google.com), and drag the map content around a bit (or a lot), sometimes double clicking. my map view starts in USA, but i generally drag to just west of France/England.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

This was a bit tricky to debug.

So this is caused by the fact that JSD enabled scripts at unexpected time, and
even so that JS timeouts get run.
The other cases when the method is used, the latter parameter is PR_FALSE.

Note, SetScriptsEnabled isn't used this ways elsewhere except in JSD.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 434225 [details] [diff] [review]
patch

Oops, wrong patch

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Note, ::RunTimeout is safe for GlobalWindow being frozen etc.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

The whole SetScriptsEnabled is a bit strange method and should be probably
removed. But in a different bug.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

And note, JSD doesn't seem to suspend timeouts properly even without the patch, 
so nothing should break there.

I could file a followup to make SetScriptsEnabled actually suspend
timeouts when needed.

Comment 8

[timeless]

afaict that code was introduced or broken by bug 116834. rginda was around in the bug but it doesn't look like he reviewed the jsd stuff. oh well. let's fix that in a new bug.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

The code isn't broken after all. It is just a bit strange.
RunTimeout stops running timeouts if scripts are disabled.

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

http://hg.mozilla.org/mozilla-central/rev/fbe66466ad9e

Comment 11

[Jesse Ruderman]

bp-66ee1ec2-0502-435f-804b-2bccb2100520 might be this bug.  Brian Krebs hit it while using the facebook privacy bookmarklet.  Should we backport this patch to 1.9.2?

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 434226 [details] [diff] [review]
patch

We could take the patch to 1.9.2, but I'm not at all sure it fixes bp-66ee1ec2-0502-435f-804b-2bccb2100520

Comment 13

[LegNeato]

Comment on attachment 434226 [details] [diff] [review]
patch

We will not be taking this for 3.6.6. Moving approval request forward.

If you disagree, send me an email.

Comment 14

[Daniel Veditz [:dveditz]]

Comment on attachment 434226 [details] [diff] [review]
patch

Is this likely to help the topcrash? I don't see much venkman in the current crashes, but a large correlation with
95% (200/211) vs.   0% (270/65663) {3DF533F5-FB3C-4c4c-A1D7-99717F8C3038}

I think that's the webroot malicious URL filtering extension. Seems unlikely to be using debug apis but who knows?

Comment 15

[Daniel Veditz [:dveditz]]

This is still the #28 topcrash on 3.6.17, no crashes on mozilla-2.0. maybe this patch worked?

Comment 16

[Daniel Veditz [:dveditz]]

Comment on attachment 434226 [details] [diff] [review]
patch

Approved for 1.9.2.18, a=dveditz for release-drivers

Comment 17

[Daniel Veditz [:dveditz]]

Not blocking, but worth landing to see if it helps the topcrash.

Comment 18

[Daniel Veditz [:dveditz]]

Comment on attachment 434226 [details] [diff] [review]
patch

Didn't make 3.6.18

Comment 19

[Al Billings [:abillings - ex-MoCo]]

Olli, can you land this on 1.9.2, please?

Comment 20

[Olli Pettay [:smaug][[e-mail]]]

Seems like 1.9.2 doesn't compile on this machine.
I need to find someone else to make sure the patch compiles etc.

Comment 21

[Olli Pettay [:smaug][[e-mail]]]

Haven't found anyone yet who could compile and land the patch.
Perhaps I'll just land and hope it compiles.

Comment 22

[Olli Pettay [:smaug][[e-mail]]]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/5d9164417ba7

Comment 23

[Simona Badau, Desktop QA]

Can someone please post some steps to reproduce or a test case in order to get this bug verified? 

Thanks!

Comment 24

[Al Billings [:abillings - ex-MoCo]]

(In reply to Simona B from comment #23)
> Can someone please post some steps to reproduce or a test case in order to
> get this bug verified? 

QA needs this as well.