refactor: cleanup how WebContents is destroyed (#27920)

shell/browser/api/electron_api_base_window.cc

@@ -114,8 +114,7 @@ BaseWindow::BaseWindow(gin_helper::Arguments* args,
 }
 
 BaseWindow::~BaseWindow() {
-  if (!window_->IsClosed())
+  CloseImmediately();
-    window_->CloseImmediately();
 
   // Destroy the native window in next tick because the native code might be
   // iterating all windows.
@@ -318,6 +317,11 @@ void BaseWindow::SetContentView(gin::Handle<View> view) {
   window_->SetContentView(view->view());
 }
 
+void BaseWindow::CloseImmediately() {
+  if (!window_->IsClosed())
+    window_->CloseImmediately();
+}
+
 void BaseWindow::Close() {
   window_->Close();
 }

shell/browser/api/electron_api_base_window.h

@@ -92,6 +92,7 @@ class BaseWindow : public gin_helper::TrackableObject<BaseWindow>,
   // Public APIs of NativeWindow.
   void SetContentView(gin::Handle<View> view);
   void Close();
+  virtual void CloseImmediately();
   virtual void Focus();
   virtual void Blur();
   bool IsFocused();

shell/browser/api/electron_api_browser_view.cc

@@ -100,11 +100,9 @@ BrowserView::BrowserView(gin::Arguments* args,
 }
 
 BrowserView::~BrowserView() {
-  if (api_web_contents_) {  // destroy() is called
+  if (api_web_contents_) {  // destroy() called without closing WebContents
-    // Destroy WebContents asynchronously unless app is shutting down,
-    // because destroy() might be called inside WebContents's event handler.
     api_web_contents_->RemoveObserver(this);
-    api_web_contents_->DestroyWebContents(!Browser::Get()->is_shutting_down());
+    api_web_contents_->Destroy();
   }
 }
 

shell/browser/api/electron_api_browser_window.cc

@@ -104,13 +104,16 @@ BrowserWindow::BrowserWindow(gin::Arguments* args,
 }
 
 BrowserWindow::~BrowserWindow() {
-  // FIXME This is a hack rather than a proper fix preventing shutdown crashes.
+  if (api_web_contents_) {
-  if (api_web_contents_)
+    // Cleanup the observers if user destroyed this instance directly instead of
+    // gracefully closing content::WebContents.
+    auto* host = web_contents()->GetRenderViewHost();
+    if (host)
+      host->GetWidget()->RemoveInputEventObserver(this);
     api_web_contents_->RemoveObserver(this);
-  // Note that the OnWindowClosed will not be called after the destructor runs,
+    // Destroy the WebContents.
-  // since the window object is managed by the BaseWindow class.
+    OnCloseContents();
-  if (web_contents())
+  }
-    Cleanup();
 }
 
 void BrowserWindow::OnInputEvent(const blink::WebInputEvent& event) {
@@ -173,34 +176,14 @@ void BrowserWindow::OnRendererUnresponsive(content::RenderProcessHost*) {
   ScheduleUnresponsiveEvent(50);
 }
 
+void BrowserWindow::WebContentsDestroyed() {
+  api_web_contents_ = nullptr;
+  CloseImmediately();
+}
+
 void BrowserWindow::OnCloseContents() {
-  // On some machines it may happen that the window gets destroyed for twice,
+  BaseWindow::ResetBrowserViews();
-  // checking web_contents() can effectively guard against that.
+  api_web_contents_->Destroy();
-  // https://github.com/electron/electron/issues/16202.
-  //
-  // TODO(zcbenz): We should find out the root cause and improve the closing
-  // procedure of BrowserWindow.
-  if (!web_contents())
-    return;
-
-  // Close all child windows before closing current window.
-  v8::Locker locker(isolate());
-  v8::HandleScope handle_scope(isolate());
-  for (v8::Local<v8::Value> value : child_windows_.Values(isolate())) {
-    gin::Handle<BrowserWindow> child;
-    if (gin::ConvertFromV8(isolate(), value, &child) && !child.IsEmpty())
-      child->window()->CloseImmediately();
-  }
-
-  // When the web contents is gone, close the window immediately, but the
-  // memory will not be freed until you call delete.
-  // In this way, it would be safe to manage windows via smart pointers. If you
-  // want to free memory when the window is closed, you can do deleting by
-  // overriding the OnWindowClosed method in the observer.
-  window()->CloseImmediately();
-
-  // Do not sent "unresponsive" event after window is closed.
-  window_unresponsive_closure_.Cancel();
 }
 
 void BrowserWindow::OnRendererResponsive(content::RenderProcessHost*) {
@@ -268,30 +251,22 @@ void BrowserWindow::OnCloseButtonClicked(bool* prevent_default) {
     web_contents()->Close();
 }
 
-void BrowserWindow::OnWindowClosed() {
-  // We need to reset the browser views before we call Cleanup, because the
-  // browser views are child views of the main web contents view, which will be
-  // deleted by Cleanup.
-  BaseWindow::ResetBrowserViews();
-  Cleanup();
-  // See BaseWindow::OnWindowClosed on why calling InvalidateWeakPtrs.
-  weak_factory_.InvalidateWeakPtrs();
-  BaseWindow::OnWindowClosed();
-}
-
 void BrowserWindow::OnWindowBlur() {
-  web_contents()->StoreFocus();
+  if (api_web_contents_)
+    web_contents()->StoreFocus();
 
   BaseWindow::OnWindowBlur();
 }
 
 void BrowserWindow::OnWindowFocus() {
-  web_contents()->RestoreFocus();
+  // focus/blur events might be emitted while closing window.
-
+  if (api_web_contents_) {
+    web_contents()->RestoreFocus();
 #if !defined(OS_MAC)
-  if (!api_web_contents_->IsDevToolsOpened())
+    if (!api_web_contents_->IsDevToolsOpened())
-    web_contents()->Focus();
+      web_contents()->Focus();
 #endif
+  }
 
   BaseWindow::OnWindowFocus();
 }
@@ -325,6 +300,22 @@ void BrowserWindow::OnWindowLeaveFullScreen() {
   BaseWindow::OnWindowLeaveFullScreen();
 }
 
+void BrowserWindow::CloseImmediately() {
+  // Close all child windows before closing current window.
+  v8::Locker locker(isolate());
+  v8::HandleScope handle_scope(isolate());
+  for (v8::Local<v8::Value> value : child_windows_.Values(isolate())) {
+    gin::Handle<BrowserWindow> child;
+    if (gin::ConvertFromV8(isolate(), value, &child) && !child.IsEmpty())
+      child->window()->CloseImmediately();
+  }
+
+  BaseWindow::CloseImmediately();
+
+  // Do not sent "unresponsive" event after window is closed.
+  window_unresponsive_closure_.Cancel();
+}
+
 void BrowserWindow::Focus() {
   if (api_web_contents_->IsOffScreen())
     FocusOnWebView();
@@ -448,17 +439,6 @@ void BrowserWindow::NotifyWindowUnresponsive() {
   }
 }
 
-void BrowserWindow::Cleanup() {
-  auto* host = web_contents()->GetRenderViewHost();
-  if (host)
-    host->GetWidget()->RemoveInputEventObserver(this);
-
-  // Destroy WebContents asynchronously unless app is shutting down,
-  // because destroy() might be called inside WebContents's event handler.
-  api_web_contents_->DestroyWebContents(!Browser::Get()->is_shutting_down());
-  Observe(nullptr);
-}
-
 void BrowserWindow::OnWindowShow() {
   web_contents()->WasShown();
   BaseWindow::OnWindowShow();

shell/browser/api/electron_api_browser_window.h

@@ -54,6 +54,7 @@ class BrowserWindow : public BaseWindow,
   void OnRendererUnresponsive(content::RenderProcessHost*) override;
   void OnRendererResponsive(
       content::RenderProcessHost* render_process_host) override;
+  void WebContentsDestroyed() override;
 
   // ExtendedWebContentsObserver:
   void OnCloseContents() override;
@@ -73,11 +74,11 @@ class BrowserWindow : public BaseWindow,
   void OnWindowIsKeyChanged(bool is_key) override;
 
   // BaseWindow:
-  void OnWindowClosed() override;
   void OnWindowBlur() override;
   void OnWindowFocus() override;
   void OnWindowResize() override;
   void OnWindowLeaveFullScreen() override;
+  void CloseImmediately() override;
   void Focus() override;
   void Blur() override;
   void SetBackgroundColor(const std::string& color_name) override;
@@ -114,9 +115,6 @@ class BrowserWindow : public BaseWindow,
   // Dispatch unresponsive event to observers.
   void NotifyWindowUnresponsive();
 
-  // Cleanup our WebContents observers.
-  void Cleanup();
-
   // Closure that would be called when window is unresponsive when closing,
   // it should be cancelled when we can prove that the window is responsive.
   base::CancelableClosure window_unresponsive_closure_;

shell/browser/api/electron_api_web_contents.cc

@@ -909,51 +909,49 @@ void WebContents::InitWithWebContents(content::WebContents* web_contents,
 }
 
 WebContents::~WebContents() {
-  MarkDestroyed();
+  if (!inspectable_web_contents_) {
-  // The destroy() is called.
+    WebContentsDestroyed();
-  if (inspectable_web_contents_) {
+    return;
-#if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
-    if (type_ == Type::kBackgroundPage) {
-      // Background pages are owned by extensions::ExtensionHost
-      inspectable_web_contents_->ReleaseWebContents();
-    }
-#endif
-
-    inspectable_web_contents_->GetView()->SetDelegate(nullptr);
-
-    if (web_contents()) {
-      RenderViewDeleted(web_contents()->GetRenderViewHost());
-    }
-
-    if (type_ == Type::kBrowserWindow && owner_window()) {
-      // For BrowserWindow we should close the window and clean up everything
-      // before WebContents is destroyed.
-      for (ExtendedWebContentsObserver& observer : observers_)
-        observer.OnCloseContents();
-      // BrowserWindow destroys WebContents asynchronously, manually emit the
-      // destroyed event here.
-      WebContentsDestroyed();
-    } else if (Browser::Get()->is_shutting_down()) {
-      // Destroy WebContents directly when app is shutting down.
-      DestroyWebContents(false /* async */);
-    } else {
-      // Destroy WebContents asynchronously unless app is shutting down,
-      // because destroy() might be called inside WebContents's event handler.
-      bool is_browser_view = type_ == Type::kBrowserView;
-      DestroyWebContents(!(IsGuest() || is_browser_view) /* async */);
-      // The WebContentsDestroyed will not be called automatically because we
-      // destroy the webContents in the next tick. So we have to manually
-      // call it here to make sure "destroyed" event is emitted.
-      WebContentsDestroyed();
-    }
   }
-}
 
-void WebContents::DestroyWebContents(bool async) {
+  if (guest_delegate_)
+    guest_delegate_->WillDestroy();
+
   // This event is only for internal use, which is emitted when WebContents is
   // being destroyed.
   Emit("will-destroy");
-  ResetManagedWebContents(async);
+
+  // For guest view based on OOPIF, the WebContents is released by the embedder
+  // frame, and we need to clear the reference to the memory.
+  bool not_owned_by_this = IsGuest() && attached_;
+#if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
+  // And background pages are owned by extensions::ExtensionHost.
+  if (type_ == Type::kBackgroundPage)
+    not_owned_by_this = true;
+#endif
+  if (not_owned_by_this) {
+    inspectable_web_contents_->ReleaseWebContents();
+    WebContentsDestroyed();
+  }
+
+  // InspectableWebContents will be automatically destroyed.
+}
+
+void WebContents::Destroy() {
+  // The content::WebContents should be destroyed asyncronously when possible
+  // as user may choose to destroy WebContents during an event of it.
+  if (Browser::Get()->is_shutting_down() || IsGuest() ||
+      type_ == Type::kBrowserView) {
+    delete this;
+  } else {
+    base::PostTask(FROM_HERE, {content::BrowserThread::UI},
+                   base::BindOnce(
+                       [](base::WeakPtr<WebContents> contents) {
+                         if (contents)
+                           delete contents.get();
+                       },
+                       GetWeakPtr()));
+  }
 }
 
 bool WebContents::DidAddMessageToConsole(
@@ -1063,7 +1061,7 @@ void WebContents::AddNewContents(
            initial_rect.x(), initial_rect.y(), initial_rect.width(),
            initial_rect.height(), tracker->url, tracker->frame_name,
            tracker->referrer, tracker->raw_features, tracker->body)) {
-    api_web_contents->DestroyWebContents(false /* async */);
+    api_web_contents->Destroy();
   }
 }
 
@@ -1133,8 +1131,6 @@ void WebContents::CloseContents(content::WebContents* source) {
     autofill_driver_factory->CloseAllPopups();
   }
 
-  if (inspectable_web_contents_)
-    inspectable_web_contents_->GetView()->SetDelegate(nullptr);
   for (ExtendedWebContentsObserver& observer : observers_)
     observer.OnCloseContents();
 }
@@ -1811,21 +1807,6 @@ void WebContents::SetOwnerWindow(content::WebContents* web_contents,
 #endif
 }
 
-void WebContents::ResetManagedWebContents(bool async) {
-  if (async) {
-    // Browser context should be destroyed only after the WebContents,
-    // this is guaranteed in the sync mode by the order of declaration,
-    // in the async version we maintain a reference until the WebContents
-    // is destroyed.
-    // //electron/patches/chromium/content_browser_main_loop.patch
-    // is required to get the right quit closure for the main message loop.
-    base::ThreadTaskRunnerHandle::Get()->DeleteSoon(
-        FROM_HERE, inspectable_web_contents_.release());
-  } else {
-    inspectable_web_contents_.reset();
-  }
-}
-
 content::WebContents* WebContents::GetWebContents() const {
   if (!inspectable_web_contents_)
     return nullptr;
@@ -1838,7 +1819,8 @@ content::WebContents* WebContents::GetDevToolsWebContents() const {
   return inspectable_web_contents_->GetDevToolsWebContents();
 }
 
-void WebContents::MarkDestroyed() {
+void WebContents::WebContentsDestroyed() {
+  // Clear the pointer stored in wrapper.
   if (GetAllWebContents().Lookup(id_))
     GetAllWebContents().Remove(id_);
   v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
@@ -1847,52 +1829,9 @@ void WebContents::MarkDestroyed() {
   if (!GetWrapper(isolate).ToLocal(&wrapper))
     return;
   wrapper->SetAlignedPointerInInternalField(0, nullptr);
-}
 
-// There are three ways of destroying a webContents:
+  Observe(nullptr);
-// 1. call webContents.destroy();
-// 2. garbage collection;
-// 3. user closes the window of webContents;
-// 4. the embedder detaches the frame.
-// For webview only #4 will happen, for BrowserWindow both #1 and #3 may
-// happen. The #2 should never happen for webContents, because webview is
-// managed by GuestViewManager, and BrowserWindow's webContents is managed
-// by api::BrowserWindow.
-// For #1, the destructor will do the cleanup work and we only need to make
-// sure "destroyed" event is emitted. For #3, the content::WebContents will
-// be destroyed on close, and WebContentsDestroyed would be called for it, so
-// we need to make sure the api::WebContents is also deleted.
-// For #4, the WebContents will be destroyed by embedder.
-void WebContents::WebContentsDestroyed() {
-  // Give chance for guest delegate to cleanup its observers
-  // since the native class is only destroyed in the next tick.
-  if (guest_delegate_)
-    guest_delegate_->WillDestroy();
-
-  // Cleanup relationships with other parts.
-
-  // We can not call Destroy here because we need to call Emit first, but we
-  // also do not want any method to be used, so just mark as destroyed here.
-  MarkDestroyed();
-
-  Observe(nullptr);  // this->web_contents() will return nullptr
   Emit("destroyed");
-
-  // For guest view based on OOPIF, the WebContents is released by the embedder
-  // frame, and we need to clear the reference to the memory.
-  if (IsGuest() && inspectable_web_contents_) {
-    inspectable_web_contents_->ReleaseWebContents();
-    ResetManagedWebContents(false);
-  }
-
-  // Destroy the native class in next tick.
-  base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE, base::BindOnce(
-                     [](base::WeakPtr<WebContents> wc) {
-                       if (wc)
-                         delete wc.get();
-                     },
-                     GetWeakPtr()));
 }
 
 void WebContents::NavigationEntryCommitted(
@@ -2885,6 +2824,7 @@ bool WebContents::IsGuest() const {
 
 void WebContents::AttachToIframe(content::WebContents* embedder_web_contents,
                                  int embedder_frame_id) {
+  attached_ = true;
   if (guest_delegate_)
     guest_delegate_->AttachToIframe(embedder_web_contents, embedder_frame_id);
 }
@@ -3537,17 +3477,11 @@ v8::Local<v8::ObjectTemplate> WebContents::FillObjectTemplate(
       gin::CreateFunctionTemplate(
           isolate, base::BindRepeating(&gin_helper::Destroyable::IsDestroyed),
           options));
-  templ->Set(
-      gin::StringToSymbol(isolate, "destroy"),
-      gin::CreateFunctionTemplate(
-          isolate, base::BindRepeating([](gin::Handle<WebContents> handle) {
-            delete handle.get();
-          }),
-          options));
   // We use gin_helper::ObjectTemplateBuilder instead of
   // gin::ObjectTemplateBuilder here to handle the fact that WebContents is
   // destroyable.
   return gin_helper::ObjectTemplateBuilder(isolate, templ)
+      .SetMethod("destroy", &WebContents::Destroy)
       .SetMethod("getBackgroundThrottling",
                  &WebContents::GetBackgroundThrottling)
       .SetMethod("setBackgroundThrottling",

shell/browser/api/electron_api_web_contents.h

@@ -144,23 +144,9 @@ class WebContents : public gin::Wrappable<WebContents>,
       v8::Local<v8::ObjectTemplate>);
   const char* GetTypeName() override;
 
+  void Destroy();
   base::WeakPtr<WebContents> GetWeakPtr() { return weak_factory_.GetWeakPtr(); }
 
-  // Destroy the managed content::WebContents instance.
-  //
-  // Note: The |async| should only be |true| when users are expecting to use the
-  // webContents immediately after the call. Always pass |false| if you are not
-  // sure.
-  // See https://github.com/electron/electron/issues/8930.
-  //
-  // Note: When destroying a webContents member inside a destructor, the |async|
-  // should always be |false|, otherwise the destroy task might be delayed after
-  // normal shutdown procedure, resulting in an assertion.
-  // The normal pattern for calling this method in destructor is:
-  // api_web_contents_->DestroyWebContents(!Browser::Get()->is_shutting_down())
-  // See https://github.com/electron/electron/issues/15133.
-  void DestroyWebContents(bool async);
-
   bool GetBackgroundThrottling() const;
   void SetBackgroundThrottling(bool allowed);
   int GetProcessID() const;
@@ -362,8 +348,6 @@ class WebContents : public gin::Wrappable<WebContents>,
     return EmitCustomEvent(name, event, std::forward<Args>(args)...);
   }
 
-  void MarkDestroyed();
-
   WebContents* embedder() { return embedder_; }
 
 #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
@@ -672,9 +656,6 @@ class WebContents : public gin::Wrappable<WebContents>,
                                 std::string* class_name) override;
 #endif
 
-  // Destroy the managed InspectableWebContents object.
-  void ResetManagedWebContents(bool async);
-
   // DevTools index event callbacks.
   void OnDevToolsIndexingWorkCalculated(int request_id,
                                         const std::string& file_system_path,
@@ -706,6 +687,9 @@ class WebContents : public gin::Wrappable<WebContents>,
   // The host webcontents that may contain this webcontents.
   WebContents* embedder_ = nullptr;
 
+  // Whether the guest view has been attached.
+  bool attached_ = false;
+
   // The zoom controller for this webContents.
   WebContentsZoomController* zoom_controller_ = nullptr;
 

shell/browser/api/electron_api_web_contents_view.cc

@@ -44,18 +44,8 @@ WebContentsView::WebContentsView(v8::Isolate* isolate,
 }
 
 WebContentsView::~WebContentsView() {
-  if (api_web_contents_) {  // destroy() is called
+  if (api_web_contents_)  // destroy() called without closing WebContents
-    // Destroy WebContents asynchronously, as we might be in GC currently and
+    api_web_contents_->Destroy();
-    // WebContents emits an event in its destructor.
-    base::PostTask(FROM_HERE, {content::BrowserThread::UI},
-                   base::BindOnce(
-                       [](base::WeakPtr<WebContents> contents) {
-                         if (contents)
-                           contents->DestroyWebContents(
-                               !Browser::Get()->is_shutting_down());
-                       },
-                       api_web_contents_->GetWeakPtr()));
-  }
 }
 
 gin::Handle<WebContents> WebContentsView::GetWebContents(v8::Isolate* isolate) {

shell/browser/ui/cocoa/electron_inspectable_web_contents_view.h

@@ -34,13 +34,13 @@ using electron::InspectableWebContentsViewMac;
   BOOL devtools_visible_;
   BOOL devtools_docked_;
   BOOL devtools_is_first_responder_;
+  BOOL attached_to_window_;
 
   DevToolsContentsResizingStrategy strategy_;
 }
 
 - (instancetype)initWithInspectableWebContentsViewMac:
     (InspectableWebContentsViewMac*)view;
-- (void)removeObservers;
 - (void)notifyDevToolsFocused;
 - (void)setDevToolsVisible:(BOOL)visible activate:(BOOL)activate;
 - (BOOL)isDevToolsVisible;

shell/browser/ui/cocoa/electron_inspectable_web_contents_view.mm

@@ -35,18 +35,7 @@
   devtools_visible_ = NO;
   devtools_docked_ = NO;
   devtools_is_first_responder_ = NO;
-
+  attached_to_window_ = NO;
-  [[NSNotificationCenter defaultCenter]
-      addObserver:self
-         selector:@selector(viewDidBecomeFirstResponder:)
-             name:kViewDidBecomeFirstResponder
-           object:nil];
-
-  [[NSNotificationCenter defaultCenter]
-      addObserver:self
-         selector:@selector(parentWindowBecameMain:)
-             name:NSWindowDidBecomeMainNotification
-           object:nil];
 
   if (inspectableWebContentsView_->inspectable_web_contents()->IsGuest()) {
     fake_view_.reset([[NSView alloc] init]);
@@ -69,14 +58,34 @@
   return self;
 }
 
-- (void)removeObservers {
+- (void)dealloc {
   [[NSNotificationCenter defaultCenter] removeObserver:self];
+  [super dealloc];
 }
 
 - (void)resizeSubviewsWithOldSize:(NSSize)oldBoundsSize {
   [self adjustSubviews];
 }
 
+- (void)viewDidMoveToWindow {
+  if (attached_to_window_ && !self.window) {
+    attached_to_window_ = NO;
+    [[NSNotificationCenter defaultCenter] removeObserver:self];
+  } else if (!attached_to_window_ && self.window) {
+    attached_to_window_ = YES;
+    [[NSNotificationCenter defaultCenter]
+        addObserver:self
+           selector:@selector(viewDidBecomeFirstResponder:)
+               name:kViewDidBecomeFirstResponder
+             object:nil];
+    [[NSNotificationCenter defaultCenter]
+        addObserver:self
+           selector:@selector(parentWindowBecameMain:)
+               name:NSWindowDidBecomeMainNotification
+             object:nil];
+  }
+}
+
 - (IBAction)showDevTools:(id)sender {
   inspectableWebContentsView_->inspectable_web_contents()->ShowDevTools(true);
 }
@@ -253,8 +262,7 @@
 - (void)viewDidBecomeFirstResponder:(NSNotification*)notification {
   auto* inspectable_web_contents =
       inspectableWebContentsView_->inspectable_web_contents();
-  if (!inspectable_web_contents || inspectable_web_contents->IsGuest())
+  DCHECK(inspectable_web_contents);
-    return;
   auto* webContents = inspectable_web_contents->GetWebContents();
   auto* webContentsView = webContents->GetNativeView().GetNativeNSView();
 

shell/browser/ui/inspectable_web_contents.cc

@@ -371,13 +371,8 @@ InspectableWebContents::InspectableWebContents(
 InspectableWebContents::~InspectableWebContents() {
   g_web_contents_instances_.remove(this);
   // Unsubscribe from devtools and Clean up resources.
-  if (GetDevToolsWebContents()) {
+  if (GetDevToolsWebContents())
-    if (managed_devtools_web_contents_)
-      managed_devtools_web_contents_->SetDelegate(nullptr);
-    // Calling this also unsubscribes the observer, so WebContentsDestroyed
-    // won't be called again.
     WebContentsDestroyed();
-  }
   // Let destructor destroy managed_devtools_web_contents_.
 }
 
@@ -416,6 +411,8 @@ bool InspectableWebContents::IsGuest() const {
 
 void InspectableWebContents::ReleaseWebContents() {
   web_contents_.release();
+  WebContentsDestroyed();
+  view_.reset();
 }
 
 void InspectableWebContents::SetDockState(const std::string& state) {
@@ -936,6 +933,9 @@ void InspectableWebContents::RenderFrameHostChanged(
 }
 
 void InspectableWebContents::WebContentsDestroyed() {
+  if (managed_devtools_web_contents_)
+    managed_devtools_web_contents_->SetDelegate(nullptr);
+
   frontend_loaded_ = false;
   external_devtools_web_contents_ = nullptr;
   Observe(nullptr);

shell/browser/ui/inspectable_web_contents_view_mac.mm

@@ -26,7 +26,6 @@ InspectableWebContentsViewMac::InspectableWebContentsViewMac(
           initWithInspectableWebContentsViewMac:this]) {}
 
 InspectableWebContentsViewMac::~InspectableWebContentsViewMac() {
-  [view_ removeObservers];
   CloseDevTools();
 }