From 765c08c600ece850702f78b4e92d7fdc529f2aaa Mon Sep 17 00:00:00 2001 From: Jeremy Apthorp Date: Tue, 31 Mar 2020 11:42:32 -0700 Subject: [PATCH] refactor: ginify Notification (#22821) --- lib/browser/api/notification.js | 3 -- .../browser/api/electron_api_notification.cc | 34 ++++++++----------- shell/browser/api/electron_api_notification.h | 27 +++++++++++---- spec-main/api-notification-spec.ts | 34 +++++++++++++++++++ 4 files changed, 68 insertions(+), 30 deletions(-) diff --git a/lib/browser/api/notification.js b/lib/browser/api/notification.js index 862f2646471a3..4619b1223fb12 100644 --- a/lib/browser/api/notification.js +++ b/lib/browser/api/notification.js @@ -1,10 +1,7 @@ 'use strict'; -const { EventEmitter } = require('events'); const { Notification, isSupported } = process.electronBinding('notification'); -Object.setPrototypeOf(Notification.prototype, EventEmitter.prototype); - Notification.isSupported = isSupported; module.exports = Notification; diff --git a/shell/browser/api/electron_api_notification.cc b/shell/browser/api/electron_api_notification.cc index 1aaedb6701cfd..f7bf4f973badf 100644 --- a/shell/browser/api/electron_api_notification.cc +++ b/shell/browser/api/electron_api_notification.cc @@ -6,6 +6,7 @@ #include "base/guid.h" #include "base/strings/utf_string_conversions.h" +#include "gin/handle.h" #include "shell/browser/api/electron_api_menu.h" #include "shell/browser/browser.h" #include "shell/browser/electron_browser_client.h" @@ -48,9 +49,9 @@ namespace electron { namespace api { -Notification::Notification(gin::Arguments* args) { - InitWithArgs(args); +gin::WrapperInfo Notification::kWrapperInfo = {gin::kEmbedderNativeGin}; +Notification::Notification(gin::Arguments* args) { presenter_ = static_cast(ElectronBrowserClient::Get()) ->GetNotificationPresenter(); @@ -80,13 +81,13 @@ Notification::~Notification() { } // static -gin_helper::WrappableBase* Notification::New(gin_helper::ErrorThrower thrower, - gin::Arguments* args) { +gin::Handle Notification::New(gin_helper::ErrorThrower thrower, + gin::Arguments* args) { if (!Browser::Get()->is_ready()) { thrower.ThrowError("Cannot create Notification before app is ready"); - return nullptr; + return gin::Handle(); } - return new Notification(args); + return gin::CreateHandle(thrower.isolate(), new Notification(args)); } // Getters @@ -240,12 +241,10 @@ bool Notification::IsSupported() { ->GetNotificationPresenter(); } -// static -void Notification::BuildPrototype(v8::Isolate* isolate, - v8::Local prototype) { - prototype->SetClassName(gin::StringToV8(isolate, "Notification")); - gin_helper::Destroyable::MakeDestroyable(isolate, prototype); - gin_helper::ObjectTemplateBuilder(isolate, prototype->PrototypeTemplate()) +v8::Local Notification::FillObjectTemplate( + v8::Isolate* isolate, + v8::Local templ) { + return gin::ObjectTemplateBuilder(isolate, "Notification", templ) .SetMethod("show", &Notification::Show) .SetMethod("close", &Notification::Close) .SetProperty("title", &Notification::GetTitle, &Notification::SetTitle) @@ -265,7 +264,8 @@ void Notification::BuildPrototype(v8::Isolate* isolate, .SetProperty("actions", &Notification::GetActions, &Notification::SetActions) .SetProperty("closeButtonText", &Notification::GetCloseButtonText, - &Notification::SetCloseButtonText); + &Notification::SetCloseButtonText) + .Build(); } } // namespace api @@ -281,14 +281,8 @@ void Initialize(v8::Local exports, v8::Local context, void* priv) { v8::Isolate* isolate = context->GetIsolate(); - Notification::SetConstructor(isolate, - base::BindRepeating(&Notification::New)); - gin_helper::Dictionary dict(isolate, exports); - dict.Set("Notification", Notification::GetConstructor(isolate) - ->GetFunction(context) - .ToLocalChecked()); - + dict.Set("Notification", Notification::GetConstructor(context)); dict.SetMethod("isSupported", &Notification::IsSupported); } diff --git a/shell/browser/api/electron_api_notification.h b/shell/browser/api/electron_api_notification.h index d903ff050c9fa..18b3bff41c381 100644 --- a/shell/browser/api/electron_api_notification.h +++ b/shell/browser/api/electron_api_notification.h @@ -10,30 +10,40 @@ #include #include "base/strings/utf_string_conversions.h" +#include "gin/wrappable.h" +#include "shell/browser/event_emitter_mixin.h" #include "shell/browser/notifications/notification.h" #include "shell/browser/notifications/notification_delegate.h" #include "shell/browser/notifications/notification_presenter.h" +#include "shell/common/gin_helper/cleaned_up_at_exit.h" +#include "shell/common/gin_helper/constructible.h" #include "shell/common/gin_helper/error_thrower.h" -#include "shell/common/gin_helper/trackable_object.h" #include "ui/gfx/image/image.h" namespace gin { class Arguments; -} +template +class Handle; +} // namespace gin namespace electron { namespace api { -class Notification : public gin_helper::TrackableObject, +class Notification : public gin::Wrappable, + public gin_helper::EventEmitterMixin, + public gin_helper::Constructible, + public gin_helper::CleanedUpAtExit, public NotificationDelegate { public: - static gin_helper::WrappableBase* New(gin_helper::ErrorThrower thrower, - gin::Arguments* args); static bool IsSupported(); - static void BuildPrototype(v8::Isolate* isolate, - v8::Local prototype); + // gin_helper::Constructible + static gin::Handle New(gin_helper::ErrorThrower thrower, + gin::Arguments* args); + static v8::Local FillObjectTemplate( + v8::Isolate*, + v8::Local); // NotificationDelegate: void NotificationAction(int index) override; @@ -43,6 +53,9 @@ class Notification : public gin_helper::TrackableObject, void NotificationDestroyed() override; void NotificationClosed() override; + // gin::Wrappable + static gin::WrapperInfo kWrapperInfo; + protected: explicit Notification(gin::Arguments* args); ~Notification() override; diff --git a/spec-main/api-notification-spec.ts b/spec-main/api-notification-spec.ts index f30cae8d50c15..9862eabcc36d7 100644 --- a/spec-main/api-notification-spec.ts +++ b/spec-main/api-notification-spec.ts @@ -1,7 +1,13 @@ import { expect } from 'chai'; import { Notification } from 'electron'; +import { emittedOnce } from './events-helpers'; +import { ifit } from './spec-helpers'; describe('Notification module', () => { + it('is supported', () => { + expect(Notification.isSupported()).to.be.a('boolean'); + }); + it('inits, gets and sets basic string properties correctly', () => { const n = new Notification({ title: 'title', @@ -92,5 +98,33 @@ describe('Notification module', () => { expect(n.actions[1].text).to.equal('4'); }); + it('can be shown and closed', () => { + const n = new Notification({ + title: 'test notification', + body: 'test body', + silent: true + }); + n.show(); + n.close(); + }); + + ifit(process.platform === 'darwin')('emits show and close events', async () => { + const n = new Notification({ + title: 'test notification', + body: 'test body', + silent: true + }); + { + const e = emittedOnce(n, 'show'); + n.show(); + await e; + } + { + const e = emittedOnce(n, 'close'); + n.close(); + await e; + } + }); + // TODO(sethlu): Find way to test init with notification icon? });