Closed Bug 641057 Opened 11 years ago Closed 11 years ago

[Mac] Firefox 4.0 crash [@ -[ChildView updatePluginFocusStatus:] ]

Categories

(Core :: Widget: Cocoa, defect)

All
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla5
Tracking Status
blocking2.0 --- Macaw+
status2.0 --- .1-fixed

People

(Reporter: smichaud, Assigned: smichaud)

References

Details

(Keywords: crash, topcrash)

Crash Data

Attachments

(2 files, 2 obsolete files)

There are a bunch of these at https://crash-stats.mozilla.com/.
There've been 24 in the last week, which puts this crash at #17 in the
list of Mac topcrashers.

https://crash-stats.mozilla.com/report/list?product=Firefox&version=Firefox%3A4.0&platform=mac&query_search=signature&query_type=exact&query=&date=03%2F11%2F2011%2010%3A36%3A53&range_value=1&range_unit=weeks&hang_type=any&process_type=any&plugin_field=&plugin_query_type=&plugin_query=&do_query=1&admin=&signature=-%5BChildView%20updatePluginFocusStatus%3A%5D

They're all null-dereferences at the following line:

http://hg.mozilla.org/releases/mozilla-2.0/annotate/5f8f494a4c29/widget/src/cocoa/nsChildView.mm#l5944

Which shows that the NS_QUERY_DOM_WIDGET_HITTEST event sent by
[ChildView currentEventShouldFocusPlugin] can cause our widget to be
deleted, nulling out mGeckoChild.

I'll post a patch in my next comment.
Assignee: nobody → smichaud
Blocks: 627649
Severity: normal → critical
Summary: Firefox 4.0 crash [@ -[ChildView updatePluginFocusStatus:] ] → [Mac] Firefox 4.0 crash [@ -[ChildView updatePluginFocusStatus:] ]
Keywords: crash, topcrash
Attached patch Fix (obsolete) — Splinter Review
Attachment #518826 - Flags: review?(joshmoz)
It's been a couple of years since I looked at this code, but don't you need
  nsAutoRetainCocoaObject kungFuDeathGrip(self);
before that call to ensure it's safe to peek at the mGeckoChild member?
Yes, that's probably a good idea.  And I see there are already several places in nsChildView.mm where "nsAutoRetainCocoaObject kungFuDeathGrip(self);" is added before dispatching a Gecko event, if mGeckoChild is to be checked afterwards.

New patch coming up.

Thanks :-)
Attachment #518826 - Attachment is obsolete: true
Attachment #518862 - Flags: review?(joshmoz)
Attachment #518826 - Flags: review?(joshmoz)
Two comments in crash stats indicate the following:

I was working on my farm on Facebook, I toggled the full screen tab and crashed.

Refreshing FB page that had flash content
Attachment #518862 - Flags: review?(joshmoz) → review+
It is #7 top crasher on Mac OS X in 4.0.
Attached patch Unbitrot my rev1 patch (obsolete) — Splinter Review
Ehsan landed this for me on cedar:
http://hg.mozilla.org/projects/cedar/rev/4c7d52ed5ccd

It should get merged to mozilla-central later today.
My rev1 patch just landed on mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/4c7d52ed5ccd
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
I'm tentatively marking this status2.0.1-fixed+

But (with our new arrangements) I don't know whether this is enough by itself to get my patch into the next release on the 2.0 branch.
> But (with our new arrangements) I don't know whether this is enough
> by itself to get my patch into the next release on the 2.0 branch.

Actually it probably isn't, since we're still using the old system for
Firefox 4.0.X.
Comment on attachment 523354 [details] [diff] [review]
Unbitrotted rev1 patch that's easier for someone else to land

This is a trivial patch (a NULL check) that fixes a Mac topcrasher.
It has zero risk.
Attachment #523354 - Flags: approval2.0?
blocking2.0: --- → .x+
blocking2.0: .x+ → Macaw
Comment on attachment 523354 [details] [diff] [review]
Unbitrotted rev1 patch that's easier for someone else to land

Approved for the mozilla2.0 repository, a=dveditz for release-drivers
Attachment #523354 - Flags: approval2.0? → approval2.0+
Comment on attachment 523354 [details] [diff] [review]
Unbitrotted rev1 patch that's easier for someone else to land

Landed on the 2.0 branch:
http://hg.mozilla.org/releases/mozilla-2.0/rev/12107407598c
Crash Signature: [@ -[ChildView updatePluginFocusStatus:] ]
You need to log in before you can comment on or make changes to this bug.