Crash in upstream RenderWidgetHostImpl when CanCreateWindow sets no_javascript_access=true

[pilgrim-brave]

In the course of implementing RequestOTRTab #28750, we want in some circumstances to set no_javascript_access to true via an overridden BraveContentBrowserClient::CanCreateWindow method, which allows this (by passing a pointer to the variable) and documents it in the upstream header files.

Unfortunately, calling window.open under such conditions leads to a hard application-level crash.

[53457:259:0426/164108.802095:FATAL:associated_remote.h(81)] Check failed: is_bound(). Cannot issue Interface method calls on an unbound AssociatedRem
ote
0   libbase.dylib                       0x0000000102835c12 base::debug::CollectStackTrace(void**, unsigned long) + 18
1   libbase.dylib                       0x000000010281aee3 base::debug::StackTrace::StackTrace() + 19
2   libbase.dylib                       0x00000001026f1395 logging::LogMessage::~LogMessage() + 181
3   libbase.dylib                       0x00000001026f20de logging::LogMessage::~LogMessage() + 14
4   libbase.dylib                       0x00000001026d2567 logging::CheckError::~CheckError() + 23
5   libbase.dylib                       0x00000001026d2589 logging::CheckError::~CheckError() + 9
6   libcontent.dylib                    0x000000010901cb80 content::RenderWidgetHostImpl::WasShown(mojo::StructPtr<blink::mojom::RecordContentToVisibl
eTimeRequest>) + 1408
7   libcontent.dylib                    0x00000001092c3f79 content::RenderWidgetHostViewMac::NotifyHostAndDelegateOnWasShown(mojo::StructPtr<blink::mo
jom::RecordContentToVisibleTimeRequest>) + 185
8   libcontent.dylib                    0x000000010903c1a8 content::RenderWidgetHostViewBase::OnShowWithPageVisibility(blink::mojom::PageVisibilitySta
te) + 136
9   libcontent.dylib                    0x00000001091dc6f5 content::WebContentsImpl::CreateNewWindow(content::RenderFrameHostImpl*, content::mojom::Cr
eateNewWindowParams const&, bool, bool, content::SessionStorageNamespace*) + 1701
10  libcontent.dylib                    0x0000000108f9683f content::RenderFrameHostImpl::CreateNewWindow(mojo::StructPtr<content::mojom::CreateNewWind
owParams>, base::OnceCallback<void (content::mojom::CreateNewWindowStatus, mojo::StructPtr<content::mojom::CreateNewWindowReply>)>) + 3407
11  libcontent.dylib                    0x000000010859578f content::mojom::FrameHostStubDispatch::AcceptWithResponder(content::mojom::FrameHost*, mojo
::Message*, std::Cr::unique_ptr<mojo::MessageReceiverWithStatus, std::Cr::default_delete<mojo::MessageReceiverWithStatus>>) + 319
12  libmojo_public_cpp_bindings.dylib   0x0000000102db5b44 mojo::InterfaceEndpointClient::HandleValidatedMessage(mojo::Message*) + 1284
13  libmojo_public_cpp_bindings.dylib   0x0000000102dbd767 mojo::MessageDispatcher::Accept(mojo::Message*) + 183
14  libmojo_public_cpp_bindings.dylib   0x0000000102db7ac2 mojo::InterfaceEndpointClient::HandleIncomingMessage(mojo::Message*) + 114
15  libipc.dylib                        0x0000000103ff7b99 IPC::(anonymous namespace)::ChannelAssociatedGroupController::AcceptSyncMessage(unsigned in
t, unsigned int) + 729
16  libipc.dylib                        0x0000000103ff8202 base::internal::Invoker<base::internal::BindState<void (IPC::(anonymous namespace)::ChannelAssociatedGroupController::*)(unsigned int, unsigned int), scoped_refptr<IPC::(anonymous namespace)::ChannelAssociatedGroupController>, unsigned int, unsigned int>, void ()>::RunOnce(base::internal::BindStateBase*) + 66
17  libbase.dylib                       0x0000000102772793 base::TaskAnnotator::RunTaskImpl(base::PendingTask&) + 227
18  libbase.dylib                       0x00000001027a0400 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl(base::LazyNow*) + 1584
19  libbase.dylib                       0x000000010279f8eb base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork() + 123
20  libbase.dylib                       0x00000001027a0e25 non-virtual thunk to base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork() + 21
21  libbase.dylib                       0x00000001028618ff base::MessagePumpCFRunLoopBase::RunWork() + 95
22  libbase.dylib                       0x000000010285a912 base::mac::CallWithEHFrame(void () block_pointer) + 10
23  libbase.dylib                       0x0000000102860eaf base::MessagePumpCFRunLoopBase::RunWorkSource(void*) + 63
24  CoreFoundation                      0x00007ff802d8706a __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
25  CoreFoundation                      0x00007ff802d8700c __CFRunLoopDoSource0 + 157
26  CoreFoundation                      0x00007ff802d86de5 __CFRunLoopDoSources0 + 217
27  CoreFoundation                      0x00007ff802d85a6f __CFRunLoopRun + 916
28  CoreFoundation                      0x00007ff802d85071 CFRunLoopRunSpecific + 560
29  HIToolbox                           0x00007ff80c7edfcd RunCurrentEventLoopInMode + 292
30  HIToolbox                           0x00007ff80c7eddde ReceiveNextEventCommon + 657
31  HIToolbox                           0x00007ff80c7edb38 _BlockUntilNextEventMatchingListInModeWithFilter + 64
32  AppKit                              0x00007ff805e177a0 _DPSNextEvent + 858
33  AppKit                              0x00007ff805e1664a -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 1214
34  libchrome_dll.dylib                 0x000000010e41f0c0 __71-[BrowserCrApplication nextEventMatchingMask:untilDate:inMode:dequeue:]_block_invoke + 64
35  libbase.dylib                       0x000000010285a912 base::mac::CallWithEHFrame(void () block_pointer) + 10
36  libchrome_dll.dylib                 0x000000010e41f039 -[BrowserCrApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 153
37  AppKit                              0x00007ff805e08cb8 -[NSApplication run] + 586
38  libbase.dylib                       0x00000001028624dc base::MessagePumpNSApplication::DoRun(base::MessagePump::Delegate*) + 300
39  libbase.dylib                       0x000000010286099c base::MessagePumpCFRunLoopBase::Run(base::MessagePump::Delegate*) + 140
40  libbase.dylib                       0x00000001027a132d base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run(bool, base::TimeDelta) + 493
41  libbase.dylib                       0x000000010273dfe5 base::RunLoop::Run(base::Location const&) + 501
42  libcontent.dylib                    0x00000001089222e5 content::BrowserMainLoop::RunMainMessageLoop() + 229
43  libcontent.dylib                    0x00000001089241e3 content::BrowserMainRunnerImpl::Run() + 35
44  libcontent.dylib                    0x000000010891f546 content::BrowserMain(content::MainFunctionParams) + 134
45  libcontent.dylib                    0x0000000109786cb4 content::RunBrowserProcessMain(content::MainFunctionParams, content::ContentMainDelegate*) + 212
46  libcontent.dylib                    0x0000000109788238 content::ContentMainRunnerImpl::RunBrowser(content::MainFunctionParams, bool) + 616
47  libcontent.dylib                    0x0000000109787e3c content::ContentMainRunnerImpl::Run() + 844
48  libcontent.dylib                    0x0000000109786352 content::RunContentProcess(content::ContentMainParams, content::ContentMainRunner*) + 158649  libcontent.dylib                    0x0000000109786557 content::ContentMain(content::ContentMainParams) + 103
50  libchrome_dll.dylib                 0x000000010cf35ed0 ChromeMain + 560
51  Brave Browser Development           0x0000000101f7c81e main + 286
52  dyld                                0x00007ff80295141f start + 1903
Task trace:
0   libipc.dylib                        0x0000000103ff32d2 IPC::(anonymous namespace)::ChannelAssociatedGroupController::Accept(mojo::Message*) + 1746
1   libmojo_public_system_cpp.dylib     0x00000001020df454 mojo::SimpleWatcher::Context::Notify(unsigned int, MojoHandleSignalsState, unsigned int) + 228

The root cause appears to be that the immediate caller (RenderFrameHostImpl::CreateNewWindow) properly handles the case of no_javascript_access being true, but this is only a local variable and does not affect the new window params that are then passed around. Later, in WebContentsImpl::CreateNewWindow, there is logic https://source.chromium.org/chromium/chromium/src/+/main:content/browser/web_contents/web_contents_impl.cc;l=4175 that checks params.opener_suppressed and conditionally skips some code and view widgets. This is the block that is crashing. It seems like Chromium would also want to skip this block in the case where opener_suppressed=false but no_javascript_access=true. Unfortunately, no_javascript_access is no longer available by this point.

Potential solution is to set opener_suppressed=true based if no_javascript_access=true, back in RenderFrameHostImpl. This seems to resolve the crash by skipping the problematic block later.