Crash/a236502437fa21cb.

Deleted: screen.mov 7.2 MB

	screen.mov 7.2 MB View Download

Thanks for the report!

Judging by crash/a236502437fa21cb, this doesn't look like a security issue to me (but I could be wrong). Assigning to ellyjones for further triage; please feel free to remove the security labels if you agree.

I don't think this is a security issue, no. Over to avi@.

Can you please share with us the call traces by crash/a236502437fa21cb?

0x00007fffba6fe44e	(AppKit + 0x003a544e )	-[NSApplication _crashOnException:]
0x00007fffba6fe381	(AppKit + 0x003a5381 )	-[NSApplication reportException:]
0x00007fffba7d4456	(AppKit + 0x0047b456 )	uncaughtErrorProc
0x00007fffbc9b0018	(CoreFoundation + 0x0018b018 )	__handleUncaughtException
0x00007fffd1b98334	(libobjc.A.dylib + 0x00017334 )	_objc_terminate()
0x00007fffd1086d48	(libc++abi.dylib + 0x00022d48 )	std::__terminate(void (*)())
0x00007fffd10867bd	(libc++abi.dylib + 0x000227bd )	__cxa_throw
0x00007fffd1b96302	(libobjc.A.dylib + 0x00015302 )	objc_exception_throw
0x00007fffbac33ab7	(AppKit + 0x008daab7 )	_NSRunModal
0x00007fffba5f3c35	(AppKit + 0x0029ac35 )	-[NSApplication runModalForWindow:]
0x00007fffbaa8107f	(AppKit + 0x0072807f )	-[NSPrintPanel runModalWithPrintInfo:]
0x0000000104e4866c	(Google Chrome Framework -printing_context_mac.mm:124 )	printing::PrintingContextMac::AskUserForSettings(int, bool, bool, base::OnceCallback<void (printing::PrintingContext::Result)>)
0x000000010325e917	(Google Chrome Framework -print_job_worker.cc:307 )	printing::PrintJobWorker::GetSettingsWithUI(int, bool, bool)

This hangs for me on 10.11. Not sure what's going on.

NSGenericException reason -[NSApplication runModalForWindow:] may not be invoked inside of transaction commit (usually this means it was invoked inside of a view's -drawRect: method.)

On my local build, I see

[46926:775:0605/154347.337010:WARNING:ca_layer_tree_coordinator.mm(55)] Blank frame: No overlays or CALayers

Something about the repro html file causes drawing issues. Chrome seems to be stuck in the middle of a draw when trying to print that page, and 10.12 explodes when you try to show a dialog in the middle of a draw.

Chris, you know graphics. What's going on here?

The "Blank frame" warning is a red herring. I don't have any insight into this, and I don't think it's related to Chrome's compositing.

It does have something to do with graphics given the exception thrown: "-[NSApplication runModalForWindow:] may not be invoked inside of transaction commit (usually this means it was invoked inside of a view's -drawRect: method.)". Can you think of who might be helpful in this area?

FWIW on 10.13.3, 69.0.3451.0, with #views-browser-windows this doesn't seem to happen - I can't switch the tab, since the system dialog pops up immediately (sometimes) or it hangs forever on drawing the preview (sometimes). Turning off #views-browser-windows, I can reproduce.

Likely what's happening here is we're once more run into the nested event loop wonderland of print preview. (Preview is a separate renderer - which triggers the alert on mediaMatch, but isn't allowed to spin that nested loop)

+thestig mostly because it's an interesting crash involving print preview, but I'd suggest we WontFix, since #views-browser-windows will at some point go live, and the crash circumstances are somewhat rare. Elly?

I think WontFix here also.

What's bothering me is that that's not how I see it.

"-[NSApplication runModalForWindow:] may not be invoked inside of transaction commit"

Are we accidentally leaving a Core Animation transaction open? This is shouting to me as a symptom of something deeper.

#13: Hm, okay. Chris, is that something that can happen? If so, where are we likely to do that?

Full stack:

0x00007fffba6fe44e	(AppKit + 0x003a544e )	-[NSApplication _crashOnException:]
0x00007fffba6fe381	(AppKit + 0x003a5381 )	-[NSApplication reportException:]
0x00007fffba7d4456	(AppKit + 0x0047b456 )	uncaughtErrorProc
0x00007fffbc9b0018	(CoreFoundation + 0x0018b018 )	__handleUncaughtException
0x00007fffd1b98334	(libobjc.A.dylib + 0x00017334 )	_objc_terminate()
0x00007fffd1086d48	(libc++abi.dylib + 0x00022d48 )	std::__terminate(void (*)())
0x00007fffd10867bd	(libc++abi.dylib + 0x000227bd )	__cxa_throw
0x00007fffd1b96302	(libobjc.A.dylib + 0x00015302 )	objc_exception_throw
0x00007fffbac33ab7	(AppKit + 0x008daab7 )	_NSRunModal
0x00007fffba5f3c35	(AppKit + 0x0029ac35 )	-[NSApplication runModalForWindow:]
0x00007fffbaa8107f	(AppKit + 0x0072807f )	-[NSPrintPanel runModalWithPrintInfo:]
0x0000000104e4866c	(Google Chrome Framework -printing_context_mac.mm:124 )	printing::PrintingContextMac::AskUserForSettings(int, bool, bool, base::OnceCallback<void (printing::PrintingContext::Result)>)
0x000000010325e917	(Google Chrome Framework -print_job_worker.cc:307 )	printing::PrintJobWorker::GetSettingsWithUI(int, bool, bool)
0x000000010325e798	(Google Chrome Framework -callback.h:96 )	printing::(anonymous namespace)::WorkerHoldRefCallback(scoped_refptr<printing::PrinterQuery>, base::OnceCallback<void ()>)
0x000000010325f448	(Google Chrome Framework -bind_internal.h:407 )	base::internal::Invoker<base::internal::BindState<void (*)(scoped_refptr<printing::PrinterQuery>, base::OnceCallback<void ()>), scoped_refptr<printing::PrinterQuery>, base::OnceCallback<void ()> >, void ()>::RunOnce(base::internal::BindStateBase*)
0x0000000103351a46	(Google Chrome Framework -callback.h:96 )	base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*)
0x00000001033713d3	(Google Chrome Framework -message_loop.cc:319 )	base::MessageLoop::RunTask(base::PendingTask*)
0x00000001033718a7	(Google Chrome Framework -message_loop.cc:329 )	base::MessageLoop::DoWork()
0x0000000103373569	(Google Chrome Framework -message_pump_mac.mm:455 )	base::MessagePumpCFRunLoopBase::RunWork()
0x0000000103364009	(Google Chrome Framework + 0x021f1009 )	base::mac::CallWithEHFrame(void () block_pointer)
0x0000000103372e8e	(Google Chrome Framework -message_pump_mac.mm:431 )	base::MessagePumpCFRunLoopBase::RunWorkSource(void*)
0x00007fffbc8c93e0	(CoreFoundation + 0x000a43e0 )	__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__
0x00007fffbc8aa65b	(CoreFoundation + 0x0008565b )	__CFRunLoopDoSources0
0x00007fffbc8a9b45	(CoreFoundation + 0x00084b45 )	__CFRunLoopRun
0x00007fffbc8a9543	(CoreFoundation + 0x00084543 )	CFRunLoopRunSpecific
0x00007fffbe2da251	(Foundation + 0x00022251 )	-[NSRunLoop(NSRunLoop) runMode:beforeDate:]
0x0000000104539f35	(Google Chrome Framework -ca_transaction_observer.mm:58 )	___ZN2ui24CATransactionCoordinator15SynchronizeImplEv_block_invoke
0x00007fffc2793cc5	(QuartzCore + 0x00011cc5 )	CA::Transaction::run_commit_handlers(CATransactionPhase)
0x00007fffc289dbca	(QuartzCore + 0x0011bbca )	CA::Context::commit_transaction(CA::Transaction*)
0x00007fffc27927a0	(QuartzCore + 0x000107a0 )	CA::Transaction::commit()
0x00007fffba7d18b0	(AppKit + 0x004788b0 )	__37+[NSDisplayCycle currentDisplayCycle]_block_invoke.31
0x00007fffbc8c8de6	(CoreFoundation + 0x000a3de6 )	__CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__
0x00007fffbc8c8d56	(CoreFoundation + 0x000a3d56 )	__CFRunLoopDoObservers
0x00007fffbc8a9b08	(CoreFoundation + 0x00084b08 )	__CFRunLoopRun
0x00007fffbc8a9543	(CoreFoundation + 0x00084543 )	CFRunLoopRunSpecific
0x00007fffbbe08ebb	(HIToolbox + 0x00030ebb )	RunCurrentEventLoopInMode
0x00007fffbbe08bf8	(HIToolbox + 0x00030bf8 )	ReceiveNextEventCommon
0x00007fffbbe08b25	(HIToolbox + 0x00030b25 )	_BlockUntilNextEventMatchingListInModeWithFilter
0x00007fffba39fa53	(AppKit + 0x00046a53 )	_DPSNextEvent
0x00007fffbab1b7ed	(AppKit + 0x007c27ed )	-[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:]
0x0000000102fa582f	(Google Chrome Framework -chrome_browser_application_mac.mm:233 )	__71-[BrowserCrApplication nextEventMatchingMask:untilDate:inMode:dequeue:]_block_invoke
0x0000000103364009	(Google Chrome Framework + 0x021f1009 )	base::mac::CallWithEHFrame(void () block_pointer)
0x0000000102fa5763	(Google Chrome Framework -chrome_browser_application_mac.mm:232 )	-[BrowserCrApplication nextEventMatchingMask:untilDate:inMode:dequeue:]
0x00007fffba3943da	(AppKit + 0x0003b3da )	-[NSApplication run]
0x0000000103373e4b	(Google Chrome Framework -message_pump_mac.mm:808 )	base::MessagePumpNSApplication::DoRun(base::MessagePump::Delegate*)
0x00000001033729ad	(Google Chrome Framework -message_pump_mac.mm:184 )	base::MessagePumpCFRunLoopBase::Run(base::MessagePump::Delegate*)
0x0000000103395d14	(Google Chrome Framework -run_loop.cc:102 )	<name omitted>
0x0000000102fac4da	(Google Chrome Framework -chrome_browser_main.cc:2191 )	ChromeBrowserMainParts::MainMessageLoopRun(int*)

Ah, avi@ is 100% correct and this is a real issue:

CATransactionCoordinator::SynchronizeImpl() registers a commit handler, which does:

      [NSRunLoop.currentRunLoop runMode:NSDefaultRunLoopMode
                             beforeDate:deadline];

which according to this assertion message is not allowed. So, we were premature in WontFixing this - it is a real issue.

Looks like we'll need to keep the WindowResizeHelperMac task runner :/

There may still be a compromise — runModalForWindow: != runMode:beforeDate:. I want to play with it a bit. One potential compromise could be running the loop in a mode other than NSDefaultRunLoopMode.

Thinking aloud.

The sequence is:
1. sequence coordinator starts a transaction
2. we run the page a bit to let it draw in a nested loop
3. page asks to print
4. we print
5. which dies because we're within a transaction

Can we detach step 3 from 4? Page asks to print, so we post a task back to the main loop? Note that the task we'd post would have to be of a type that wouldn't be executed in the nested loop in step 2.

This feels vaguely reasonable though we'd need to make sure that any kind of dialog or other thing that the page could ask us to do in step 3 properly posted a task, which is why I worry about something like this possibly being a bandaid.

That analysis sounds right, avi@.

The best band-aid I found is to run the print dialog in a `[CATransaction setCompletionBlock:^{ … }]` block. That way, it's guaranteed to happen after the current CATransaction is done. I put a CL for that up here:

https://chromium-review.googlesource.com/c/chromium/src/+/1092232

A better world might look like this:
- The CATransaction handlers would spin the event loop in a custom mode that's not in NSRunLoopCommonModes, like AppKit does for menu fades and the like.
- Non-UI-presenting code would all run in that mode…
  - or, even better, would run from GCD (and not in *any* run loop mode)…
  - or, better still, would run off the main thread.
- UI would be presented by scheduling a task in NSDefaultRunLoopMode.

I'm kinda okay with this band-aid because showing modal UI is so rare.

One thing to note: it looks like the wish of the comment right above this code might have come true:

    // TODO(stuartmorgan): We really want a tab sheet here, not a modal window.
    // Will require restructuring the PrintingContext API to use a callback.

…this function *does* seem to use a callback now, so a good followup action might be to switch to presenting it non-modally.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a1bec15b552ea793cc090cf67ac7515273f05573

commit a1bec15b552ea793cc090cf67ac7515273f05573
Author: Sidney San Martín <sdy@chromium.org>
Date: Fri Jun 08 18:08:19 2018

Wrap showing the system print dialog in a CATransaction completion block to fix a crash.

Bug:  849538 
Change-Id: Ib04c020dbc50aae398255654f5404fd8904a5326
Reviewed-on: https://chromium-review.googlesource.com/1092232
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Sidney San Martín <sdy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565684}
[modify] https://crrev.com/a1bec15b552ea793cc090cf67ac7515273f05573/printing/BUILD.gn
[modify] https://crrev.com/a1bec15b552ea793cc090cf67ac7515273f05573/printing/printing_context_mac.mm