From ca8f45a5019432fbe76f03390eae6b969e603bd2 Mon Sep 17 00:00:00 2001 From: Matt Crocker Date: Mon, 23 Oct 2017 22:37:03 -0700 Subject: [PATCH 1/5] Notifications should emit close on close, not eventual GC --- atom/browser/api/atom_api_notification.cc | 2 +- docs/api/notification.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/atom/browser/api/atom_api_notification.cc b/atom/browser/api/atom_api_notification.cc index ef5b9ce129..7db7dbf38a 100644 --- a/atom/browser/api/atom_api_notification.cc +++ b/atom/browser/api/atom_api_notification.cc @@ -169,10 +169,10 @@ void Notification::NotificationDisplayed() { } void Notification::NotificationDestroyed() { - Emit("close"); } void Notification::NotificationClosed() { + Emit("close"); } // Showing notifications diff --git a/docs/api/notification.md b/docs/api/notification.md index 402bbb69db..9c041a76ce 100644 --- a/docs/api/notification.md +++ b/docs/api/notification.md @@ -74,7 +74,7 @@ Returns: Emitted when the notification is closed by manual intervention from the user. -This event is not guarunteed to be emitted in all cases where the notification +This event is not guaranteed to be emitted in all cases where the notification is closed. #### Event: 'reply' _macOS_ From c5914516c851b3d3948c4d98c88c3b3f289b1bed Mon Sep 17 00:00:00 2001 From: Matt Crocker Date: Mon, 23 Oct 2017 22:38:55 -0700 Subject: [PATCH 2/5] Upstream good ideas from Muon --- atom/browser/api/event_emitter.h | 7 +++++-- atom/browser/api/trackable_object.h | 5 ++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/atom/browser/api/event_emitter.h b/atom/browser/api/event_emitter.h index ead3beddaa..cf580cc53b 100644 --- a/atom/browser/api/event_emitter.h +++ b/atom/browser/api/event_emitter.h @@ -79,8 +79,11 @@ class EventEmitter : public Wrappable { const Args&... args) { v8::Locker locker(isolate()); v8::HandleScope handle_scope(isolate()); - v8::Local event = internal::CreateJSEvent( - isolate(), GetWrapper(), sender, message); + v8::Local wrapper = GetWrapper(); + if (wrapper.IsEmpty()) + return false; + v8::Local event = internal::CreateJSEvent( + isolate(), wrapper, sender, message); return EmitWithEvent(name, event, args...); } diff --git a/atom/browser/api/trackable_object.h b/atom/browser/api/trackable_object.h index e632f52b84..f9225924d0 100644 --- a/atom/browser/api/trackable_object.h +++ b/atom/browser/api/trackable_object.h @@ -62,7 +62,10 @@ class TrackableObject : public TrackableObjectBase, public: // Mark the JS object as destroyed. void MarkDestroyed() { - Wrappable::GetWrapper()->SetAlignedPointerInInternalField(0, nullptr); + v8::Local wrapper = Wrappable::GetWrapper(); + if (!wrapper.IsEmpty()) { + Wrappable::GetWrapper()->SetAlignedPointerInInternalField(0, nullptr); + } } bool IsDestroyed() { From 77a26882a30a3dbd20987b5fc98ec34575c618d2 Mon Sep 17 00:00:00 2001 From: Matt Crocker Date: Mon, 23 Oct 2017 22:58:43 -0700 Subject: [PATCH 3/5] Make linter happy --- atom/browser/api/event_emitter.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/atom/browser/api/event_emitter.h b/atom/browser/api/event_emitter.h index cf580cc53b..f5a8025e86 100644 --- a/atom/browser/api/event_emitter.h +++ b/atom/browser/api/event_emitter.h @@ -80,9 +80,10 @@ class EventEmitter : public Wrappable { v8::Locker locker(isolate()); v8::HandleScope handle_scope(isolate()); v8::Local wrapper = GetWrapper(); - if (wrapper.IsEmpty()) + if (wrapper.IsEmpty()) { return false; - v8::Local event = internal::CreateJSEvent( + } + v8::Local event = internal::CreateJSEvent( isolate(), wrapper, sender, message); return EmitWithEvent(name, event, args...); } From b6fb016a9a720d633cb312f5801e326f0a0565ca Mon Sep 17 00:00:00 2001 From: Matt Crocker Date: Fri, 27 Oct 2017 00:07:54 -0700 Subject: [PATCH 4/5] Cleanup per review comment --- atom/browser/api/trackable_object.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atom/browser/api/trackable_object.h b/atom/browser/api/trackable_object.h index f9225924d0..6fcde898a6 100644 --- a/atom/browser/api/trackable_object.h +++ b/atom/browser/api/trackable_object.h @@ -64,7 +64,7 @@ class TrackableObject : public TrackableObjectBase, void MarkDestroyed() { v8::Local wrapper = Wrappable::GetWrapper(); if (!wrapper.IsEmpty()) { - Wrappable::GetWrapper()->SetAlignedPointerInInternalField(0, nullptr); + wrapper->SetAlignedPointerInInternalField(0, nullptr); } } From 42da83f8ca0d15d57e3c3a9b8aaa6a932560f55e Mon Sep 17 00:00:00 2001 From: Matt Crocker Date: Fri, 27 Oct 2017 00:11:16 -0700 Subject: [PATCH 5/5] Update native-mate to pick up related changes --- vendor/native_mate | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vendor/native_mate b/vendor/native_mate index f047bb61bb..bf92fa88b7 160000 --- a/vendor/native_mate +++ b/vendor/native_mate @@ -1 +1 @@ -Subproject commit f047bb61bbd985079dd93e7ed322999550efed1d +Subproject commit bf92fa88b735b8c9ff951e6ef78a531bd10e8778