From 8417f158b416c48c7ffa135e847c3f618109426f Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 23 Apr 2014 09:53:38 +0800 Subject: [PATCH] Do not free memory used by native window immediately. Otherwise we would have two ways to destroy a window, making code much more complicated. --- atom/browser/api/atom_api_window.cc | 8 +------- atom/browser/native_window.cc | 26 ++++++++++++++++++-------- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/atom/browser/api/atom_api_window.cc b/atom/browser/api/atom_api_window.cc index 6c8544d8a178..1aa67b93113d 100644 --- a/atom/browser/api/atom_api_window.cc +++ b/atom/browser/api/atom_api_window.cc @@ -97,13 +97,7 @@ void Window::WillCloseWindow(bool* prevent_default) { void Window::OnWindowClosed() { Emit("closed"); - if (window_) { - window_->RemoveObserver(this); - - // Free memory when native window is closed, the delete is delayed so other - // observers would not get a invalid pointer of NativeWindow. - base::MessageLoop::current()->DeleteSoon(FROM_HERE, window_.release()); - } + window_->RemoveObserver(this); } void Window::OnWindowBlur() { diff --git a/atom/browser/native_window.cc b/atom/browser/native_window.cc index fa39ac5677f3..0c9211ec1281 100644 --- a/atom/browser/native_window.cc +++ b/atom/browser/native_window.cc @@ -99,6 +99,10 @@ NativeWindow::NativeWindow(content::WebContents* web_contents, } NativeWindow::~NativeWindow() { + // Make sure we have the OnRenderViewDeleted message sent even when the window + // is destroyed directly. + DestroyWebContents(); + // It's possible that the windows gets destroyed before it's closed, in that // case we need to ensure the OnWindowClosed message is still notified. NotifyWindowClosed(); @@ -298,6 +302,17 @@ void NativeWindow::CapturePage(const gfx::Rect& rect, } void NativeWindow::DestroyWebContents() { + if (!inspectable_web_contents_) + return; + + // The OnRenderViewDeleted is not called when the WebContents is destroyed + // directly (e.g. when closing the window), so we make sure it's always + // emitted to users by sending it before window is closed.. + FOR_EACH_OBSERVER(NativeWindowObserver, observers_, + OnRenderViewDeleted( + GetWebContents()->GetRenderProcessHost()->GetID(), + GetWebContents()->GetRoutingID())); + inspectable_web_contents_.reset(); } @@ -351,14 +366,6 @@ void NativeWindow::NotifyWindowClosed() { if (is_closed_) return; - // The OnRenderViewDeleted is not called when the WebContents is destroyed - // directly (e.g. when closing the window), so we make sure it's always - // emitted to users by sending it before window is closed.. - FOR_EACH_OBSERVER(NativeWindowObserver, observers_, - OnRenderViewDeleted( - GetWebContents()->GetRenderProcessHost()->GetID(), - GetWebContents()->GetRoutingID())); - is_closed_ = true; FOR_EACH_OBSERVER(NativeWindowObserver, observers_, OnWindowClosed()); @@ -444,6 +451,9 @@ void NativeWindow::MoveContents(content::WebContents* source, } void NativeWindow::CloseContents(content::WebContents* source) { + // Destroy the WebContents before we close the window. + DestroyWebContents(); + // 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