815276 - Crash during cycle collection with <bdi>

Status: VERIFIED FIXED

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

Description

[Jesse Ruderman]

Attachment: testcase (crashes Firefox on next CC)

1. Load the testcase.
2. Trigger a cycle collection, e.g. by quitting Firefox.

Crash [@ mozilla::nsTextNodeDirectionalityMap::ResetTextNodeDirection]

Comment 1

[Jesse Ruderman]

Attachment: stack

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Presumably a regression from bug 548206; seems like something we shouldn't ship with.

Comment 3

[Scoobidiver (away)]

On Windows: bp-65046e3f-0ad5-4a00-b244-ded0f2121126

Comment 4

[Boris Zbarsky [:bzbarsky]]

So we're doing ResetTextNodeDirection from the unbind triggered by unlink.  Presumably GetDirectionalityMap is returning null because we've already unlinked the textnode and cleared its properties.

Comment 5

[Simon Montagu :smontagu]

Attachment: Patch

Comment 6

[Peter Van der Beken [:peterv]]

Comment on attachment 685809 [details] [diff] [review]
Patch

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

Wouldn't it make more sense to do this in nsTextNodeDirectionalityMapDtor (you get the text node passed in through aObject I think)?

Comment 7

[Simon Montagu :smontagu]

Attachment: Patch v.2

Yes, I agree that that's better.

Comment 8

[Peter Van der Beken [:peterv]]

Comment on attachment 697592 [details] [diff] [review]
Patch v.2

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

::: content/base/src/DirectionalityUtils.cpp
@@ +433,5 @@
>    static void
>    nsTextNodeDirectionalityMapDtor(void *aObject, nsIAtom* aPropertyName,
>                                    void *aPropertyValue, void* aData)
>    {
> +    nsINode* textNode = reinterpret_cast<nsINode * >(aObject);

This should be a static_cast.

Comment 9

[Peter Van der Beken [:peterv]]

And please check in the testcase too.

Comment 10

[Simon Montagu :smontagu]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c813ba800bd3
https://hg.mozilla.org/integration/mozilla-inbound/rev/390baa4e7a43

Comment 11

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/c813ba800bd3
https://hg.mozilla.org/mozilla-central/rev/390baa4e7a43

Comment 12

[Mihai Morar, (:MihaiMorar)]

I confirm the fix is verified on FF 20b6 on Windows 7x64, Linux 12.04 and MacOS 10.8

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20100101 Firefox/20.0
Mozilla/5.0 (X11; Linux i686; rv:20.0) Gecko/20100101 Firefox/20.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:20.0) Gecko/20100101 Firefox/20.0

BuildID: 20130320062118