From 5c5cf3c66e7fad29f8cf65c7e38b218502d5e5af Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 11 Apr 2014 18:43:01 +0800 Subject: [PATCH] Fix a crash when destroying window. --- atom/browser/native_window.cc | 10 +++++++--- atom/browser/native_window.h | 10 ++++++++-- atom/browser/native_window_mac.mm | 2 +- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/atom/browser/native_window.cc b/atom/browser/native_window.cc index 95fa029bf20b..088f1427947e 100644 --- a/atom/browser/native_window.cc +++ b/atom/browser/native_window.cc @@ -57,12 +57,12 @@ NativeWindow::NativeWindow(content::WebContents* web_contents, base::DictionaryValue* options) : content::WebContentsObserver(web_contents), has_frame_(true), - inspectable_web_contents_( - brightray::InspectableWebContents::Create(web_contents)), is_closed_(false), node_integration_("except-iframe"), has_dialog_attached_(false), - weak_factory_(this) { + weak_factory_(this), + inspectable_web_contents_( + brightray::InspectableWebContents::Create(web_contents)) { options->GetBoolean(switches::kFrame, &has_frame_); #if defined(OS_MACOSX) @@ -365,6 +365,10 @@ void NativeWindow::NotifyWindowBlur() { FOR_EACH_OBSERVER(NativeWindowObserver, observers_, OnWindowBlur()); } +void NativeWindow::DestroyWebContents() { + inspectable_web_contents_.reset(); +} + // In atom-shell all reloads and navigations started by renderer process would // be redirected to this method, so we can have precise control of how we // would open the url (in our case, is to restart the renderer process). See diff --git a/atom/browser/native_window.h b/atom/browser/native_window.h index 17b06b45d67c..47ce314fbfa5 100644 --- a/atom/browser/native_window.h +++ b/atom/browser/native_window.h @@ -196,6 +196,9 @@ class NativeWindow : public brightray::DefaultWebContentsDelegate, void NotifyWindowClosed(); void NotifyWindowBlur(); + // Destroy the inspectable_web_contents. + void DestroyWebContents(); + // Called when the window needs to update its draggable region. virtual void UpdateDraggableRegions( const std::vector& regions) = 0; @@ -251,8 +254,6 @@ class NativeWindow : public brightray::DefaultWebContentsDelegate, // Window icon. gfx::Image icon_; - scoped_ptr inspectable_web_contents_; - private: // Schedule a notification unresponsive event. void ScheduleUnresponsiveEvent(int ms); @@ -307,6 +308,11 @@ class NativeWindow : public brightray::DefaultWebContentsDelegate, scoped_ptr dialog_manager_; + // Notice that inspectable_web_contents_ must be placed after dialog_manager_, + // so we can make sure inspectable_web_contents_ is destroyed before + // dialog_manager_, otherwise a crash would happen. + scoped_ptr inspectable_web_contents_; + // Maps url to file path, used by the file requests sent from devtools. typedef std::map PathsMap; PathsMap saved_files_; diff --git a/atom/browser/native_window_mac.mm b/atom/browser/native_window_mac.mm index d0ea3efb88e5..673da67c3495 100644 --- a/atom/browser/native_window_mac.mm +++ b/atom/browser/native_window_mac.mm @@ -201,7 +201,7 @@ NativeWindowMac::NativeWindowMac(content::WebContents* web_contents, NativeWindowMac::~NativeWindowMac() { // Force InspectableWebContents to be destroyed before we destroy window, // because it may still be observing the window at this time. - inspectable_web_contents_.reset(); + DestroyWebContents(); } void NativeWindowMac::Close() {