Merge pull request #3676 from atom/callback-race-condition

Make sure V8 handles are deleted on UI thread
This commit is contained in:
Cheng Zhao 2015-12-03 18:06:34 +08:00
commit f3645c6661
31 changed files with 292 additions and 318 deletions

View file

@ -322,14 +322,6 @@ void Cookies::OnSetCookies(const CookiesCallback& callback,
callback));
}
mate::ObjectTemplateBuilder Cookies::GetObjectTemplateBuilder(
v8::Isolate* isolate) {
return mate::ObjectTemplateBuilder(isolate)
.SetMethod("get", &Cookies::Get)
.SetMethod("remove", &Cookies::Remove)
.SetMethod("set", &Cookies::Set);
}
net::CookieStore* Cookies::GetCookieStore() {
return request_context_getter_->GetURLRequestContext()->cookie_store();
}
@ -341,6 +333,15 @@ mate::Handle<Cookies> Cookies::Create(
return mate::CreateHandle(isolate, new Cookies(browser_context));
}
// static
void Cookies::BuildPrototype(v8::Isolate* isolate,
v8::Local<v8::ObjectTemplate> prototype) {
mate::ObjectTemplateBuilder(isolate, prototype)
.SetMethod("get", &Cookies::Get)
.SetMethod("remove", &Cookies::Remove)
.SetMethod("set", &Cookies::Set);
}
} // namespace api
} // namespace atom

View file

@ -7,8 +7,8 @@
#include <string>
#include "atom/browser/api/trackable_object.h"
#include "base/callback.h"
#include "native_mate/wrappable.h"
#include "native_mate/handle.h"
#include "net/cookies/canonical_cookie.h"
@ -33,7 +33,7 @@ namespace atom {
namespace api {
class Cookies : public mate::Wrappable {
class Cookies : public mate::TrackableObject<Cookies> {
public:
// node.js style callback function(error, result)
typedef base::Callback<void(v8::Local<v8::Value>, v8::Local<v8::Value>)>
@ -42,6 +42,10 @@ class Cookies : public mate::Wrappable {
static mate::Handle<Cookies> Create(v8::Isolate* isolate,
content::BrowserContext* browser_context);
// mate::TrackableObject:
static void BuildPrototype(v8::Isolate* isolate,
v8::Local<v8::ObjectTemplate> prototype);
protected:
explicit Cookies(content::BrowserContext* browser_context);
~Cookies();
@ -70,10 +74,6 @@ class Cookies : public mate::Wrappable {
void OnSetCookies(const CookiesCallback& callback,
bool set_success);
// mate::Wrappable:
mate::ObjectTemplateBuilder GetObjectTemplateBuilder(
v8::Isolate* isolate) override;
private:
// Must be called on IO thread.
net::CookieStore* GetCookieStore();

View file

@ -70,29 +70,20 @@ DownloadItem::DownloadItem(content::DownloadItem* download_item) :
}
DownloadItem::~DownloadItem() {
Destroy();
}
void DownloadItem::Destroy() {
if (download_item_) {
download_item_->RemoveObserver(this);
auto iter = g_download_item_objects.find(download_item_->GetId());
if (iter != g_download_item_objects.end())
g_download_item_objects.erase(iter);
download_item_ = nullptr;
}
}
bool DownloadItem::IsDestroyed() const {
return download_item_ == nullptr;
if (download_item_)
OnDownloadDestroyed(download_item_);
}
void DownloadItem::OnDownloadUpdated(content::DownloadItem* item) {
download_item_->IsDone() ? Emit("done", item->GetState()) : Emit("updated");
}
void DownloadItem::OnDownloadDestroyed(content::DownloadItem* download) {
Destroy();
void DownloadItem::OnDownloadDestroyed(content::DownloadItem* download_item) {
download_item_->RemoveObserver(this);
auto iter = g_download_item_objects.find(download_item_->GetId());
if (iter != g_download_item_objects.end())
g_download_item_objects.erase(iter);
download_item_ = nullptr;
}
int64 DownloadItem::GetReceivedBytes() {
@ -144,9 +135,11 @@ void DownloadItem::Cancel() {
download_item_->Cancel(true);
}
mate::ObjectTemplateBuilder DownloadItem::GetObjectTemplateBuilder(
v8::Isolate* isolate) {
return mate::ObjectTemplateBuilder(isolate)
// static
void DownloadItem::BuildPrototype(v8::Isolate* isolate,
v8::Local<v8::ObjectTemplate> prototype) {
mate::ObjectTemplateBuilder(isolate, prototype)
.MakeDestroyable()
.SetMethod("pause", &DownloadItem::Pause)
.SetMethod("resume", &DownloadItem::Resume)
.SetMethod("cancel", &DownloadItem::Cancel)

View file

@ -17,7 +17,7 @@ namespace atom {
namespace api {
class DownloadItem : public mate::EventEmitter,
class DownloadItem : public mate::TrackableObject<DownloadItem>,
public content::DownloadItem::Observer {
public:
class SavePathData : public base::SupportsUserData::Data {
@ -32,6 +32,10 @@ class DownloadItem : public mate::EventEmitter,
content::DownloadItem* item);
static void* UserDataKey();
// mate::TrackableObject:
static void BuildPrototype(v8::Isolate* isolate,
v8::Local<v8::ObjectTemplate> prototype);
protected:
explicit DownloadItem(content::DownloadItem* download_item);
~DownloadItem();
@ -53,13 +57,6 @@ class DownloadItem : public mate::EventEmitter,
void SetSavePath(const base::FilePath& path);
private:
// mate::Wrappable:
mate::ObjectTemplateBuilder GetObjectTemplateBuilder(
v8::Isolate* isolate) override;
bool IsDestroyed() const override;
void Destroy();
content::DownloadItem* download_item_;
DISALLOW_COPY_AND_ASSIGN(DownloadItem);

View file

@ -23,9 +23,6 @@ GlobalShortcut::GlobalShortcut() {
}
GlobalShortcut::~GlobalShortcut() {
}
void GlobalShortcut::Destroy() {
UnregisterAll();
}

View file

@ -27,9 +27,6 @@ class GlobalShortcut : public extensions::GlobalShortcutListener::Observer,
GlobalShortcut();
~GlobalShortcut() override;
// mate::TrackableObject:
void Destroy() override;
// mate::Wrappable implementations:
mate::ObjectTemplateBuilder GetObjectTemplateBuilder(
v8::Isolate* isolate) override;

View file

@ -27,14 +27,6 @@ Menu::Menu()
Menu::~Menu() {
}
void Menu::Destroy() {
model_.reset();
}
bool Menu::IsDestroyed() const {
return !model_;
}
void Menu::AfterInit(v8::Isolate* isolate) {
mate::Dictionary wrappable(isolate, GetWrapper(isolate));
mate::Dictionary delegate;
@ -159,6 +151,7 @@ bool Menu::IsVisibleAt(int index) const {
void Menu::BuildPrototype(v8::Isolate* isolate,
v8::Local<v8::ObjectTemplate> prototype) {
mate::ObjectTemplateBuilder(isolate, prototype)
.MakeDestroyable()
.SetMethod("insertItem", &Menu::InsertItemAt)
.SetMethod("insertCheckItem", &Menu::InsertCheckItemAt)
.SetMethod("insertRadioItem", &Menu::InsertRadioItemAt)

View file

@ -39,11 +39,7 @@ class Menu : public mate::TrackableObject<Menu>,
Menu();
~Menu() override;
// mate::TrackableObject:
void Destroy() override;
// mate::Wrappable:
bool IsDestroyed() const override;
void AfterInit(v8::Isolate* isolate) override;
// ui::SimpleMenuModel::Delegate:

View file

@ -19,7 +19,6 @@ class MenuMac : public Menu {
protected:
MenuMac();
void Destroy() override;
void Popup(Window* window) override;
void PopupAt(Window* window, int x, int y) override;

View file

@ -18,11 +18,6 @@ namespace api {
MenuMac::MenuMac() {
}
void MenuMac::Destroy() {
menu_controller_.reset();
Menu::Destroy();
}
void MenuMac::Popup(Window* window) {
NativeWindow* native_window = window->window();
if (!native_window)

View file

@ -19,9 +19,6 @@ PowerMonitor::PowerMonitor() {
}
PowerMonitor::~PowerMonitor() {
}
void PowerMonitor::Destroy() {
base::PowerMonitor::Get()->RemoveObserver(this);
}

View file

@ -23,9 +23,6 @@ class PowerMonitor : public mate::TrackableObject<PowerMonitor>,
PowerMonitor();
~PowerMonitor() override;
// mate::TrackableObject:
void Destroy() override;
// base::PowerObserver implementations:
void OnPowerStateChange(bool on_battery_power) override;
void OnSuspend() override;

View file

@ -45,11 +45,6 @@ PowerSaveBlocker::PowerSaveBlocker()
PowerSaveBlocker::~PowerSaveBlocker() {
}
void PowerSaveBlocker::Destroy() {
power_save_blocker_types_.clear();
power_save_blocker_.reset();
}
void PowerSaveBlocker::UpdatePowerSaveBlocker() {
if (power_save_blocker_types_.empty()) {
power_save_blocker_.reset();

View file

@ -28,9 +28,6 @@ class PowerSaveBlocker : public mate::TrackableObject<PowerSaveBlocker> {
PowerSaveBlocker();
~PowerSaveBlocker() override;
// mate::TrackableObject:
void Destroy() override;
// mate::Wrappable implementations:
mate::ObjectTemplateBuilder GetObjectTemplateBuilder(
v8::Isolate* isolate) override;

View file

@ -253,7 +253,6 @@ Session::Session(AtomBrowserContext* browser_context)
Session::~Session() {
content::BrowserContext::GetDownloadManager(browser_context())->
RemoveObserver(this);
Destroy();
}
void Session::OnDownloadCreated(content::DownloadManager* manager,
@ -271,14 +270,6 @@ 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);
}
@ -376,20 +367,6 @@ v8::Local<v8::Value> Session::Cookies(v8::Isolate* isolate) {
return v8::Local<v8::Value>::New(isolate, cookies_);
}
mate::ObjectTemplateBuilder Session::GetObjectTemplateBuilder(
v8::Isolate* isolate) {
return mate::ObjectTemplateBuilder(isolate)
.SetMethod("resolveProxy", &Session::ResolveProxy)
.SetMethod("clearCache", &Session::ClearCache)
.SetMethod("clearStorageData", &Session::ClearStorageData)
.SetMethod("setProxy", &Session::SetProxy)
.SetMethod("setDownloadPath", &Session::SetDownloadPath)
.SetMethod("enableNetworkEmulation", &Session::EnableNetworkEmulation)
.SetMethod("disableNetworkEmulation", &Session::DisableNetworkEmulation)
.SetMethod("setCertificateVerifyProc", &Session::SetCertVerifyProc)
.SetProperty("cookies", &Session::Cookies);
}
// static
mate::Handle<Session> Session::CreateFrom(
v8::Isolate* isolate, AtomBrowserContext* browser_context) {
@ -410,6 +387,22 @@ mate::Handle<Session> Session::FromPartition(
static_cast<AtomBrowserContext*>(browser_context.get()));
}
// static
void Session::BuildPrototype(v8::Isolate* isolate,
v8::Local<v8::ObjectTemplate> prototype) {
mate::ObjectTemplateBuilder(isolate, prototype)
.MakeDestroyable()
.SetMethod("resolveProxy", &Session::ResolveProxy)
.SetMethod("clearCache", &Session::ClearCache)
.SetMethod("clearStorageData", &Session::ClearStorageData)
.SetMethod("setProxy", &Session::SetProxy)
.SetMethod("setDownloadPath", &Session::SetDownloadPath)
.SetMethod("enableNetworkEmulation", &Session::EnableNetworkEmulation)
.SetMethod("disableNetworkEmulation", &Session::DisableNetworkEmulation)
.SetMethod("setCertificateVerifyProc", &Session::SetCertVerifyProc)
.SetProperty("cookies", &Session::Cookies);
}
void ClearWrapSession() {
g_wrap_session.Reset();
}

View file

@ -48,6 +48,10 @@ class Session: public mate::TrackableObject<Session>,
AtomBrowserContext* browser_context() const { return browser_context_.get(); }
// mate::TrackableObject:
static void BuildPrototype(v8::Isolate* isolate,
v8::Local<v8::ObjectTemplate> prototype);
protected:
explicit Session(AtomBrowserContext* browser_context);
~Session();
@ -56,15 +60,7 @@ class Session: public mate::TrackableObject<Session>,
void OnDownloadCreated(content::DownloadManager* manager,
content::DownloadItem* item) override;
// 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);

View file

@ -94,14 +94,6 @@ void Tray::OnDragEnded() {
Emit("drag-end");
}
bool Tray::IsDestroyed() const {
return !tray_icon_;
}
void Tray::Destroy() {
tray_icon_.reset();
}
void Tray::SetImage(mate::Arguments* args, const gfx::Image& image) {
tray_icon_->SetImage(image);
}
@ -162,8 +154,7 @@ v8::Local<v8::Object> Tray::ModifiersToObject(v8::Isolate* isolate,
void Tray::BuildPrototype(v8::Isolate* isolate,
v8::Local<v8::ObjectTemplate> prototype) {
mate::ObjectTemplateBuilder(isolate, prototype)
.SetMethod("destroy", &Tray::Destroy, true)
.SetMethod("isDestroyed", &Tray::IsDestroyed, true)
.MakeDestroyable()
.SetMethod("setImage", &Tray::SetImage)
.SetMethod("setPressedImage", &Tray::SetPressedImage)
.SetMethod("setToolTip", &Tray::SetToolTip)

View file

@ -54,12 +54,6 @@ class Tray : public mate::TrackableObject<Tray>,
void OnDragExited() override;
void OnDragEnded() override;
// mate::Wrappable:
bool IsDestroyed() const override;
// mate::TrackableObject:
void Destroy() override;
void SetImage(mate::Arguments* args, const gfx::Image& image);
void SetPressedImage(mate::Arguments* args, const gfx::Image& image);
void SetToolTip(mate::Arguments* args, const std::string& tool_tip);

View file

@ -194,8 +194,6 @@ namespace api {
namespace {
v8::Persistent<v8::ObjectTemplate> template_;
// The wrapWebContents function which is implemented in JavaScript
using WrapWebContentsCallback = base::Callback<void(v8::Local<v8::Value>)>;
WrapWebContentsCallback g_wrap_web_contents;
@ -290,7 +288,15 @@ WebContents::WebContents(v8::Isolate* isolate,
}
WebContents::~WebContents() {
Destroy();
if (type_ == WEB_VIEW && managed_web_contents()) {
// When force destroying the "destroyed" event is not emitted.
WebContentsDestroyed();
guest_delegate_->Destroy();
Observe(nullptr);
DestroyWebContents();
}
}
bool WebContents::AddMessageToConsole(content::WebContents* source,
@ -591,19 +597,6 @@ void WebContents::NavigationEntryCommitted(
details.is_in_page, details.did_replace_entry);
}
void WebContents::Destroy() {
session_.Reset();
if (type_ == WEB_VIEW && managed_web_contents()) {
// When force destroying the "destroyed" event is not emitted.
WebContentsDestroyed();
guest_delegate_->Destroy();
Observe(nullptr);
DestroyWebContents();
}
}
int WebContents::GetID() const {
return web_contents()->GetRenderProcessHost()->GetID();
}
@ -987,12 +980,11 @@ v8::Local<v8::Value> WebContents::DevToolsWebContents(v8::Isolate* isolate) {
return v8::Local<v8::Value>::New(isolate, devtools_web_contents_);
}
mate::ObjectTemplateBuilder WebContents::GetObjectTemplateBuilder(
v8::Isolate* isolate) {
if (template_.IsEmpty())
template_.Reset(isolate, mate::ObjectTemplateBuilder(isolate)
.SetMethod("destroy", &WebContents::Destroy, true)
.SetMethod("isDestroyed", &WebContents::IsDestroyed, true)
// static
void WebContents::BuildPrototype(v8::Isolate* isolate,
v8::Local<v8::ObjectTemplate> prototype) {
mate::ObjectTemplateBuilder(isolate, prototype)
.MakeDestroyable()
.SetMethod("getId", &WebContents::GetID)
.SetMethod("equal", &WebContents::Equal)
.SetMethod("_loadURL", &WebContents::LoadURL)
@ -1034,7 +1026,7 @@ mate::ObjectTemplateBuilder WebContents::GetObjectTemplateBuilder(
.SetMethod("replaceMisspelling", &WebContents::ReplaceMisspelling)
.SetMethod("focus", &WebContents::Focus)
.SetMethod("tabTraverse", &WebContents::TabTraverse)
.SetMethod("_send", &WebContents::SendIPCMessage, true)
.SetMethod("_send", &WebContents::SendIPCMessage)
.SetMethod("sendInputEvent", &WebContents::SendInputEvent)
.SetMethod("beginFrameSubscription",
&WebContents::BeginFrameSubscription)
@ -1052,17 +1044,8 @@ mate::ObjectTemplateBuilder WebContents::GetObjectTemplateBuilder(
.SetMethod("_printToPDF", &WebContents::PrintToPDF)
.SetMethod("addWorkSpace", &WebContents::AddWorkSpace)
.SetMethod("removeWorkSpace", &WebContents::RemoveWorkSpace)
.SetProperty("session", &WebContents::Session, true)
.SetProperty("devToolsWebContents",
&WebContents::DevToolsWebContents, true)
.Build());
return mate::ObjectTemplateBuilder(
isolate, v8::Local<v8::ObjectTemplate>::New(isolate, template_));
}
bool WebContents::IsDestroyed() const {
return !web_contents();
.SetProperty("session", &WebContents::Session)
.SetProperty("devToolsWebContents", &WebContents::DevToolsWebContents);
}
AtomBrowserContext* WebContents::GetBrowserContext() const {

View file

@ -54,9 +54,6 @@ class WebContents : public mate::TrackableObject<WebContents>,
static mate::Handle<WebContents> Create(
v8::Isolate* isolate, const mate::Dictionary& options);
// mate::TrackableObject:
void Destroy() override;
int GetID() const;
bool Equal(const WebContents* web_contents) const;
void LoadURL(const GURL& url, const mate::Dictionary& options);
@ -144,16 +141,15 @@ class WebContents : public mate::TrackableObject<WebContents>,
v8::Local<v8::Value> Session(v8::Isolate* isolate);
v8::Local<v8::Value> DevToolsWebContents(v8::Isolate* isolate);
// mate::TrackableObject:
static void BuildPrototype(v8::Isolate* isolate,
v8::Local<v8::ObjectTemplate> prototype);
protected:
explicit WebContents(content::WebContents* web_contents);
WebContents(v8::Isolate* isolate, const mate::Dictionary& options);
~WebContents();
// mate::Wrappable:
mate::ObjectTemplateBuilder GetObjectTemplateBuilder(
v8::Isolate* isolate) override;
bool IsDestroyed() const override;
// content::WebContentsDelegate:
bool AddMessageToConsole(content::WebContents* source,
int32 level,

View file

@ -157,8 +157,8 @@ Window::Window(v8::Isolate* isolate, const mate::Dictionary& options) {
}
Window::~Window() {
if (window_)
Destroy();
if (!window_->IsClosed())
window_->CloseContents(nullptr);
}
void Window::WillCloseWindow(bool* prevent_default) {
@ -166,19 +166,19 @@ void Window::WillCloseWindow(bool* prevent_default) {
}
void Window::OnWindowClosed() {
if (api_web_contents_) {
api_web_contents_->DestroyWebContents();
api_web_contents_ = nullptr;
web_contents_.Reset();
}
RemoveFromWeakMap();
window_->RemoveObserver(this);
// 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();
Emit("closed");
// Clean up the resources after window has been closed.
base::MessageLoop::current()->DeleteSoon(FROM_HERE, window_.release());
// Destroy the native class when window is closed.
base::MessageLoop::current()->PostTask(FROM_HERE, GetDestroyClosure());
}
void Window::OnWindowBlur() {
@ -276,15 +276,6 @@ mate::Wrappable* Window::New(v8::Isolate* isolate, mate::Arguments* args) {
return new Window(isolate, options);
}
bool Window::IsDestroyed() const {
return !window_ || window_->IsClosed();
}
void Window::Destroy() {
if (window_)
window_->CloseContents(nullptr);
}
void Window::Close() {
window_->Close();
}
@ -622,8 +613,7 @@ v8::Local<v8::Value> Window::WebContents(v8::Isolate* isolate) {
void Window::BuildPrototype(v8::Isolate* isolate,
v8::Local<v8::ObjectTemplate> prototype) {
mate::ObjectTemplateBuilder(isolate, prototype)
.SetMethod("destroy", &Window::Destroy, true)
.SetMethod("isDestroyed", &Window::IsDestroyed, true)
.MakeDestroyable()
.SetMethod("close", &Window::Close)
.SetMethod("focus", &Window::Focus)
.SetMethod("isFocused", &Window::IsFocused)
@ -695,8 +685,8 @@ void Window::BuildPrototype(v8::Isolate* isolate,
.SetMethod("showDefinitionForSelection",
&Window::ShowDefinitionForSelection)
#endif
.SetProperty("id", &Window::ID, true)
.SetProperty("webContents", &Window::WebContents, true);
.SetProperty("id", &Window::ID)
.SetProperty("webContents", &Window::WebContents);
}
// static

View file

@ -77,13 +77,7 @@ class Window : public mate::TrackableObject<Window>,
void OnWindowMessage(UINT message, WPARAM w_param, LPARAM l_param) override;
#endif
// mate::Wrappable:
bool IsDestroyed() const override;
private:
// mate::TrackableObject:
void Destroy() override;
// APIs for NativeWindow.
void Close();
void Focus();

View file

@ -30,11 +30,11 @@ class IDUserData : public base::SupportsUserData::Data {
TrackableObjectBase::TrackableObjectBase()
: weak_map_id_(0), wrapped_(nullptr), weak_factory_(this) {
RegisterDestructionCallback(
base::Bind(&TrackableObjectBase::Destroy, weak_factory_.GetWeakPtr()));
cleanup_ = RegisterDestructionCallback(GetDestroyClosure());
}
TrackableObjectBase::~TrackableObjectBase() {
cleanup_.Run();
}
void TrackableObjectBase::AfterInit(v8::Isolate* isolate) {
@ -42,6 +42,18 @@ void TrackableObjectBase::AfterInit(v8::Isolate* isolate) {
AttachAsUserData(wrapped_);
}
void TrackableObjectBase::MarkDestroyed() {
GetWrapper(isolate())->SetAlignedPointerInInternalField(0, nullptr);
}
base::Closure TrackableObjectBase::GetDestroyClosure() {
return base::Bind(&TrackableObjectBase::Destroy, weak_factory_.GetWeakPtr());
}
void TrackableObjectBase::Destroy() {
delete this;
}
void TrackableObjectBase::AttachAsUserData(base::SupportsUserData* wrapped) {
if (weak_map_id_ != 0) {
wrapped->SetUserData(kTrackedObjectKey, new IDUserData(weak_map_id_));
@ -63,9 +75,9 @@ int32_t TrackableObjectBase::GetIDFromWrappedClass(base::SupportsUserData* w) {
}
// static
void TrackableObjectBase::RegisterDestructionCallback(
const base::Closure& closure) {
atom::AtomBrowserMainParts::Get()->RegisterDestructionCallback(closure);
base::Closure TrackableObjectBase::RegisterDestructionCallback(
const base::Closure& c) {
return atom::AtomBrowserMainParts::Get()->RegisterDestructionCallback(c);
}
} // namespace mate

View file

@ -12,6 +12,7 @@
#include "base/bind.h"
#include "base/memory/scoped_ptr.h"
#include "base/memory/weak_ptr.h"
#include "native_mate/object_template_builder.h"
namespace base {
class SupportsUserData;
@ -30,26 +31,32 @@ 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;
// mate::Wrappable:
void AfterInit(v8::Isolate* isolate) override;
// Mark the JS object as destroyed.
void MarkDestroyed();
// Returns a closure that can destroy the native class.
base::Closure GetDestroyClosure();
// Get the weak_map_id from SupportsUserData.
static int32_t GetIDFromWrappedClass(base::SupportsUserData* wrapped);
// Register a callback that should be destroyed before JavaScript environment
// gets destroyed.
static void RegisterDestructionCallback(const base::Closure& closure);
static base::Closure RegisterDestructionCallback(const base::Closure& c);
int32_t weak_map_id_;
base::SupportsUserData* wrapped_;
private:
void Destroy();
base::Closure cleanup_;
base::WeakPtrFactory<TrackableObjectBase> weak_factory_;
DISALLOW_COPY_AND_ASSIGN(TrackableObjectBase);
@ -91,11 +98,6 @@ class TrackableObject : public TrackableObjectBase {
return std::vector<v8::Local<v8::Object>>();
}
TrackableObject() {
RegisterDestructionCallback(
base::Bind(&TrackableObject<T>::ReleaseAllWeakReferences));
}
// Removes this instance from the weak map.
void RemoveFromWeakMap() {
if (weak_map_ && weak_map_->Has(weak_map_id()))
@ -103,28 +105,49 @@ class TrackableObject : public TrackableObjectBase {
}
protected:
TrackableObject() {}
~TrackableObject() override {
RemoveFromWeakMap();
}
void AfterInit(v8::Isolate* isolate) override {
if (!weak_map_)
if (!weak_map_) {
weak_map_.reset(new atom::IDWeakMap);
RegisterDestructionCallback(
base::Bind(&TrackableObject<T>::ReleaseAllWeakReferences));
}
weak_map_id_ = weak_map_->Add(isolate, GetWrapper(isolate));
TrackableObjectBase::AfterInit(isolate);
}
private:
// mate::Wrappable:
mate::ObjectTemplateBuilder GetObjectTemplateBuilder(
v8::Isolate* isolate) override {
if (template_.IsEmpty()) {
auto templ = v8::ObjectTemplate::New(isolate);
T::BuildPrototype(isolate, templ);
template_.Reset(isolate, templ);
}
return ObjectTemplateBuilder(
isolate, v8::Local<v8::ObjectTemplate>::New(isolate, template_));
}
// Releases all weak references in weak map, called when app is terminating.
static void ReleaseAllWeakReferences() {
weak_map_.reset();
}
static v8::Persistent<v8::ObjectTemplate> template_;
static scoped_ptr<atom::IDWeakMap> weak_map_;
DISALLOW_COPY_AND_ASSIGN(TrackableObject);
};
template<typename T>
v8::Persistent<v8::ObjectTemplate> TrackableObject<T>::template_;
template<typename T>
scoped_ptr<atom::IDWeakMap> TrackableObject<T>::weak_map_;

View file

@ -25,6 +25,11 @@
namespace atom {
template<typename T>
void Erase(T* container, typename T::iterator iter) {
container->erase(iter);
}
// static
AtomBrowserMainParts* AtomBrowserMainParts::self_ = NULL;
@ -56,9 +61,10 @@ bool AtomBrowserMainParts::SetExitCode(int code) {
return true;
}
void AtomBrowserMainParts::RegisterDestructionCallback(
base::Closure AtomBrowserMainParts::RegisterDestructionCallback(
const base::Closure& callback) {
destruction_callbacks_.push_back(callback);
auto iter = destructors_.insert(destructors_.end(), callback);
return base::Bind(&Erase<std::list<base::Closure>>, &destructors_, iter);
}
void AtomBrowserMainParts::PreEarlyInitialization() {
@ -150,8 +156,13 @@ void AtomBrowserMainParts::PostMainMessageLoopRun() {
// Make sure destruction callbacks are called before message loop is
// destroyed, otherwise some objects that need to be deleted on IO thread
// won't be freed.
for (const auto& callback : destruction_callbacks_)
// We don't use ranged for loop because iterators are getting invalided when
// the callback runs.
for (auto iter = destructors_.begin(); iter != destructors_.end();) {
base::Closure& callback = *iter;
++iter;
callback.Run();
}
// Destroy JavaScript environment immediately after running destruction
// callbacks.

View file

@ -36,7 +36,8 @@ class AtomBrowserMainParts : public brightray::BrowserMainParts {
// Register a callback that should be destroyed before JavaScript environment
// gets destroyed.
void RegisterDestructionCallback(const base::Closure& callback);
// Returns a closure that can be used to remove |callback| from the list.
base::Closure RegisterDestructionCallback(const base::Closure& callback);
Browser* browser() { return browser_.get(); }
@ -82,7 +83,7 @@ class AtomBrowserMainParts : public brightray::BrowserMainParts {
base::Timer gc_timer_;
// List of callbacks should be executed before destroying JS env.
std::list<base::Closure> destruction_callbacks_;
std::list<base::Closure> destructors_;
static AtomBrowserMainParts* self_;

View file

@ -39,11 +39,12 @@ createGuest = (embedder, url, frameName, options) ->
# When |embedder| is destroyed we should also destroy attached guest, and if
# guest is closed by user then we should prevent |embedder| from double
# closing guest.
guestId = guest.id
closedByEmbedder = ->
guest.removeListener 'closed', closedByUser
guest.destroy()
closedByUser = ->
embedder.send 'ATOM_SHELL_GUEST_WINDOW_MANAGER_WINDOW_CLOSED', guest.id
embedder.send 'ATOM_SHELL_GUEST_WINDOW_MANAGER_WINDOW_CLOSED', guestId
embedder.removeListener 'render-view-deleted', closedByEmbedder
embedder.once 'render-view-deleted', closedByEmbedder
guest.once 'closed', closedByUser

View file

@ -18,6 +18,8 @@
namespace {
v8::Persistent<v8::ObjectTemplate> template_;
class Archive : public mate::Wrappable {
public:
static v8::Local<v8::Value> Create(v8::Isolate* isolate,
@ -101,7 +103,8 @@ class Archive : public mate::Wrappable {
// mate::Wrappable:
mate::ObjectTemplateBuilder GetObjectTemplateBuilder(v8::Isolate* isolate) {
return mate::ObjectTemplateBuilder(isolate)
if (template_.IsEmpty())
template_.Reset(isolate, mate::ObjectTemplateBuilder(isolate)
.SetValue("path", archive_->path())
.SetMethod("getFileInfo", &Archive::GetFileInfo)
.SetMethod("stat", &Archive::Stat)
@ -109,7 +112,11 @@ class Archive : public mate::Wrappable {
.SetMethod("realpath", &Archive::Realpath)
.SetMethod("copyFileOut", &Archive::CopyFileOut)
.SetMethod("getFd", &Archive::GetFD)
.SetMethod("destroy", &Archive::Destroy);
.SetMethod("destroy", &Archive::Destroy)
.Build());
return mate::ObjectTemplateBuilder(
isolate, v8::Local<v8::ObjectTemplate>::New(isolate, template_));
}
private:

View file

@ -4,7 +4,9 @@
#include "atom/common/native_mate_converters/callback.h"
#include "atom/browser/atom_browser_main_parts.h"
#include "content/public/browser/browser_thread.h"
using content::BrowserThread;
namespace mate {
@ -56,31 +58,59 @@ v8::Local<v8::Value> BindFunctionWith(v8::Isolate* isolate,
} // namespace
// Destroy the class on UI thread when possible.
struct DeleteOnUIThread {
template<typename T>
static void Destruct(const T* x) {
if (Locker::IsBrowserProcess() &&
!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
BrowserThread::DeleteSoon(BrowserThread::UI, FROM_HERE, x);
} else {
delete x;
}
}
};
// Like v8::Global, but ref-counted.
template<typename T>
class RefCountedGlobal : public base::RefCountedThreadSafe<RefCountedGlobal<T>,
DeleteOnUIThread> {
public:
RefCountedGlobal(v8::Isolate* isolate, v8::Local<v8::Value> value)
: handle_(isolate, v8::Local<T>::Cast(value)) {
}
bool IsAlive() const {
return !handle_.IsEmpty();
}
v8::Local<T> NewHandle(v8::Isolate* isolate) const {
return v8::Local<T>::New(isolate, handle_);
}
private:
v8::Global<T> handle_;
DISALLOW_COPY_AND_ASSIGN(RefCountedGlobal);
};
SafeV8Function::SafeV8Function(v8::Isolate* isolate, v8::Local<v8::Value> value)
: v8_function_(new RefCountedPersistent<v8::Function>(isolate, value)),
weak_factory_(this) {
Init();
: v8_function_(new RefCountedGlobal<v8::Function>(isolate, value)) {
}
SafeV8Function::SafeV8Function(const SafeV8Function& other)
: v8_function_(other.v8_function_),
weak_factory_(this) {
Init();
: v8_function_(other.v8_function_) {
}
v8::Local<v8::Function> SafeV8Function::NewHandle() const {
return v8_function_->NewHandle();
SafeV8Function::~SafeV8Function() {
}
void SafeV8Function::Init() {
if (Locker::IsBrowserProcess() && atom::AtomBrowserMainParts::Get())
atom::AtomBrowserMainParts::Get()->RegisterDestructionCallback(
base::Bind(&SafeV8Function::FreeHandle, weak_factory_.GetWeakPtr()));
bool SafeV8Function::IsAlive() const {
return v8_function_.get() && v8_function_->IsAlive();
}
void SafeV8Function::FreeHandle() {
Locker locker(v8_function_->isolate());
v8_function_ = nullptr;
v8::Local<v8::Function> SafeV8Function::NewHandle(v8::Isolate* isolate) const {
return v8_function_->NewHandle(isolate);
}
v8::Local<v8::Value> CreateFunctionFromTranslater(

View file

@ -19,23 +19,21 @@ namespace mate {
namespace internal {
// Manages the V8 function with RAII, and automatically cleans the handle when
// JavaScript context is destroyed, even when the class is not destroyed.
template<typename T>
class RefCountedGlobal;
// Manages the V8 function with RAII.
class SafeV8Function {
public:
SafeV8Function(v8::Isolate* isolate, v8::Local<v8::Value> value);
SafeV8Function(const SafeV8Function& other);
~SafeV8Function();
bool is_alive() const { return v8_function_.get(); }
v8::Local<v8::Function> NewHandle() const;
bool IsAlive() const;
v8::Local<v8::Function> NewHandle(v8::Isolate* isolate) const;
private:
void Init();
void FreeHandle();
scoped_refptr<RefCountedPersistent<v8::Function>> v8_function_;
base::WeakPtrFactory<SafeV8Function> weak_factory_;
scoped_refptr<RefCountedGlobal<v8::Function>> v8_function_;
};
// Helper to invoke a V8 function with C++ parameters.
@ -49,12 +47,12 @@ struct V8FunctionInvoker<v8::Local<v8::Value>(ArgTypes...)> {
ArgTypes... raw) {
Locker locker(isolate);
v8::EscapableHandleScope handle_scope(isolate);
if (!function.is_alive())
if (!function.IsAlive())
return v8::Null(isolate);
scoped_ptr<blink::WebScopedRunV8Script> script_scope(
Locker::IsBrowserProcess() ?
nullptr : new blink::WebScopedRunV8Script(isolate));
v8::Local<v8::Function> holder = function.NewHandle();
v8::Local<v8::Function> holder = function.NewHandle(isolate);
v8::Local<v8::Context> context = holder->CreationContext();
v8::Context::Scope context_scope(context);
std::vector<v8::Local<v8::Value>> args = { ConvertToV8(isolate, raw)... };
@ -70,12 +68,12 @@ struct V8FunctionInvoker<void(ArgTypes...)> {
ArgTypes... raw) {
Locker locker(isolate);
v8::HandleScope handle_scope(isolate);
if (!function.is_alive())
if (!function.IsAlive())
return;
scoped_ptr<blink::WebScopedRunV8Script> script_scope(
Locker::IsBrowserProcess() ?
nullptr : new blink::WebScopedRunV8Script(isolate));
v8::Local<v8::Function> holder = function.NewHandle();
v8::Local<v8::Function> holder = function.NewHandle(isolate);
v8::Local<v8::Context> context = holder->CreationContext();
v8::Context::Scope context_scope(context);
std::vector<v8::Local<v8::Value>> args = { ConvertToV8(isolate, raw)... };
@ -91,12 +89,12 @@ struct V8FunctionInvoker<ReturnType(ArgTypes...)> {
Locker locker(isolate);
v8::HandleScope handle_scope(isolate);
ReturnType ret = ReturnType();
if (!function.is_alive())
if (!function.IsAlive())
return ret;
scoped_ptr<blink::WebScopedRunV8Script> script_scope(
Locker::IsBrowserProcess() ?
nullptr : new blink::WebScopedRunV8Script(isolate));
v8::Local<v8::Function> holder = function.NewHandle();
v8::Local<v8::Function> holder = function.NewHandle(isolate);
v8::Local<v8::Context> context = holder->CreationContext();
v8::Context::Scope context_scope(context);
std::vector<v8::Local<v8::Value>> args = { ConvertToV8(isolate, raw)... };

2
vendor/native_mate vendored

@ -1 +1 @@
Subproject commit 93984941005bab194a2d47aff655d525c064efcb
Subproject commit e859228db163c27410fb200c2df0715478fdf0d7