Repository: electron/electron
Pull request: fix: try clearing `InspectableWebContents` delegate earlier

#### Description of Change

Closes https://github.com/electron/electron/issues/34042

Fixes a crash in `WebContents::DevToolsOpened()` that could occur when the DevTools frontend fires its `loadCompleted` callback while the WebContents destructor is running. The `DevToolsEmbedderMessageDispatcher` uses `base::Unretained` for its delegate callbacks, which means it can invoke `DevToolsOpened()` on a partially-destroyed `WebContents` whose `owner_window_` WeakPtr has corrupted internal state. This moves the `SetDelegate(nullptr)` call to the very beginning of the destructor, ensuring no DevTools callbacks can reach us during destruction.

<details><summary>Stacktrace</summary>
<p>

```
Received signal 11 SEGV_MAPERR 0000000001a0

    #0 0x55a283788b46 in ___interceptor_backtrace ??:0:0

    #1 0x55a29b987fa8 in base::debug::CollectStackTrace(base::span<void const*, 18446744073709551615ul, void const**>) ./../../base/debug/stack_trace_posix.cc:1048:7

    #2 0x55a29b93cc1f in base::debug::StackTrace::StackTrace(unsigned long) ./../../base/debug/stack_trace.cc:280:20

    #3 0x55a29b986d82 in base::debug::(anonymous namespace)::StackDumpSignalHandler(int, siginfo_t*, void*) ./../../base/debug/stack_trace_posix.cc:483:3

    #4 0x7f0c2b687520 in __GI___sigaction :?

    #5 0x55a29b5d3afc in operator bool ./../../base/memory/scoped_refptr.h:319:43

    #6 0x55a29b5d3afc in base::internal::WeakReference::IsValid() const ./../../base/memory/weak_ptr.cc:75:10

    #7 0x55a283bb8648 in get ./../../base/memory/weak_ptr.h:238:32

    #8 0x55a283bb8648 in owner_window ./../../electron/shell/browser/api/electron_api_web_contents.h:419:61

    #9 0x55a283bb8648 in electron::api::WebContents::DevToolsOpened() ./../../electron/shell/browser/api/electron_api_web_contents.cc:2207:23

    #10 0x55a283ee7f27 in electron::InspectableWebContents::LoadCompleted() ./../../electron/shell/browser/ui/inspectable_web_contents.cc:587:27

    #11 0x55a2b4db90b2 in Invoke<void (DevToolsEmbedderMessageDispatcher::Delegate::*)(), DevToolsEmbedderMessageDispatcher::Delegate *> ./../../base/functional/bind_internal.h:730:12

    #12 0x55a2b4db90b2 in MakeItSo<void (DevToolsEmbedderMessageDispatcher::Delegate::*const &)(), const std::__Cr::tuple<base::internal::UnretainedWrapper<DevToolsEmbedderMessageDispatcher::Delegate, base::unretained_traits::MayNotDangle, (partition_alloc::internal::RawPtrTraits)0> > &> ./../../base/functional/bind_internal.h:922:12

    #13 0x55a2b4db90b2 in RunImpl<void (DevToolsEmbedderMessageDispatcher::Delegate::*const &)(), const std::__Cr::tuple<base::internal::UnretainedWrapper<DevToolsEmbedderMessageDispatcher::Delegate, base::unretained_traits::MayNotDangle, (partition_alloc::internal::RawPtrTraits)0> > &, 0UL> ./../../base/functional/bind_internal.h:1059:14

    #14 0x55a2b4db90b2 in base::internal::Invoker<base::internal::FunctorTraits<void (DevToolsEmbedderMessageDispatcher::Delegate::* const&)(), DevToolsEmbedderMessageDispatcher::Delegate*>, base::internal::BindState<true, true, false, void (DevToolsEmbedderMessageDispatcher::Delegate::*)(), base::internal::UnretainedWrapper<DevToolsEmbedderMessageDispatcher::Delegate, base::unretained_traits::MayNotDangle, (partition_alloc::internal::RawPtrTraits)0>>, void ()>::Run(base::internal::BindStateBase*) ./../../base/functional/bind_internal.h:979:12

    #15 0x55a28389915c in base::RepeatingCallback<void ()>::Run() const & ./../../base/functional/callback.h:343:12

    #16 0x55a2b4db8161 in Apply<base::RepeatingCallback<void ()> > ./../../chrome/browser/devtools/devtools_embedder_message_dispatcher.cc:321:13

    #17 0x55a2b4db8161 in bool (anonymous namespace)::ParseAndHandle<>(base::RepeatingCallback<void ()> const&, std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>> const&, base::OnceCallback<void (base::Value const*)>, base::ListValue const&) ./../../chrome/browser/devtools/devtools_embedder_message_dispatcher.cc:352:9

    #18 0x55a2b4db8596 in Invoke<bool (*const &)(const base::RepeatingCallback<void ()> &, const std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char> > &, base::OnceCallback<void (const base::Value *)>, const base::ListValue &), const base::RepeatingCallback<void ()> &, const std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char> > &, base::OnceCallback<void (const base::Value *)>, const base::ListValue &> ./../../base/functional/bind_internal.h:663:12

    #19 0x55a2b4db8596 in MakeItSo<bool (*const &)(const base::RepeatingCallback<void ()> &, const std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char> > &, base::OnceCallback<void (const base::Value *)>, const base::ListValue &), const std::__Cr::tuple<base::RepeatingCallback<void ()>, std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char> > > &, base::OnceCallback<void (const base::Value *)>, const base::ListValue &> ./../../base/functional/bind_internal.h:922:12

    #20 0x55a2b4db8596 in RunImpl<bool (*const &)(const base::RepeatingCallback<void ()> &, const std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char> > &, base::OnceCallback<void (const base::Value *)>, const base::ListValue &), const std::__Cr::tuple<base::RepeatingCallback<void ()>, std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char> > > &, 0UL, 1UL> ./../../base/functional/bind_internal.h:1059:14

    #21 0x55a2b4db8596 in base::internal::Invoker<base::internal::FunctorTraits<bool (* const&)(base::RepeatingCallback<void ()> const&, std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>> const&, base::OnceCallback<void (base::Value const*)>, base::ListValue const&), base::RepeatingCallback<void ()> const&, std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>> const&>, base::internal::BindState<false, true, false, bool (*)(base::RepeatingCallback<void ()> const&, std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>> const&, base::OnceCallback<void (base::Value const*)>, base::ListValue const&), base::RepeatingCallback<void ()>, std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>>>, bool (base::OnceCallback<void (base::Value const*)>, base::ListValue const&)>::Run(base::internal::BindStateBase*, base::OnceCallback<void (base::Value const*)>&&, base::ListValue const&) ./../../base/functional/bind_internal.h:979:12

    #22 0x55a2b4db7c9b in base::RepeatingCallback<bool (base::OnceCallback<void (base::Value const*)>, base::ListValue const&)>::Run(base::OnceCallback<void (base::Value const*)>, base::ListValue const&) const & ./../../base/functional/callback.h:343:12

    #23 0x55a2b4db7859 in DispatcherImpl::Dispatch(base::OnceCallback<void (base::Value const*)>, std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>> const&, base::ListValue const&) ./../../chrome/browser/devtools/devtools_embedder_message_dispatcher.cc:390:48

    #24 0x55a283eefc8a in electron::InspectableWebContents::HandleMessageFromDevToolsFrontend(base::DictValue) ./../../electron/shell/browser/ui/inspectable_web_contents.cc:896:33

    #25 0x55a283efad23 in Invoke<void (electron::InspectableWebContents::*)(base::DictValue), electron::InspectableWebContents *, base::DictValue> ./../../base/functional/bind_internal.h:730:12

    #26 0x55a283efad23 in MakeItSo<void (electron::InspectableWebContents::*const &)(base::DictValue), const std::__Cr::tuple<base::internal::UnretainedWrapper<electron::InspectableWebContents, base::unretained_traits::MayNotDangle, (partition_alloc::internal::RawPtrTraits)0> > &, base::DictValue> ./../../base/functional/bind_internal.h:922:12

    #27 0x55a283efad23 in RunImpl<void (electron::InspectableWebContents::*const &)(base::DictValue), const std::__Cr::tuple<base::internal::UnretainedWrapper<electron::InspectableWebContents, base::unretained_traits::MayNotDangle, (partition_alloc::internal::RawPtrTraits)0> > &, 0UL> ./../../base/functional/bind_internal.h:1059:14

    #28 0x55a283efad23 in base::internal::Invoker<base::internal::FunctorTraits<void (electron::InspectableWebContents::* const&)(base::DictValue), electron::InspectableWebContents*>, base::internal::BindState<true, true, false, void (electron::InspectableWebContents::*)(base::DictValue), base::internal::UnretainedWrapper<electron::InspectableWebContents, base::unretained_traits::MayNotDangle, (partition_alloc::internal::RawPtrTraits)0>>, void (base::DictValue)>::Run(base::internal::BindStateBase*, base::DictValue&&) ./../../base/functional/bind_internal.h:979:12

    #29 0x55a2838e7b67 in base::RepeatingCallback<void (base::DictValue)>::Run(base::DictValue) const & ./../../base/functional/callback.h:343:12

    #30 0x55a2955bec64 in DispatchEmbedderMessage ./../../content/browser/devtools/devtools_frontend_host_impl.cc:133:28

    #31 0x55a2955bec64 in non-virtual thunk to content::DevToolsFrontendHostImpl::DispatchEmbedderMessage(base::DictValue) ./../../content/browser/devtools/devtools_frontend_host_impl.cc:0:0

    #32 0x55a28a69d8b0 in blink::mojom::DevToolsFrontendHostStubDispatch::Accept(blink::mojom::DevToolsFrontendHost*, mojo::Message*) ./gen/third_party/blink/public/mojom/devtools/devtools_frontend.mojom.cc:506:13

    #33 0x55a29c4af017 in mojo::InterfaceEndpointClient::HandleValidatedMessage(mojo::Message*) ./../../mojo/public/cpp/bindings/lib/interface_endpoint_client.cc:1086:54

    #34 0x55a29c4d6ce8 in mojo::MessageDispatcher::Accept(mojo::Message*) ./../../mojo/public/cpp/bindings/lib/message_dispatcher.cc:44:19

    #35 0x55a29c4b65fe in mojo::InterfaceEndpointClient::HandleIncomingMessage(mojo::Message*) ./../../mojo/public/cpp/bindings/lib/interface_endpoint_client.cc:748:20

    #36 0x55a29dc76dd6 in IPC::ChannelAssociatedGroupController::AcceptOnEndpointThread(mojo::Message, IPC::(anonymous namespace)::ScopedUrgentMessageNotification) ./../../ipc/ipc_mojo_bootstrap.cc:1200:24

    #37 0x55a29dc796df in Invoke<void (IPC::ChannelAssociatedGroupController::*)(mojo::Message, IPC::(anonymous namespace)::ScopedUrgentMessageNotification), scoped_refptr<IPC::ChannelAssociatedGroupController>, mojo::Message, IPC::(anonymous namespace)::ScopedUrgentMessageNotification> ./../../base/functional/bind_internal.h:730:12

    #38 0x55a29dc796df in MakeItSo<void (IPC::ChannelAssociatedGroupController::*)(mojo::Message, IPC::(anonymous namespace)::ScopedUrgentMessageNotification), std::__Cr::tuple<scoped_refptr<IPC::ChannelAssociatedGroupController>, mojo::Message, IPC::(anonymous namespace)::ScopedUrgentMessageNotification> > ./../../base/functional/bind_internal.h:922:12

    #39 0x55a29dc796df in RunImpl<void (IPC::ChannelAssociatedGroupController::*)(mojo::Message, IPC::(anonymous namespace)::ScopedUrgentMessageNotification), std::__Cr::tuple<scoped_refptr<IPC::ChannelAssociatedGroupController>, mojo::Message, IPC::(anonymous namespace)::ScopedUrgentMessageNotification>, 0UL, 1UL, 2UL> ./../../base/functional/bind_internal.h:1059:14

    #40 0x55a29dc796df in base::internal::Invoker<base::internal::FunctorTraits<void (IPC::ChannelAssociatedGroupController::*&&)(mojo::Message, IPC::(anonymous namespace)::ScopedUrgentMessageNotification), IPC::ChannelAssociatedGroupController*&&, mojo::Message&&, IPC::(anonymous namespace)::ScopedUrgentMessageNotification&&>, base::internal::BindState<true, true, false, void (IPC::ChannelAssociatedGroupController::*)(mojo::Message, IPC::(anonymous namespace)::ScopedUrgentMessageNotification), scoped_refptr<IPC::ChannelAssociatedGroupController>, mojo::Message, IPC::(anonymous namespace)::ScopedUrgentMessageNotification>, void ()>::RunOnce(base::internal::BindStateBase*) ./../../base/functional/bind_internal.h:972:12

    #41 0x55a28389f8d6 in base::OnceCallback<void ()>::Run() && ./../../base/functional/callback.h:155:12

    #42 0x55a29b72b768 in base::TaskAnnotator::RunTaskImpl(base::PendingTask&) ./../../base/task/common/task_annotator.cc:229:34

    #43 0x55a29b7dfaf3 in RunTask<(lambda at ../../base/task/sequence_manager/thread_controller_with_message_pump_impl.cc:474:11)> ./../../base/task/common/task_annotator.h:112:5

    #44 0x55a29b7dfaf3 in base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl(base::LazyNow*) ./../../base/task/sequence_manager/thread_controller_with_message_pump_impl.cc:472:23

    #45 0x55a29b7ddf2e in base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork() ./../../base/task/sequence_manager/thread_controller_with_message_pump_impl.cc:346:40

    #46 0x55a29b9e78a5 in base::MessagePumpGlib::Run(base::MessagePump::Delegate*) ./../../base/message_loop/message_pump_glib.cc:782:48

    #47 0x55a29b7e1ea3 in base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run(bool, base::TimeDelta) ./../../base/task/sequence_manager/thread_controller_with_message_pump_impl.cc:647:12

    #48 0x55a29b6a1b75 in base::RunLoop::Run(base::Location const&) ./../../base/run_loop.cc:135:14

    #49 0x55a292d32885 in content::BrowserMainLoop::RunMainMessageLoop() ./../../content/browser/browser_main_loop.cc:1117:18

    #50 0x55a292d3cb2c in content::BrowserMainRunnerImpl::Run() ./../../content/browser/browser_main_runner_impl.cc:151:15

    #51 0x55a292d28f23 in content::BrowserMain(content::MainFunctionParams) ./../../content/browser/browser_main.cc:32:28

    #52 0x55a2853eef1c in content::RunBrowserProcessMain(content::MainFunctionParams, content::ContentMainDelegate*) ./../../content/app/content_main_runner_impl.cc:708:10

    #53 0x55a2853f389f in content::ContentMainRunnerImpl::RunBrowser(content::MainFunctionParams, bool) ./../../content/app/content_main_runner_impl.cc:1312:10

    #54 0x55a2853f29a9 in content::ContentMainRunnerImpl::Run() ./../../content/app/content_main_runner_impl.cc:1138:12

    #55 0x55a2853eba4c in content::RunContentProcess(content::ContentMainParams, content::ContentMainRunner*) ./../../content/app/content_main.cc:356:36

    #56 0x55a2853ec05e in content::ContentMain(content::ContentMainParams) ./../../content/app/content_main.cc:369:10

    #57 0x55a28387648e in main ./../../electron/shell/app/electron_main_linux.cc:43:10

    #58 0x7f0c2b66ed90 in __libc_start_call_main ./csu/../sysdeps/nptl/libc_start_call_main.h:58:16

    #59 0x7f0c2b66ee40 in __libc_start_main ./csu/../csu/libc-start.c:392:3

    #60 0x55a28374102a in _start ??:0:0

  r8: 0000000000000000  r9: 00007fffffffff01 r10: 00007fffffffff01 r11: 0000000000000001

 r12: 00000000000001a0 r13: 00007b0c28628300 r14: 00000f61850c5060 r15: 0000000000000034

  di: f3f3f3f3f3f3f3f3  si: 00000000000000f3  bp: 00007ffce88ade90  bx: 00007ffce88ade20

  dx: 000000000000000c  ax: 00007b0c28628300  cx: f3f8f2f2f2f2f8f8  sp: 00007ffce88ade20

  ip: 000055a29b5d3afc efl: 0000000000010246 cgf: 002b000000000033 erf: 0000000000000004

 trp: 000000000000000e msk: 0000000000000000 cr2: 00000000000001a0

[end of stack trace]

```

</p>
</details> 

#### Checklist
<!-- Remove items that do not apply. For completed items, change [ ] to [x]. -->

- [x] PR description included
- [x] `npm test` passes
- [x] [PR release notes](https://github.com/electron/clerk/blob/main/README.md) describe the change in a way relevant to app developers, and are [capitalized, punctuated, and past tense](https://github.com/electron/clerk/blob/main/README.md#examples).

#### Release Notes

Notes: Fixed a potential race condition crash when opening DevTools.


Changed files:
filename: shell/browser/api/electron_api_web_contents.cc
patch: @@ -1051,6 +1051,9 @@ void WebContents::InitWithWebContents(
 }
 
 WebContents::~WebContents() {
+  if (inspectable_web_contents_)
+    inspectable_web_contents_->GetView()->SetDelegate(nullptr);
+
   if (owner_window_) {
     owner_window_->RemoveBackgroundThrottlingSource(this);
   }
@@ -1065,8 +1068,6 @@ WebContents::~WebContents() {
     return;
   }
 
-  inspectable_web_contents_->GetView()->SetDelegate(nullptr);
-
   // This event is only for internal use, which is emitted when WebContents is
   // being destroyed.
   Emit("will-destroy");
@@ -2202,8 +2203,8 @@ void WebContents::DevToolsOpened() {
   // Inherit owner window in devtools when it doesn't have one.
   auto* devtools = inspectable_web_contents_->GetDevToolsWebContents();
   bool has_window = devtools->GetUserData(NativeWindowRelay::UserDataKey());
-  if (owner_window_ && !has_window) {
-    DCHECK(!owner_window_.WasInvalidated());
+  if (owner_window() && !has_window) {
+    CHECK(!owner_window_.WasInvalidated());
     DCHECK_EQ(handle->owner_window(), nullptr);
     handle->SetOwnerWindow(devtools, owner_window());
   }