Closed Bug 815276 Opened 7 years ago Closed 7 years ago

Crash during cycle collection with <bdi>

Categories

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

defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla20
Tracking Status
firefox19 --- unaffected
firefox20 + verified
firefox-esr17 --- unaffected

People

(Reporter: jruderman, Assigned: smontagu)

References

(Blocks 2 open bugs)

Details

(Keywords: crash, regression, testcase)

Crash Data

Attachments

(3 files, 1 obsolete file)

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

Crash [@ mozilla::nsTextNodeDirectionalityMap::ResetTextNodeDirection]
Attached file stack
Presumably a regression from bug 548206; seems like something we shouldn't ship with.
Blocks: DirAuto
Keywords: regression
On Windows: bp-65046e3f-0ad5-4a00-b244-ded0f2121126
Crash Signature: [@ mozilla::nsTextNodeDirectionalityMap::ResetTextNodeDirection] → [@ mozilla::nsTextNodeDirectionalityMap::ResetTextNodeDirection] [@ nsCheapSet<nsPtrHashKey<mozilla::dom::Element> >::EnumerateEntries(PLDHashOperator (*)(nsPtrHashKey<mozilla::dom::Element>*, void*), void*)]
OS: Mac OS X → All
Hardware: x86_64 → All
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.
Attached patch Patch (obsolete) — Splinter Review
Assignee: nobody → smontagu
Attachment #685809 - Flags: review?(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)?
Attached patch Patch v.2Splinter Review
Yes, I agree that that's better.
Attachment #697592 - Flags: review?(peterv)
Attachment #685809 - Attachment is obsolete: true
Attachment #685809 - Flags: review?(peterv)
Attachment #697592 - Attachment is patch: true
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.
Attachment #697592 - Flags: review?(peterv) → review+
And please check in the testcase too.
https://hg.mozilla.org/mozilla-central/rev/c813ba800bd3
https://hg.mozilla.org/mozilla-central/rev/390baa4e7a43
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Component: Layout → DOM
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
Status: RESOLVED → VERIFIED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.