Crash with mutation listener, mutation observer, outerHTML

RESOLVED FIXED in mozilla22

Status

()

--
critical
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jruderman, Assigned: smaug)

Tracking

(Blocks: 1 bug, {crash, csectype-nullptr, testcase})

Trunk
mozilla22
crash, csectype-nullptr, testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(3 attachments)

(Reporter)

Description

6 years ago
Created attachment 723160 [details]
testcase (crashes Firefox when loaded)

bp-18837b23-0967-468e-8006-c49d52130310
(Reporter)

Comment 1

6 years ago
Created attachment 723161 [details]
stack (ASan)

Comment 2

6 years ago
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
crash-stat stack trace looks odd.
No, this is just null pointer crash.
Group: core-security
Created attachment 723195 [details] [diff] [review]
patch

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)
Keywords: csec-nullptr
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
Last Resolved: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in before you can comment on or make changes to this bug.