From e2bd1abce635f39ef8dc20b73afdaca01130761a Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Sun, 6 Sep 2015 10:30:59 +0800 Subject: [PATCH] Make sure BrowserContext is destroyed on exit --- atom/browser/api/atom_api_session.cc | 20 ++++++++++++++------ atom/browser/api/atom_api_session.h | 7 ++++++- atom/browser/api/atom_api_web_contents.h | 4 +++- atom/browser/api/atom_api_window.cc | 5 ++++- atom/browser/api/atom_api_window.h | 4 +++- atom/browser/api/trackable_object.cc | 9 ++++++--- atom/browser/api/trackable_object.h | 12 ++++++++++-- vendor/brightray | 2 +- 8 files changed, 47 insertions(+), 16 deletions(-) diff --git a/atom/browser/api/atom_api_session.cc b/atom/browser/api/atom_api_session.cc index db9d89c3dd60..7d31626c1413 100644 --- a/atom/browser/api/atom_api_session.cc +++ b/atom/browser/api/atom_api_session.cc @@ -233,16 +233,16 @@ void SetProxyInIO(net::URLRequestContextGetter* getter, Session::Session(AtomBrowserContext* browser_context) : browser_context_(browser_context) { AttachAsUserData(browser_context); + // Observe DownloadManger to get download notifications. - auto download_manager = - content::BrowserContext::GetDownloadManager(browser_context); - download_manager->AddObserver(this); + content::BrowserContext::GetDownloadManager(browser_context)-> + AddObserver(this); } Session::~Session() { - auto download_manager = - content::BrowserContext::GetDownloadManager(browser_context()); - download_manager->RemoveObserver(this); + content::BrowserContext::GetDownloadManager(browser_context())-> + RemoveObserver(this); + Destroy(); } void Session::OnDownloadCreated(content::DownloadManager* manager, @@ -257,6 +257,14 @@ void Session::OnDownloadCreated(content::DownloadManager* manager, } } +bool Session::IsDestroyed() const { + return !browser_context_; +} + +void Session::Destroy() { + browser_context_ = nullptr; +} + void Session::ResolveProxy(const GURL& url, ResolveProxyCallback callback) { new ResolveProxyHelper(browser_context(), url, callback); } diff --git a/atom/browser/api/atom_api_session.h b/atom/browser/api/atom_api_session.h index d9799861401b..14406e57af5b 100644 --- a/atom/browser/api/atom_api_session.h +++ b/atom/browser/api/atom_api_session.h @@ -51,11 +51,15 @@ class Session: public mate::TrackableObject, void OnDownloadCreated(content::DownloadManager* manager, content::DownloadItem* item) override; - // mate::Wrappable implementations: + // mate::Wrappable: mate::ObjectTemplateBuilder GetObjectTemplateBuilder( v8::Isolate* isolate) override; + bool IsDestroyed() const override; private: + // mate::TrackableObject: + void Destroy() override; + void ResolveProxy(const GURL& url, ResolveProxyCallback callback); void ClearCache(const net::CompletionCallback& callback); void ClearStorageData(mate::Arguments* args); @@ -63,6 +67,7 @@ class Session: public mate::TrackableObject, void SetDownloadPath(const base::FilePath& path); v8::Local Cookies(v8::Isolate* isolate); + // Cached object for cookies API. v8::Global cookies_; scoped_refptr browser_context_; diff --git a/atom/browser/api/atom_api_web_contents.h b/atom/browser/api/atom_api_web_contents.h index 5ea62dabff62..e2e405b73853 100644 --- a/atom/browser/api/atom_api_web_contents.h +++ b/atom/browser/api/atom_api_web_contents.h @@ -48,7 +48,9 @@ class WebContents : public mate::TrackableObject, static mate::Handle Create( v8::Isolate* isolate, const mate::Dictionary& options); - void Destroy(); + // mate::TrackableObject: + void Destroy() override; + bool IsAlive() const; int GetID() const; bool Equal(const WebContents* web_contents) const; diff --git a/atom/browser/api/atom_api_window.cc b/atom/browser/api/atom_api_window.cc index 586cfdb2a78d..e171fa7698f0 100644 --- a/atom/browser/api/atom_api_window.cc +++ b/atom/browser/api/atom_api_window.cc @@ -221,7 +221,10 @@ bool Window::IsDestroyed() const { } void Window::Destroy() { - window_->CloseContents(nullptr); + if (window_) { + window_->CloseContents(nullptr); + window_.reset(); + } } void Window::Close() { diff --git a/atom/browser/api/atom_api_window.h b/atom/browser/api/atom_api_window.h index de2f093ea3e1..4384e03a249f 100644 --- a/atom/browser/api/atom_api_window.h +++ b/atom/browser/api/atom_api_window.h @@ -78,8 +78,10 @@ class Window : public mate::TrackableObject, bool IsDestroyed() const override; private: + // mate::TrackableObject: + void Destroy() override; + // APIs for NativeWindow. - void Destroy(); void Close(); bool IsClosed(); void Focus(); diff --git a/atom/browser/api/trackable_object.cc b/atom/browser/api/trackable_object.cc index ae9fa44230a0..50bfa59e6a7a 100644 --- a/atom/browser/api/trackable_object.cc +++ b/atom/browser/api/trackable_object.cc @@ -29,7 +29,9 @@ class IDUserData : public base::SupportsUserData::Data { } // namespace TrackableObjectBase::TrackableObjectBase() - : weak_map_id_(0), wrapped_(nullptr) { + : weak_map_id_(0), wrapped_(nullptr), weak_factory_(this) { + RegisterDestructionCallback( + base::Bind(&TrackableObjectBase::Destroy, weak_factory_.GetWeakPtr())); } TrackableObjectBase::~TrackableObjectBase() { @@ -61,8 +63,9 @@ int32_t TrackableObjectBase::GetIDFromWrappedClass(base::SupportsUserData* w) { } // static -void TrackableObjectBase::RegisterDestructionCallback(void (*c)()) { - atom::AtomBrowserMainParts::Get()->RegisterDestructionCallback(base::Bind(c)); +void TrackableObjectBase::RegisterDestructionCallback( + const base::Closure& closure) { + atom::AtomBrowserMainParts::Get()->RegisterDestructionCallback(closure); } } // namespace mate diff --git a/atom/browser/api/trackable_object.h b/atom/browser/api/trackable_object.h index 656904cc45d0..8ff7d1c04041 100644 --- a/atom/browser/api/trackable_object.h +++ b/atom/browser/api/trackable_object.h @@ -9,7 +9,9 @@ #include "atom/browser/api/event_emitter.h" #include "atom/common/id_weak_map.h" +#include "base/bind.h" #include "base/memory/scoped_ptr.h" +#include "base/memory/weak_ptr.h" namespace base { class SupportsUserData; @@ -28,6 +30,9 @@ class TrackableObjectBase : public mate::EventEmitter { // Wrap TrackableObject into a class that SupportsUserData. void AttachAsUserData(base::SupportsUserData* wrapped); + // Subclasses should implement this to destroy their native types. + virtual void Destroy() = 0; + protected: ~TrackableObjectBase() override; @@ -39,12 +44,14 @@ class TrackableObjectBase : public mate::EventEmitter { // Register a callback that should be destroyed before JavaScript environment // gets destroyed. - static void RegisterDestructionCallback(void (*callback)()); + static void RegisterDestructionCallback(const base::Closure& closure); int32_t weak_map_id_; base::SupportsUserData* wrapped_; private: + base::WeakPtrFactory weak_factory_; + DISALLOW_COPY_AND_ASSIGN(TrackableObjectBase); }; @@ -85,7 +92,8 @@ class TrackableObject : public TrackableObjectBase { } TrackableObject() { - RegisterDestructionCallback(&TrackableObject::ReleaseAllWeakReferences); + RegisterDestructionCallback( + base::Bind(&TrackableObject::ReleaseAllWeakReferences)); } // Removes this instance from the weak map. diff --git a/vendor/brightray b/vendor/brightray index 251521cb8c51..8d64120b51b4 160000 --- a/vendor/brightray +++ b/vendor/brightray @@ -1 +1 @@ -Subproject commit 251521cb8c51670dbd08ab9fd2dd45e249cbb596 +Subproject commit 8d64120b51b48be46eaa419957b965c0ccfc6c8f