From 9eeec9aa0b983c08abb81fe8b23df66d53830fd3 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 25 Apr 2014 11:22:51 +0800 Subject: [PATCH] Be safe on lifetime of webContents. --- atom/browser/api/atom_api_web_contents.cc | 10 ++++++++++ atom/browser/api/atom_api_web_contents.h | 16 ++++++++-------- atom/browser/api/lib/browser-window.coffee | 18 +++++++++++++----- 3 files changed, 31 insertions(+), 13 deletions(-) diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc index d737914011c1..782e5893f459 100644 --- a/atom/browser/api/atom_api_web_contents.cc +++ b/atom/browser/api/atom_api_web_contents.cc @@ -17,11 +17,20 @@ namespace api { WebContents::WebContents(content::WebContents* web_contents) : web_contents_(web_contents) { + Observe(web_contents_); } WebContents::~WebContents() { } +void WebContents::WebContentsDestroyed(content::WebContents*) { + web_contents_ = NULL; +} + +bool WebContents::IsAlive() const { + return web_contents_ != NULL; +} + GURL WebContents::GetURL() const { return web_contents_->GetURL(); } @@ -62,6 +71,7 @@ void WebContents::ExecuteJavaScript(const string16& code) { mate::ObjectTemplateBuilder WebContents::GetObjectTemplateBuilder( v8::Isolate* isolate) { return mate::ObjectTemplateBuilder(isolate) + .SetMethod("isAlive", &WebContents::IsAlive) .SetMethod("getUrl", &WebContents::GetURL) .SetMethod("getTitle", &WebContents::GetTitle) .SetMethod("isLoading", &WebContents::IsLoading) diff --git a/atom/browser/api/atom_api_web_contents.h b/atom/browser/api/atom_api_web_contents.h index c1311f9bb938..b15b604903ad 100644 --- a/atom/browser/api/atom_api_web_contents.h +++ b/atom/browser/api/atom_api_web_contents.h @@ -6,23 +6,20 @@ #define ATOM_BROWSER_API_ATOM_API_WEB_CONTENTS_H_ #include "atom/browser/api/event_emitter.h" +#include "content/public/browser/web_contents_observer.h" #include "native_mate/handle.h" -class GURL; - -namespace content { -class WebContents; -} - namespace atom { namespace api { -class WebContents : public mate::EventEmitter { +class WebContents : public mate::EventEmitter, + public content::WebContentsObserver { public: static mate::Handle Create(v8::Isolate* isolate, content::WebContents* web_contents); + bool IsAlive() const; GURL GetURL() const; string16 GetTitle() const; bool IsLoading() const; @@ -39,7 +36,10 @@ class WebContents : public mate::EventEmitter { // mate::Wrappable implementations: virtual mate::ObjectTemplateBuilder GetObjectTemplateBuilder( - v8::Isolate* isolate); + v8::Isolate* isolate) OVERRIDE; + + // content::WebContentsObserver implementations: + virtual void WebContentsDestroyed(content::WebContents*) OVERRIDE; private: content::WebContents* web_contents_; // Weak. diff --git a/atom/browser/api/lib/browser-window.coffee b/atom/browser/api/lib/browser-window.coffee index 1687604f2f97..04b0a6f4ea33 100644 --- a/atom/browser/api/lib/browser-window.coffee +++ b/atom/browser/api/lib/browser-window.coffee @@ -15,15 +15,23 @@ BrowserWindow::_init = -> @setMenu menu if menu? # Define getter for webContents. - @__webContents = null + webContents = null @__defineGetter__ 'webContents', -> - @__webContents ?= @getWebContents() - @__devToolsWebContents = null + webContents ?= @getWebContents() + # Return null if webContents is destroyed. + webContents = null unless webContents?.isAlive() + webContents + + # And devToolsWebContents. + devToolsWebContents = null @__defineGetter__ 'devToolsWebContents', -> if @isDevToolsOpened() - @__devToolsWebContents ?= @getDevToolsWebContents() + # Get the new devToolsWebContents if previous one has been destroyed, it + # could happen when the devtools has been closed and then reopened. + devToolsWebContents = null unless devToolsWebContents?.isAlive() + devToolsWebContents ?= @getDevToolsWebContents() else - @__devToolsWebContents = null + devToolsWebContents = null # Remember the window. id = BrowserWindow.windows.add this