Closed Bug 849601 Opened 8 years ago Closed 8 years ago

Crash with mutation listener, mutation observer, outerHTML

Categories

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

defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla22

People

(Reporter: jruderman, Assigned: smaug)

Details

(Keywords: crash, csectype-nullptr, testcase)

Crash Data

Attachments

(3 files)

Attached file stack (ASan)
On Windows: bp-b30bdc05-8c74-49e9-b7fa-f59d02130310.
Crash Signature: [@ nsAutoMutationBatch::NodesAdded] [@ nsINode::ReplaceOrInsertBefore] → [@ nsAutoMutationBatch::NodesAdded() ] [@ nsINode::GetNextSibling(nsIDOMNode**) ]
OS: Mac OS X → All
Hardware: x86_64 → All
Assignee: nobody → bugs
Group: core-security
crash-stat stack trace looks odd.
No, this is just null pointer crash.
Group: core-security
Attached patch patchSplinter Review
This should do it.
outerHTML just inserts fragment so we can rely on normal mutationbatch handling.
We do have some tests for outerHTML + MutationObserver

https://tbpl.mozilla.org/?tree=Try&rev=f698b1dd8e54
Comment on attachment 723195 [details] [diff] [review]
patch

Hmm, I thought I has asked review for this.
Attachment #723195 - Flags: review?(jonas)
Comment on attachment 723195 [details] [diff] [review]
patch

Review of attachment 723195 [details] [diff] [review]:
-----------------------------------------------------------------

I won't have time to look at this so would be great if you could find another reviewer. It wasn't obvious to me though what the problem of the current code is so you might want to explain that to whoever ends up reviewing.
Attachment #723195 - Flags: review?(jonas) → review?
Comment on attachment 723195 [details] [diff] [review]
patch

In the current code SetOuterHTML doesn't push script blocker to stack, so
ReplaceChild may end up running mutation events and that confuses nsAutoMutationBatch.
I'll investigate how to add an assert to MutationBatch to catch these cases.
Attachment #723195 - Flags: review? → review?(bzbarsky)
Comment on attachment 723195 [details] [diff] [review]
patch

So there's another mutation batch inside the ReplaceChild that actually reports the mutations, right?

r=me
Attachment #723195 - Flags: review?(bzbarsky) → review+
Right. We do report it normally since we just replace some node with document fragment.
https://hg.mozilla.org/mozilla-central/rev/b09f11bca869
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.