637621 - Crash [@ nsRange::IsValidBoundary] after selected node is GCed

Status: RESOLVED FIXED

(Core :: DOM: Selection, defect)

Description

[Jesse Ruderman]

Attachment: testcase (requires extension for GC)

1. Install 'DOM Fuzz Lite' from
    https://www.squarefree.com/extensions/domFuzzLite.xpi
2. Load the testcase.

Crash [@ nsRange::IsValidBoundary]

http://hg.mozilla.org/mozilla-central/file/410519307e63/content/base/src/nsRange.cpp#l598 is the crashing line

Comment 1

[Jesse Ruderman]

Attachment: stack trace

Comment 2

[Jesse Ruderman]

Seems to be debug-only with this testcase, but that's probably just because opt is lucky and memory isn't overwritten.

Comment 3

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

patch coming.

Comment 4

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

Attachment: patch

Clear(presContext) in nsTypedSelection::Collapse deletes range objects,
and the method is called Collapse(lastRange->GetEndParent(), lastRange->EndOffset());

So, the safest patch I can think is to just keep aParentNode alive.
I know, the caller should do it, but in this case this is just simpler and safer.

Comment 5

[Johnny Stenback (:jst)]

Not blocking, but we'll take the patch once reviewed.

Comment 6

[Johnny Stenback (:jst)]

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

r+a=jst

Comment 7

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

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

Comment 8

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

Need to test if also branches need fixing.

Comment 9

[Daniel Veditz [:dveditz]]

The code being patched looks equivalent on the branches, assuming we need this and blocking next releases.

Comment 10

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

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

Seems to apply cleanly (some fuzz) to branches.

Comment 11

[LegNeato]

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

a=LegNeato for 1.9.2.16 and 1.9.1.18

Comment 12

[Daniel Veditz [:dveditz]]

Can we get this landed on branches?

Comment 13

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

Sorry, I need to land this asap.

Comment 14

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

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/74fd63b96277
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/33488621d2a4

Comment 15

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

Had to land a bustage fix for 1.9.1
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/fbb2dfda1784

I hadn't noticed that nsIDOMNode* parameter was changed to nsINode*