From 65b4d80503d818d82dc5e5669a0b16274dfc9cd9 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Wed, 6 Apr 2022 23:03:20 +0200 Subject: [PATCH] feat: allow null when subscribing notification --- docs/api/system-preferences.md | 6 ++-- .../api/electron_api_system_preferences.h | 8 ++--- .../electron_api_system_preferences_mac.mm | 29 +++++++++++----- spec-main/api-system-preferences-spec.ts | 33 +++++++++++++++++++ 4 files changed, 61 insertions(+), 15 deletions(-) diff --git a/docs/api/system-preferences.md b/docs/api/system-preferences.md index 154e5750f093d..89fb4b7338955 100644 --- a/docs/api/system-preferences.md +++ b/docs/api/system-preferences.md @@ -84,7 +84,7 @@ that contains the user information dictionary sent along with the notification. ### `systemPreferences.subscribeNotification(event, callback)` _macOS_ -* `event` string +* `event` string | null * `callback` Function * `event` string * `userInfo` Record @@ -111,7 +111,7 @@ example values of `event` are: ### `systemPreferences.subscribeLocalNotification(event, callback)` _macOS_ -* `event` string +* `event` string | null * `callback` Function * `event` string * `userInfo` Record @@ -124,7 +124,7 @@ This is necessary for events such as `NSUserDefaultsDidChangeNotification`. ### `systemPreferences.subscribeWorkspaceNotification(event, callback)` _macOS_ -* `event` string +* `event` string | null * `callback` Function * `event` string * `userInfo` Record diff --git a/shell/browser/api/electron_api_system_preferences.h b/shell/browser/api/electron_api_system_preferences.h index 592df523a3fea..d35dd5b947bea 100644 --- a/shell/browser/api/electron_api_system_preferences.h +++ b/shell/browser/api/electron_api_system_preferences.h @@ -76,17 +76,17 @@ class SystemPreferences void PostNotification(const std::string& name, base::DictionaryValue user_info, gin::Arguments* args); - int SubscribeNotification(const std::string& name, + int SubscribeNotification(v8::Local maybe_name, const NotificationCallback& callback); void UnsubscribeNotification(int id); void PostLocalNotification(const std::string& name, base::DictionaryValue user_info); - int SubscribeLocalNotification(const std::string& name, + int SubscribeLocalNotification(v8::Local maybe_name, const NotificationCallback& callback); void UnsubscribeLocalNotification(int request_id); void PostWorkspaceNotification(const std::string& name, base::DictionaryValue user_info); - int SubscribeWorkspaceNotification(const std::string& name, + int SubscribeWorkspaceNotification(v8::Local maybe_name, const NotificationCallback& callback); void UnsubscribeWorkspaceNotification(int request_id); v8::Local GetUserDefault(v8::Isolate* isolate, @@ -130,7 +130,7 @@ class SystemPreferences ~SystemPreferences() override; #if BUILDFLAG(IS_MAC) - int DoSubscribeNotification(const std::string& name, + int DoSubscribeNotification(v8::Local maybe_name, const NotificationCallback& callback, NotificationCenterKind kind); void DoUnsubscribeNotification(int request_id, NotificationCenterKind kind); diff --git a/shell/browser/api/electron_api_system_preferences_mac.mm b/shell/browser/api/electron_api_system_preferences_mac.mm index 56fd1d396921c..70006a88dd157 100644 --- a/shell/browser/api/electron_api_system_preferences_mac.mm +++ b/shell/browser/api/electron_api_system_preferences_mac.mm @@ -154,10 +154,11 @@ AVMediaType ParseMediaType(const std::string& media_type) { } int SystemPreferences::SubscribeNotification( - const std::string& name, + v8::Local maybe_name, const NotificationCallback& callback) { return DoSubscribeNotification( - name, callback, NotificationCenterKind::kNSDistributedNotificationCenter); + maybe_name, callback, + NotificationCenterKind::kNSDistributedNotificationCenter); } void SystemPreferences::UnsubscribeNotification(int request_id) { @@ -174,9 +175,9 @@ AVMediaType ParseMediaType(const std::string& media_type) { } int SystemPreferences::SubscribeLocalNotification( - const std::string& name, + v8::Local maybe_name, const NotificationCallback& callback) { - return DoSubscribeNotification(name, callback, + return DoSubscribeNotification(maybe_name, callback, NotificationCenterKind::kNSNotificationCenter); } @@ -196,10 +197,11 @@ AVMediaType ParseMediaType(const std::string& media_type) { } int SystemPreferences::SubscribeWorkspaceNotification( - const std::string& name, + v8::Local maybe_name, const NotificationCallback& callback) { return DoSubscribeNotification( - name, callback, NotificationCenterKind::kNSWorkspaceNotificationCenter); + maybe_name, callback, + NotificationCenterKind::kNSWorkspaceNotificationCenter); } void SystemPreferences::UnsubscribeWorkspaceNotification(int request_id) { @@ -208,14 +210,25 @@ AVMediaType ParseMediaType(const std::string& media_type) { } int SystemPreferences::DoSubscribeNotification( - const std::string& name, + v8::Local maybe_name, const NotificationCallback& callback, NotificationCenterKind kind) { int request_id = g_next_id++; __block NotificationCallback copied_callback = callback; + v8::Isolate* isolate = JavascriptEnvironment::GetIsolate(); + std::string name_str; + if (!(maybe_name->IsNull() || + gin::ConvertFromV8(isolate, maybe_name, &name_str))) { + isolate->ThrowException(v8::Exception::Error( + gin::StringToV8(isolate, "Must pass null or a string"))); + return -1; + } + + auto* name = maybe_name->IsNull() ? nil : base::SysUTF8ToNSString(name_str); + g_id_map[request_id] = [GetNotificationCenter(kind) - addObserverForName:base::SysUTF8ToNSString(name) + addObserverForName:name object:nil queue:nil usingBlock:^(NSNotification* notification) { diff --git a/spec-main/api-system-preferences-spec.ts b/spec-main/api-system-preferences-spec.ts index 3670527649302..38a16045ddb13 100644 --- a/spec-main/api-system-preferences-spec.ts +++ b/spec-main/api-system-preferences-spec.ts @@ -116,6 +116,39 @@ describe('systemPreferences module', () => { }); }); + ifdescribe(process.platform === 'darwin')('systemPreferences.subscribeNotification(event, callback)', () => { + it('throws an error if invalid arguments are passed', () => { + const badArgs = [123, {}, ['hi', 'bye'], new Date()]; + for (const bad of badArgs) { + expect(() => { + systemPreferences.subscribeNotification(bad as any, () => {}); + }).to.throw('Must pass null or a string'); + } + }); + }); + + ifdescribe(process.platform === 'darwin')('systemPreferences.subscribeLocalNotification(event, callback)', () => { + it('throws an error if invalid arguments are passed', () => { + const badArgs = [123, {}, ['hi', 'bye'], new Date()]; + for (const bad of badArgs) { + expect(() => { + systemPreferences.subscribeNotification(bad as any, () => {}); + }).to.throw('Must pass null or a string'); + } + }); + }); + + ifdescribe(process.platform === 'darwin')('systemPreferences.subscribeWorkspaceNotification(event, callback)', () => { + it('throws an error if invalid arguments are passed', () => { + const badArgs = [123, {}, ['hi', 'bye'], new Date()]; + for (const bad of badArgs) { + expect(() => { + systemPreferences.subscribeWorkspaceNotification(bad as any, () => {}); + }).to.throw('Must pass null or a string'); + } + }); + }); + ifdescribe(process.platform === 'darwin')('systemPreferences.getSystemColor(color)', () => { it('throws on invalid system colors', () => { const color = 'bad-color';