1350738 - [e10s] Crash in mozilla::gfx::DrawTargetRecording::DrawTargetRecording

Status: RESOLVED FIXED

Description

[[:philipp]]

+++ This bug was initially created as a clone of Bug #1315212 +++

This bug was filed from the Socorro interface and is 
report bp-1ce13c02-b7d8-4089-9e0d-8b79f2170326.
=============================================================
Crashing Thread (0)
Frame 	Module 	Signature 	Source
0 	xul.dll 	mozilla::gfx::DrawTargetRecording::DrawTargetRecording(mozilla::gfx::DrawEventRecorder*, mozilla::gfx::DrawTarget*, bool) 	gfx/2d/DrawTargetRecording.cpp:269
1 	xul.dll 	mozilla::gfx::Factory::CreateRecordingDrawTarget(mozilla::gfx::DrawEventRecorder*, mozilla::gfx::DrawTarget*) 	gfx/2d/Factory.cpp:363
2 	xul.dll 	nsSimplePageSequenceFrame::PrePrintNextPage(nsITimerCallback*, bool*) 	layout/generic/nsSimplePageSequenceFrame.cpp:637
3 	xul.dll 	nsPrintEngine::PrePrintPage() 	layout/printing/nsPrintEngine.cpp:2659
4 	xul.dll 	nsPagePrintTimer::Notify(nsITimer*) 	layout/printing/nsPagePrintTimer.cpp:156
5 	xul.dll 	nsTimerImpl::Fire(int) 	xpcom/threads/nsTimerImpl.cpp:482
6 	xul.dll 	nsTimerEvent::Run() 	xpcom/threads/TimerThread.cpp:297
7 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp:1240
8 	xul.dll 	NS_ProcessNextEvent(nsIThread*, bool) 	xpcom/glue/nsThreadUtils.cpp:390
9 	xul.dll 	mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp:124
10 	xul.dll 	mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp:301
11 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc:231
12 	xul.dll 	MessageLoop::Run() 	ipc/chromium/src/base/message_loop.cc:211
13 	xul.dll 	nsBaseAppShell::Run() 	widget/nsBaseAppShell.cpp:156
14 	xul.dll 	nsAppShell::Run() 	widget/windows/nsAppShell.cpp:262
15 	xul.dll 	XRE_RunAppShell() 	toolkit/xre/nsEmbedFunctions.cpp:924
16 	xul.dll 	mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp:269
17 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc:231
18 	xul.dll 	MessageLoop::Run() 	ipc/chromium/src/base/message_loop.cc:211
19 	xul.dll 	XRE_InitChildProcess(int, char** const, XREChildData const*) 	toolkit/xre/nsEmbedFunctions.cpp:756
20 	xul.dll 	mozilla::BootstrapImpl::XRE_InitChildProcess(int, char** const, XREChildData const*) 	toolkit/xre/Bootstrap.cpp:65
21 	firefox.exe 	content_process_main(mozilla::Bootstrap*, int, char** const) 	ipc/contentproc/plugin-container.cpp:115
22 	firefox.exe 	wmain 	toolkit/xre/nsWindowsWMain.cpp:115
23 	firefox.exe 	__scrt_common_main_seh 	f:/dd/vctools/crt/vcstartup/src/startup/exe_common.inl:253
24 	kernel32.dll 	BaseThreadInitThunk 	
25 	ntdll.dll 	__RtlUserThreadStart 	
26 	ntdll.dll 	_RtlUserThreadStart 	

this e10s crash signature was fixed in bug 1315212 but it's resurfacing in the 53.0b cycle - it's accounting for 0.5% of content crashes: 
https://crash-stats.mozilla.com/signature/?release_channel=beta&signature=mozilla%3A%3Agfx%3A%3ADrawTargetRecording%3A%3ADrawTargetRecording&date=>%3D2016-09-26T20%3A32%3A38.000Z#graphs

Correlations for Firefox Beta
(99.31% in signature vs 00.05% overall) address = 0x39
(100.0% in signature vs 11.32% overall) reason = EXCEPTION_ACCESS_VIOLATION_READ
(100.0% in signature vs 21.65% overall) moz_crash_reason = null
(100.0% in signature vs 23.75% overall) graphics_startup_test = null
(100.0% in signature vs 23.76% overall) ipc_system_error = null
(100.0% in signature vs 00.07% overall) GFX_ERROR "Failed to create similar cairo surface! Size: " = true [69.70% vs 00.18% if startup_crash = null]

Comment 1

[Bob Owen (:bobowen)]

This is a different crash to bug 1315212, it is in a different constructor.
(Although, it is called directly after the previous one, so it is probably the same underlying cause where the CreateSimilarDrawTarget on the wrapped (non-recording) DrawTarget has failed.)

The code at [1] looks a little suspect anyway.
At the very least the creation of the DrawEventRecorder and call to CreateRecordingDrawTarget should go after the null check.

Also, if we are printing via the parent (which is starting to become the norm in all e10s case), then canvasTarget is already a DrawTargetRecording.
Wrapping it again seems a little weird and will possibly cause issues.

I would have thought that we could just use the normal printing DrawTargetRecording canvasTarget, but other code in that patch assumes that we have a DrawEventRecorderMemory, so something will probably break.
However, the original DrawTargetRecording (and its DrawEventRecorder) should already know about that SourceSurfaceRecording
so I'm not sure we would have to do any of the InlineTranslator dance in FillRectWithMask in that case.

tobytailor - was the use of DrawTargetRecording trying to solve something for the non-print via parent case, which would include non-e10s as well?

[1] https://hg.mozilla.org/releases/mozilla-beta/annotate/eb20f71a4f43/layout/generic/nsSimplePageSequenceFrame.cpp#l637

Comment 2

[Blake Wu [:bwu][:blakewu]]

Peter, 
This is "firefox53 --- affected". Can you help move this bug forward? Maybe it is related to layout.

Comment 3

[Peter Chang[:pchang]]

From crash report, we were failed to allocate the draw target for DrawTargetRecording.
We should fix the crash fist by adding the nullptr checking in [1].

Vincent, please help on this.

|[C0][GFX1]: Failed to create similar cairo surface! Size: Size(1275,1650) Status: 10 format 0 (t=260484) 
[1]http://searchfox.org/mozilla-central/source/layout/generic/nsSimplePageSequenceFrame.cpp#648

Comment 4

[Bob Owen (:bobowen)]

This should have been fixed by my backout in bug 1347646, where I backed out bug 1311512.

It's already been approved for uplift to Beta.