613744 - crash [@ nsCOMArray<nsIDOMRange>::nsCOMArray<nsIDOMRange>() ][@ nsCOMArray<nsIDOMSVGPoint>::nsCOMArray<nsIDOMSVGPoint>() ][@ nsCOMArray<nsIPermission>::nsCOMArray<nsIPermission>() ] when trying to turn off HW acceleration

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Scoobidiver (away)]

It is a new crash signature that was introduced by 4.0b8pre/20101120 build.
It happens when you disable HW acceleration on sites with Flash.

Signature	nsCOMArray<nsIDOMRange>::nsCOMArray<nsIDOMRange>()
UUID	2feb4be7-0999-4b58-91d2-3978d2101120
Time 	2010-11-20 07:56:20.137715
Uptime	143
Last Crash	79499 seconds (22.1 hours) before submission
Install Age	143 seconds since version was first installed.
Product	Firefox
Version	4.0b8pre
Build ID	20101120044537
Branch	2.0
OS	Windows NT
OS Version	6.1.7600
CPU	x86
CPU Info	GenuineIntel family 6 model 23 stepping 10
Crash Reason	EXCEPTION_ACCESS_VIOLATION_WRITE
Crash Address	0x0
App Notes 	AdapterVendorID: 8086, AdapterDeviceID: 2a42

Frame 	Module 	Signature [Expand] 	Source
0 	xul.dll 	nsCOMArray<nsIDOMRange>::nsCOMArray<nsIDOMRange> 	obj-firefox/dist/include/nsCOMPtr.h:575
1 	xul.dll 	mozilla::layers::ImageLayerD3D9::RenderLayer 	gfx/layers/d3d9/ImageLayerD3D9.cpp:219
2 	xul.dll 	mozilla::layers::ContainerLayerD3D9::RenderLayer 	gfx/layers/d3d9/ContainerLayerD3D9.cpp:231
3 	xul.dll 	mozilla::layers::ContainerLayerD3D9::RenderLayer 	gfx/layers/d3d9/ContainerLayerD3D9.cpp:231
4 	xul.dll 	mozilla::layers::LayerManagerD3D9::Render 	gfx/layers/d3d9/LayerManagerD3D9.cpp:306
5 	xul.dll 	mozilla::layers::LayerManagerD3D9::EndTransaction 	gfx/layers/d3d9/LayerManagerD3D9.cpp:163
6 	xul.dll 	nsDisplayList::PaintForFrame 	layout/base/nsDisplayList.cpp:476
7 	xul.dll 	nsLayoutUtils::PaintFrame 	layout/base/nsLayoutUtils.cpp:1442
8 	xul.dll 	PresShell::Paint 	layout/base/nsPresShell.cpp:6112
9 	xul.dll 	nsViewManager::RenderViews 	view/src/nsViewManager.cpp:447
10 	xul.dll 	nsViewManager::Refresh 	view/src/nsViewManager.cpp:413
11 	xul.dll 	nsViewManager::DispatchEvent 	view/src/nsViewManager.cpp:912
12 	xul.dll 	AttachedHandleEvent 	view/src/nsView.cpp:193
13 	xul.dll 	nsWindow::DispatchEvent 	widget/src/windows/nsWindow.cpp:3559
14 	xul.dll 	nsWindow::DispatchWindowEvent 	widget/src/windows/nsWindow.cpp:3587
15 	xul.dll 	nsWindow::OnPaint 	
16 	ntdll.dll 	RtlpLowFragHeapAllocFromContext 	
17 	user32.dll 	EnumDisplaySettingsA 	

More reports at:
http://crash-stats.mozilla.com/report/list?product=Firefox&query_search=signature&query_type=exact&query=&range_value=4&range_unit=weeks&hang_type=any&process_type=any&plugin_field=&plugin_query_type=&plugin_query=&do_query=1&admin=&signature=nsCOMArray%3CnsIDOMRange%3E%3A%3AnsCOMArray%3CnsIDOMRange%3E%28%29

Comment 1

[Bas Schouten (:bas.schouten)]

This happens presumably because whatever is drawing the plugin does not have code to deal with its ImageContainer's layerManager not being the same as its Layer's LayerManager. This situation occurs when LayerManagers are changed. I can add some code to ImageLayerD3D9 to make sure this doesn't crash, but the user should probably deal with this and start using a new image container.

Comment 2

[Benjamin Smedberg]

Bug 611596 has the same basic root cause, I think. How can one be notified when the layer manager changes?

Comment 3

[Bas Schouten (:bas.schouten)]

(In reply to comment #2)
> Bug 611596 has the same basic root cause, I think. How can one be notified when
> the layer manager changes?

Other code compares its container's manager with the layer manager during display list construction I believe.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Yes, that's the best way.

Comment 5

[Benjamin Smedberg]

Attachment: Compare layer managers before using cached ImageContainer, rev. 1

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Actually I think I made a mistake here.

I think this comment on ImageLayer::SetContainer is wrong:
   * Set the ImageContainer. aContainer must have the same layer manager
   * as this layer.
In fact, we should allow aContainer to have any layer manager (although having the same layer manager is best for performance).

ImageLayerD3D9, ImageLayerD3D10 and ImageLayerOGL should be updated to handle that. Basically if the container is an unexpected type, we should just call GetCurrentAsSurface on it and proceed.

But for this bug, we still want the change in this patch for nsObjectFrame::GetImageContainer, because recreating the image container when the layer manager changes is good for performance. But do we need the null checks in the rest of the patch?

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Filed bug 615316 on the first part of comment #6.

Comment 8

[Benjamin Smedberg]

The null checks were because I assumed that nsContentUtils::LayerManagerForDocument was possibly fallible. If it is infallible and manager->CreateImageContainer is infallible, nsObjectFrame::GetImageContainer is also infallible and we don't need the null checks.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 493671 [details] [diff] [review]
Compare layer managers before using cached ImageContainer, rev. 1

Fair enough.

+     if (container)
+       container->SetCurrentImage(nsnull);

{} around the body

Comment 10

[Benjamin Smedberg]

http://hg.mozilla.org/mozilla-central/rev/1fa8a96b1c2c