Do not manually manage native resources

We should rely on the destructor to cleanup everything, instead of
putting them in the Destroy method.
This commit is contained in:
Cheng Zhao 2015-12-03 15:38:43 +08:00
parent e5358d405a
commit 6795bd1d96
23 changed files with 59 additions and 149 deletions

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() {
@ -147,6 +138,7 @@ void DownloadItem::Cancel() {
mate::ObjectTemplateBuilder DownloadItem::GetObjectTemplateBuilder(
v8::Isolate* isolate) {
return mate::ObjectTemplateBuilder(isolate)
.MakeDestroyable()
.SetMethod("pause", &DownloadItem::Pause)
.SetMethod("resume", &DownloadItem::Resume)
.SetMethod("cancel", &DownloadItem::Cancel)

View file

@ -56,9 +56,6 @@ class DownloadItem : public mate::EventEmitter,
// mate::Wrappable:
mate::ObjectTemplateBuilder GetObjectTemplateBuilder(
v8::Isolate* isolate) override;
bool IsDestroyed() const override;
void Destroy();
content::DownloadItem* download_item_;

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);
}
@ -379,6 +370,7 @@ v8::Local<v8::Value> Session::Cookies(v8::Isolate* isolate) {
mate::ObjectTemplateBuilder Session::GetObjectTemplateBuilder(
v8::Isolate* isolate) {
return mate::ObjectTemplateBuilder(isolate)
.MakeDestroyable()
.SetMethod("resolveProxy", &Session::ResolveProxy)
.SetMethod("clearCache", &Session::ClearCache)
.SetMethod("clearStorageData", &Session::ClearStorageData)

View file

@ -59,12 +59,8 @@ class Session: public mate::TrackableObject<Session>,
// 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

@ -290,7 +290,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 +599,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();
}
@ -991,8 +986,7 @@ 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)
.MakeDestroyable()
.SetMethod("getId", &WebContents::GetID)
.SetMethod("equal", &WebContents::Equal)
.SetMethod("_loadURL", &WebContents::LoadURL)
@ -1034,7 +1028,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,19 +1046,14 @@ 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)
.SetProperty("session", &WebContents::Session)
.SetProperty("devToolsWebContents", &WebContents::DevToolsWebContents)
.Build());
return mate::ObjectTemplateBuilder(
isolate, v8::Local<v8::ObjectTemplate>::New(isolate, template_));
}
bool WebContents::IsDestroyed() const {
return !web_contents();
}
AtomBrowserContext* WebContents::GetBrowserContext() const {
return static_cast<AtomBrowserContext*>(web_contents()->GetBrowserContext());
}

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);
@ -152,7 +149,6 @@ class WebContents : public mate::TrackableObject<WebContents>,
// mate::Wrappable:
mate::ObjectTemplateBuilder GetObjectTemplateBuilder(
v8::Isolate* isolate) override;
bool IsDestroyed() const override;
// content::WebContentsDelegate:
bool AddMessageToConsole(content::WebContents* source,

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();
}
api_web_contents_->DestroyWebContents();
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,8 +30,7 @@ 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()));
RegisterDestructionCallback(GetDestroyClosure());
}
TrackableObjectBase::~TrackableObjectBase() {
@ -42,6 +41,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_));

View file

@ -30,15 +30,18 @@ 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);
@ -50,6 +53,8 @@ class TrackableObjectBase : public mate::EventEmitter {
base::SupportsUserData* wrapped_;
private:
void Destroy();
base::WeakPtrFactory<TrackableObjectBase> weak_factory_;
DISALLOW_COPY_AND_ASSIGN(TrackableObjectBase);

2
vendor/native_mate vendored

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