[FIX] Crash on quit [@ nsImageMap::FreeAreas]

VERIFIED FIXED

Status

()

--
critical
VERIFIED FIXED
16 years ago
14 years ago

People

(Reporter: bc, Assigned: mats)

Tracking

({crash, testcase})

Trunk
crash, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(6 attachments, 1 obsolete attachment)

(Reporter)

Description

16 years ago
Found a script in npd.dom which seems like it should work but actually crashes.
The script dynamically adds/removes children of a MAP. This seems related to the
fix for bug 49122.

Steps to reproduce:

1. load test case
2. click around in the map
3. exit browser
4. crash

Talkback: 20164875

Looks like

nsImageMap::FreeAreas()
{
nsCOMPtr<nsIFrameManager> frameManager;
*here*=> mPresShell->GetFrameManager(getter_AddRefs(frameManager));


Reproducible Everytime

Build 2003051608 on Win2k
(Reporter)

Comment 1

16 years ago
Created attachment 123562 [details]
Crash TestCase

Updated

16 years ago
Severity: normal → critical
Keywords: crash, testcase
Summary: Crash - nsImageMap::FreeAreas → Crash [@ nsImageMap::FreeAreas]

Comment 2

16 years ago
Reproduced using FizzillaMach/2003-05-16-08-trunk, generating TB262612Q. Setting
All/All.
OS: Windows 2000 → All
Hardware: PC → All
Summary: Crash [@ nsImageMap::FreeAreas] → Crash on quit [@ nsImageMap::FreeAreas]

Comment 3

16 years ago
Created attachment 123571 [details]
Crash report generated by FizzillaMach/2003-05-16-08-trunk showing crash at 0x0
(Assignee)

Comment 4

15 years ago
Created attachment 137352 [details]
Crash data
(Assignee)

Comment 5

15 years ago
We crash in |nsImageMap::FreeAreas| using a null |frameManager|.
The real problem is that it is called from |nsImageMap::~nsImageMap| which
is too late since the owning frame |mImageFrame| is destroyed by that time and
thus we can't use the weak pointers (nsImageMap::mPresShell etc).

Taking, patch coming up...
Assignee: waterson → mats.palmgren
Summary: Crash on quit [@ nsImageMap::FreeAreas] → [FIX] Crash on quit [@ nsImageMap::FreeAreas]
(Assignee)

Comment 6

15 years ago
Created attachment 137353 [details] [diff] [review]
Patch rev. 1

Move the |SetPrimaryFrameFor| and |RemoveObserver| calls from the destructor
to |nsImageMap::Destroy|.
(Assignee)

Updated

15 years ago
Attachment #137353 - Flags: review?(dbaron)
Comment on attachment 137353 [details] [diff] [review]
Patch rev. 1

Wouldn't it be easier to just NS_ASSERTION(mAreas.Count() == 0, "..."); in
~nsImageMap and skip the extra parameter to FreeAreas?
Attachment #137353 - Flags: review?(dbaron) → review-
(Assignee)

Comment 8

14 years ago
Created attachment 162337 [details]
Testcase #2
(Assignee)

Comment 9

14 years ago
Created attachment 162338 [details]
Stack from crash on Testcase #2

I suspect this is the same bug...
Mats, any chance of an updated patch here?  (Did my comment not make sense?)
Created attachment 178653 [details] [diff] [review]
patch rev. 2

This is what I was thinking of.  I can't reproduce the crash, though, so I
can't tell if this really fixes it.
Attachment #137353 - Attachment is obsolete: true
Attachment #178653 - Flags: review?(mats.palmgren)
(Assignee)

Comment 12

14 years ago
Comment on attachment 178653 [details] [diff] [review]
patch rev. 2

I can still reproduce the crashes (both testcases) in a 
Linux debug build and this patch (also) fix them.
r=mats

How about that assertion in ~nsImageMap() ? (comment 7)
Attachment #178653 - Flags: review?(mats.palmgren) → review+
Comment on attachment 178653 [details] [diff] [review]
patch rev. 2

ok, assertion added (with text "Destroy was not called")
Attachment #178653 - Flags: superreview?(roc)
Attachment #178653 - Flags: superreview?(roc) → superreview+
Fix checked in to trunk, 2005-03-28 15:03 -0800.
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
Verified FIXED using build 2005-04-18-05, Windows XP Seamonkey trunk.
Status: RESOLVED → VERIFIED
Crash Signature: [@ nsImageMap::FreeAreas]
You need to log in before you can comment on or make changes to this bug.