Last Comment Bug 651177 - [Mac] Crashes [@ nsObjectFrame::GetLayerState ]
: [Mac] Crashes [@ nsObjectFrame::GetLayerState ]
Status: RESOLVED FIXED
: crash, regression, topcrash
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: x86_64 Mac OS X
: -- critical (vote)
: ---
Assigned To: Steven Michaud [:smichaud] (Retired)
:
Mentors:
Depends on:
Blocks: 617539
  Show dependency treegraph
 
Reported: 2011-04-19 10:23 PDT by Steven Michaud [:smichaud] (Retired)
Modified: 2011-06-09 14:58 PDT (History)
2 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed


Attachments
Fix (872 bytes, patch)
2011-04-19 10:41 PDT, Steven Michaud [:smichaud] (Retired)
benjamin: review+
dbaron: approval‑mozilla‑aurora+
Details | Diff | Review

Description Steven Michaud [:smichaud] (Retired) 2011-04-19 10:23:27 PDT
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) 2011-04-19 10:25:19 PDT
I've no idea why this bug happens only on OS X.
Comment 2 Steven Michaud [:smichaud] (Retired) 2011-04-19 10:32:33 PDT
> 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) 2011-04-19 10:41:24 PDT
Created attachment 527037 [details] [diff] [review]
Fix
Comment 4 Benjamin Smedberg [:bsmedberg] 2011-04-19 10:56:38 PDT
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) 2011-04-19 11:10:58 PDT
> 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) 2011-04-19 12:15:59 PDT
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) 2011-04-19 12:21:01 PDT
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 [:bsmedberg] 2011-04-19 12:35:44 PDT
Yes, you should request approval, as noted in the dev-planning threads.
Comment 9 Steven Michaud [:smichaud] (Retired) 2011-04-19 12:45:13 PDT
> 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-7 (review requests must explain patch) 2011-04-19 16:04:26 PDT
Comment on attachment 527037 [details] [diff] [review]
Fix

approved for mozilla-aurora (from triage meeting)
Comment 11 Steven Michaud [:smichaud] (Retired) 2011-04-20 07:34:39 PDT
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 - use ni] 2011-04-28 14:41:06 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.