651177 - [Mac] Crashes [@ nsObjectFrame::GetLayerState ]

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Steven Michaud [:smichaud] (Retired)]

In the last week (since about the 13th) this crash has become the
trunk and 2.0-branch topcrasher:

https://crash-stats.mozilla.com/query/query?product=Firefox&version=Firefox%3A6.0a1&version=Firefox%3A5.0a2&range_value=2&range_unit=weeks&date=04%2F19%2F2011+10%3A04%3A22&query_search=signature&query_type=contains&query=nsObjectFrame%3A%3AGetLayerState&build_id=&process_type=any&hang_type=any&do_query=1

All these crashes are NULL-dereferences.  They're caused by a mistake
(basically a typo) in the patch for bug 617539.  What should have been
a NULL check in nsPluginInstanceOwner::IsRemoteDrawingCoreAnimation()
has been turned into its opposite:

This code was used

if (mInstance)
  return PR_FALSE;

where the following code should have been used

if (!mInstance)
  return PR_FALSE;

I'll post a patch shortly.

Comment 1

[Steven Michaud [:smichaud] (Retired)]

I've no idea why this bug happens only on OS X.

Comment 2

[Steven Michaud [:smichaud] (Retired)]

> I've no idea why this bug happens only on OS X.

Actually, nsPluginInstanceOwner::IsRemoteDrawingCoreAnimation() is only ever called on OS X.

Comment 3

[Steven Michaud [:smichaud] (Retired)]

Attachment: Fix

Comment 4

[Benjamin Smedberg]

Comment on attachment 527037 [details] [diff] [review]
Fix

This cannot have been a 2.0 branch topcrasher, if it was caused by the other bug which only landed for Firefox 5. But we do need this on aurora.

Comment 5

[Steven Michaud [:smichaud] (Retired)]

> This cannot have been a 2.0 branch topcrasher, if it was caused by the other
> bug which only landed for Firefox 5. But we do need this on aurora.

You're right.  Sorry, I got my branches mixes up.

Comment 6

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 527037 [details] [diff] [review]
Fix

Landed on the trunk:
http://hg.mozilla.org/mozilla-central/rev/28bcd72b4d92

Comment 7

[Steven Michaud [:smichaud] (Retired)]

So what happens next?

I suspect this fix should be pushed to aurora before the next
scheduled merge from mozilla-central.  Should I request approval?  Or
should I just wait for others to deal with it?

Comment 8

[Benjamin Smedberg]

Yes, you should request approval, as noted in the dev-planning threads.

Comment 9

[Steven Michaud [:smichaud] (Retired)]

> Yes, you should request approval, as noted in the dev-planning
> threads.

Thanks.

I've read the thread at
http://groups.google.com/group/mozilla.dev.planning/browse_thread/thread/61efaf6a258cbb4c#.
But it was unclear to me who should initiate the approval process --
the developer (as has been the case up til now) or others.

You've told me that developers should continue to have the primary
responsibility for making approval requests.

Comment 10

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

Comment on attachment 527037 [details] [diff] [review]
Fix

approved for mozilla-aurora (from triage meeting)

Comment 11

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 527037 [details] [diff] [review]
Fix

Landed on mozilla-aurora:
http://hg.mozilla.org/mozilla-aurora/rev/072e8fbd2867

Comment 12

[Marcia Knous [:marcia]]

This crash no longer shows in trunk crash data.

On Aurora there are a few crashes with Build ID 20110420042005 but that build might not have picked up the fix.