Closed Bug 689366 Opened 10 years ago Closed 10 years ago

Crash [@ mozilla::layout::RenderFrameParent::GetLayerManager]


(Core :: Web Painting, defect)

8 Branch
Not set



Tracking Status
firefox9 --- fixed


(Reporter: nhirata, Assigned: cjones)


(Keywords: crash, Whiteboard: [mobile-crash] [inbound])

Crash Data


(1 file)

This bug was filed from the Socorro interface and is 
report bp-6ebf49e2-2163-449e-89a7-a2d832110920 .

Frame 	Module 	Signature [Expand] 	Source
0 	mozilla::layout::RenderFrameParent::GetLayerManager 	nsIDocument.h:485
1 	mozilla::layout::RenderFrameParent::AllocPLayers 	layout/ipc/RenderFrameParent.cpp:720
2 	mozilla::layout::PRenderFrameParent::OnMessageReceived 	obj-firefox/ipc/ipdl/PRenderFrameParent.cpp:105
3 	mozilla::dom::PContentParent::OnMessageReceived 	obj-firefox/ipc/ipdl/PContentParent.cpp:689
4 	mozilla::ipc::AsyncChannel::OnDispatchMessage 	ipc/glue/AsyncChannel.cpp:294
5 	mozilla::ipc::RPCChannel::OnMaybeDequeueOne 	ipc/glue/RPCChannel.cpp:435
6 	RunnableMethod<mozilla::ipc::RPCChannel, bool , Tuple0>::Run 	ipc/chromium/src/base/task.h:308
7 	mozilla::ipc::RPCChannel::DequeueTask::Run 	RPCChannel.h:487
8 	MessageLoop::RunTask 	ipc/chromium/src/base/
9 	MessageLoop::DeferOrRunPendingTask 	ipc/chromium/src/base/
10 	MessageLoop::DoWork 	ipc/chromium/src/base/
11 	mozilla::ipc::MessagePump::Run 	ipc/glue/MessagePump.cpp:115
12 	MessageLoop::RunInternal 	ipc/chromium/src/base/
13 	MessageLoop::Run 	ipc/chromium/src/base/
14 	nsBaseAppShell::Run 	widget/src/xpwidgets/nsBaseAppShell.cpp:191
15 	nsAppStartup::Run 	toolkit/components/startup/nsAppStartup.cpp:223
16 	XRE_main 	toolkit/xre/nsAppRunner.cpp:3572
17 	Java_org_mozilla_gecko_GeckoAppShell_nativeRun 	toolkit/xre/nsAndroidStartup.cpp:132
18 	Java_org_mozilla_gecko_GeckoAppShell_nativeRun 	other-licenses/android/APKOpen.cpp:234
20 	dalvik-LinearAlloc (deleted) 	dalvik-LinearAlloc @0x22b6d3 	
21 	2 (deleted) 	2 @0x111d2f 	
24 	mnt@asec@org.mozilla.firefox_beta-1@pkg.apk@classes.dex 	mnt@asec@org.mozilla.firefox_beta-1@pkg.apk@classes.dex@0x10169 	
25 	Java_org_mozilla_gecko_GeckoAppShell_nativeRun 	other-licenses/android/APKOpen.cpp:234
26 	dalvik-LinearAlloc (deleted) 	dalvik-LinearAlloc @0x22b6d3 	
27 	dalvik-LinearAlloc (deleted) 	dalvik-LinearAlloc @0x22b6d3 	
29 	2 (deleted) 	2 @0x111d2f 	
30 	Java_org_mozilla_gecko_GeckoAppShell_nativeRun 	other-licenses/android/APKOpen.cpp:234
32 	core.odex 	core.odex@0xe616f 	
33 	mnt@asec@org.mozilla.firefox_beta-1@pkg.apk@classes.dex 	mnt@asec@org.mozilla.firefox_beta-1@pkg.apk@classes.dex@0x9ead 	
34 	2 (deleted) 	2 @0x111d2f 	
41 	core.odex 	core.odex@0xe4ffb 	
42 	2 (deleted) 	2 @0x163497 	
43 	dalvik-LinearAlloc (deleted) 	dalvik-LinearAlloc @0x22ba7b 	
51 	core.odex 	core.odex@0x147139 	
52 	core.odex 	core.odex@0x147139 	
53 	core.odex 	core.odex@0x147187

More Crashes :

See Bug 603680; seems to be the same crash?
Have you found a testcase or STR?

This looks like a null deref

RenderFrameParent::GetLayerManager() const
  nsIDocument* doc = mFrameLoader->GetOwnerDoc();
  return doc->GetShell()->GetLayerManager();        <==== HERE

of null |doc|.

bz, is a document-less frame loader a state we need to check for here?
nsFrameLoader::GetOwnerDoc looks like this:

  248   { return mOwnerContent ? mOwnerContent->GetOwnerDoc() : nsnull; }

At this point GetOwnerDoc never returns null except in the middle of the document and element being cycle collected. But mOwnerContent can absolutel go away if someone is holding a strong ref to the frameloader and the element gets GCed.  Note that if that happens there will be a destroy() call on the frameloader.

Can a RenderFrameParent still be running after a destroy() on its mFrameLoader?  If so, it should probably have checks for the frameloader being already destroyed.
Yes, it looks like what could be happening is
 - nsFrameLoader::Destroy() is called, which calls TabParent::Destroy()
 - TabParent::Destroy() starts the destruction process, and calls RenderFrameParent::Destroy() for all live frames
 - RenderFrameParent::Destroy() kicks off its own destruction process, and ...

... doesn't record that the mFrameLoader was destroyed!  So it looks like if we try to create a new layer tree in the content process after nsFrameLoader::Destroy(), we could tickle this bug.  (If there's already a layer created but a paint comes in after nsFrameLoader::Destroy(), which is common, then nothing bad happens.)  That situation could arise if a tab is opened then closed really fast, but it would be a totally unreliable test.

I think I know how to patch this.
Assignee: nobody → jones.chris.g
Comment on attachment 562670 [details] [diff] [review]
Don't ask our frame loader for its layer manager after Destroy()

Attachment #562670 - Flags: review?(bzbarsky) → review+
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
Can we have this pushed to Aurora please?  This crasher also happens in Aurora.
What's the process for landing upstream now?  Do we have to request approval somewhere?
cjones, apparantly one would "set the approval aurora flag on the patch attachment , and make a comment about why it's important.

The release team evaluates those several times a week. If you think it's valuable to advocate for it in person, you can come to one of the meetings on Tuesday and Thursday at 2pm or talk ahead of time with someone who attends those meetings. "

This was from Asa.  I don't have the rights to set an approval aurora flag on the patch attachment.  Can you do this please?
Comment on attachment 562670 [details] [diff] [review]
Don't ask our frame loader for its layer manager after Destroy()

Requesting aurora approval.  This is a safe patch to make sure that we're not trying to operate on already-destroyed frame managers.
Attachment #562670 - Flags: approval-mozilla-aurora?
Comment on attachment 562670 [details] [diff] [review]
Don't ask our frame loader for its layer manager after Destroy()

Approved for releases/mozilla-aurora
Attachment #562670 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Chris, you going to push this to aurora, or want me to?
Do you have anything else to ride along with it?  I should have time this evening, but I don't have an aurora clone yet.  If you're ready to fire away, please feel free! :)
Component: Layout: View Rendering → Layout: Web Painting
You need to log in before you can comment on or make changes to this bug.