From 362da77c0a0223a32526543f7b80ef4d9d76c105 Mon Sep 17 00:00:00 2001 From: Jeremy Rose Date: Tue, 28 Jul 2020 11:03:30 -0700 Subject: [PATCH] refactor: ginify SystemPreferences (#24675) --- lib/browser/api/system-preferences.ts | 25 ++++----- .../api/electron_api_system_preferences.cc | 26 +++++----- .../api/electron_api_system_preferences.h | 35 ++++++++----- .../electron_api_system_preferences_mac.mm | 52 ++++++++++--------- .../electron_api_system_preferences_win.cc | 6 +-- spec-main/api-system-preferences-spec.ts | 2 +- 6 files changed, 76 insertions(+), 70 deletions(-) diff --git a/lib/browser/api/system-preferences.ts b/lib/browser/api/system-preferences.ts index 9d39f4f2b722..b2509f0a3a75 100644 --- a/lib/browser/api/system-preferences.ts +++ b/lib/browser/api/system-preferences.ts @@ -1,15 +1,10 @@ -import { EventEmitter } from 'events'; import { deprecate } from 'electron/main'; -const { systemPreferences, SystemPreferences } = process._linkedBinding('electron_browser_system_preferences'); - -// SystemPreferences is an EventEmitter. -Object.setPrototypeOf(SystemPreferences.prototype, EventEmitter.prototype); -EventEmitter.call(systemPreferences); +const { systemPreferences } = process._linkedBinding('electron_browser_system_preferences'); if ('getAppLevelAppearance' in systemPreferences) { const nativeALAGetter = systemPreferences.getAppLevelAppearance; const nativeALASetter = systemPreferences.setAppLevelAppearance; - Object.defineProperty(SystemPreferences.prototype, 'appLevelAppearance', { + Object.defineProperty(systemPreferences, 'appLevelAppearance', { get: () => nativeALAGetter.call(systemPreferences), set: (appearance) => nativeALASetter.call(systemPreferences, appearance) }); @@ -17,25 +12,25 @@ if ('getAppLevelAppearance' in systemPreferences) { if ('getEffectiveAppearance' in systemPreferences) { const nativeEAGetter = systemPreferences.getAppLevelAppearance; - Object.defineProperty(SystemPreferences.prototype, 'effectiveAppearance', { + Object.defineProperty(systemPreferences, 'effectiveAppearance', { get: () => nativeEAGetter.call(systemPreferences) }); } -SystemPreferences.prototype.isDarkMode = deprecate.moveAPI( - SystemPreferences.prototype.isDarkMode, +systemPreferences.isDarkMode = deprecate.moveAPI( + systemPreferences.isDarkMode, 'systemPreferences.isDarkMode()', 'nativeTheme.shouldUseDarkColors' ); -SystemPreferences.prototype.isInvertedColorScheme = deprecate.moveAPI( - SystemPreferences.prototype.isInvertedColorScheme, +systemPreferences.isInvertedColorScheme = deprecate.moveAPI( + systemPreferences.isInvertedColorScheme, 'systemPreferences.isInvertedColorScheme()', 'nativeTheme.shouldUseInvertedColorScheme' ); -SystemPreferences.prototype.isHighContrastColorScheme = deprecate.moveAPI( - SystemPreferences.prototype.isHighContrastColorScheme, +systemPreferences.isHighContrastColorScheme = deprecate.moveAPI( + systemPreferences.isHighContrastColorScheme, 'systemPreferences.isHighContrastColorScheme()', 'nativeTheme.shouldUseHighContrastColors' ); -module.exports = systemPreferences; +export default systemPreferences; diff --git a/shell/browser/api/electron_api_system_preferences.cc b/shell/browser/api/electron_api_system_preferences.cc index 7fb6fd409c23..db2d2f597c3a 100644 --- a/shell/browser/api/electron_api_system_preferences.cc +++ b/shell/browser/api/electron_api_system_preferences.cc @@ -7,7 +7,7 @@ #include "shell/common/gin_converters/callback_converter.h" #include "shell/common/gin_converters/value_converter.h" #include "shell/common/gin_helper/dictionary.h" -#include "shell/common/gin_helper/object_template_builder.h" +#include "shell/common/gin_helper/function_template_extensions.h" #include "shell/common/node_includes.h" #include "ui/gfx/animation/animation.h" #include "ui/gfx/color_utils.h" @@ -17,8 +17,9 @@ namespace electron { namespace api { -SystemPreferences::SystemPreferences(v8::Isolate* isolate) { - Init(isolate); +gin::WrapperInfo SystemPreferences::kWrapperInfo = {gin::kEmbedderNativeGin}; + +SystemPreferences::SystemPreferences() { #if defined(OS_WIN) InitializeWindow(); #endif @@ -61,15 +62,13 @@ v8::Local SystemPreferences::GetAnimationSettings( // static gin::Handle SystemPreferences::Create(v8::Isolate* isolate) { - return gin::CreateHandle(isolate, new SystemPreferences(isolate)); + return gin::CreateHandle(isolate, new SystemPreferences()); } -// static -void SystemPreferences::BuildPrototype( - v8::Isolate* isolate, - v8::Local prototype) { - prototype->SetClassName(gin::StringToV8(isolate, "SystemPreferences")); - gin_helper::ObjectTemplateBuilder(isolate, prototype->PrototypeTemplate()) +gin::ObjectTemplateBuilder SystemPreferences::GetObjectTemplateBuilder( + v8::Isolate* isolate) { + return gin_helper::EventEmitterMixin< + SystemPreferences>::GetObjectTemplateBuilder(isolate) #if defined(OS_WIN) || defined(OS_MACOSX) .SetMethod("getColor", &SystemPreferences::GetColor) .SetMethod("getAccentColor", &SystemPreferences::GetAccentColor) @@ -125,6 +124,10 @@ void SystemPreferences::BuildPrototype( &SystemPreferences::GetAnimationSettings); } +const char* SystemPreferences::GetTypeName() { + return "SystemPreferences"; +} + } // namespace api } // namespace electron @@ -140,9 +143,6 @@ void Initialize(v8::Local exports, v8::Isolate* isolate = context->GetIsolate(); gin_helper::Dictionary dict(isolate, exports); dict.Set("systemPreferences", SystemPreferences::Create(isolate)); - dict.Set("SystemPreferences", SystemPreferences::GetConstructor(isolate) - ->GetFunction(context) - .ToLocalChecked()); } } // namespace diff --git a/shell/browser/api/electron_api_system_preferences.h b/shell/browser/api/electron_api_system_preferences.h index 3f9b0bcbede7..aee1900752ab 100644 --- a/shell/browser/api/electron_api_system_preferences.h +++ b/shell/browser/api/electron_api_system_preferences.h @@ -10,8 +10,9 @@ #include "base/values.h" #include "gin/handle.h" +#include "gin/wrappable.h" +#include "shell/browser/event_emitter_mixin.h" #include "shell/common/gin_helper/error_thrower.h" -#include "shell/common/gin_helper/event_emitter.h" #include "shell/common/gin_helper/promise.h" #if defined(OS_WIN) @@ -32,25 +33,30 @@ enum NotificationCenterKind { }; #endif -class SystemPreferences : public gin_helper::EventEmitter +class SystemPreferences + : public gin::Wrappable, + public gin_helper::EventEmitterMixin #if defined(OS_WIN) , - public BrowserObserver, - public gfx::SysColorChangeListener + public BrowserObserver, + public gfx::SysColorChangeListener #endif { public: static gin::Handle Create(v8::Isolate* isolate); - static void BuildPrototype(v8::Isolate* isolate, - v8::Local prototype); + // gin::Wrappable + static gin::WrapperInfo kWrapperInfo; + gin::ObjectTemplateBuilder GetObjectTemplateBuilder( + v8::Isolate* isolate) override; + const char* GetTypeName() override; #if defined(OS_WIN) || defined(OS_MACOSX) std::string GetAccentColor(); std::string GetColor(gin_helper::ErrorThrower thrower, const std::string& color); - std::string GetMediaAccessStatus(const std::string& media_type, - gin_helper::Arguments* args); + std::string GetMediaAccessStatus(gin_helper::ErrorThrower thrower, + const std::string& media_type); #endif #if defined(OS_WIN) bool IsAeroGlassEnabled(); @@ -69,7 +75,7 @@ class SystemPreferences : public gin_helper::EventEmitter void PostNotification(const std::string& name, base::DictionaryValue user_info, - gin_helper::Arguments* args); + gin::Arguments* args); int SubscribeNotification(const std::string& name, const NotificationCallback& callback); void UnsubscribeNotification(int id); @@ -83,12 +89,13 @@ class SystemPreferences : public gin_helper::EventEmitter int SubscribeWorkspaceNotification(const std::string& name, const NotificationCallback& callback); void UnsubscribeWorkspaceNotification(int request_id); - v8::Local GetUserDefault(const std::string& name, + v8::Local GetUserDefault(v8::Isolate* isolate, + const std::string& name, const std::string& type); - void RegisterDefaults(gin_helper::Arguments* args); + void RegisterDefaults(gin::Arguments* args); void SetUserDefault(const std::string& name, const std::string& type, - gin_helper::Arguments* args); + gin::Arguments* args); void RemoveUserDefault(const std::string& name); bool IsSwipeTrackingFromScrollEventsEnabled(); @@ -108,7 +115,7 @@ class SystemPreferences : public gin_helper::EventEmitter // are running tests on a Mojave machine v8::Local GetEffectiveAppearance(v8::Isolate* isolate); v8::Local GetAppLevelAppearance(v8::Isolate* isolate); - void SetAppLevelAppearance(gin_helper::Arguments* args); + void SetAppLevelAppearance(gin::Arguments* args); #endif bool IsDarkMode(); bool IsInvertedColorScheme(); @@ -116,7 +123,7 @@ class SystemPreferences : public gin_helper::EventEmitter v8::Local GetAnimationSettings(v8::Isolate* isolate); protected: - explicit SystemPreferences(v8::Isolate* isolate); + SystemPreferences(); ~SystemPreferences() override; #if defined(OS_MACOSX) diff --git a/shell/browser/api/electron_api_system_preferences_mac.mm b/shell/browser/api/electron_api_system_preferences_mac.mm index 489548f34eb2..a06879bfb18c 100644 --- a/shell/browser/api/electron_api_system_preferences_mac.mm +++ b/shell/browser/api/electron_api_system_preferences_mac.mm @@ -125,7 +125,7 @@ std::string ConvertSystemPermission( void SystemPreferences::PostNotification(const std::string& name, base::DictionaryValue user_info, - gin_helper::Arguments* args) { + gin::Arguments* args) { bool immediate = false; args->GetNext(&immediate); @@ -257,60 +257,61 @@ void SystemPreferences::DoUnsubscribeNotification(int request_id, } v8::Local SystemPreferences::GetUserDefault( + v8::Isolate* isolate, const std::string& name, const std::string& type) { NSUserDefaults* defaults = [NSUserDefaults standardUserDefaults]; NSString* key = base::SysUTF8ToNSString(name); if (type == "string") { return gin::StringToV8( - isolate(), base::SysNSStringToUTF8([defaults stringForKey:key])); + isolate, base::SysNSStringToUTF8([defaults stringForKey:key])); } else if (type == "boolean") { - return v8::Boolean::New(isolate(), [defaults boolForKey:key]); + return v8::Boolean::New(isolate, [defaults boolForKey:key]); } else if (type == "float") { - return v8::Number::New(isolate(), [defaults floatForKey:key]); + return v8::Number::New(isolate, [defaults floatForKey:key]); } else if (type == "integer") { - return v8::Integer::New(isolate(), [defaults integerForKey:key]); + return v8::Integer::New(isolate, [defaults integerForKey:key]); } else if (type == "double") { - return v8::Number::New(isolate(), [defaults doubleForKey:key]); + return v8::Number::New(isolate, [defaults doubleForKey:key]); } else if (type == "url") { - return gin::ConvertToV8(isolate(), + return gin::ConvertToV8(isolate, net::GURLWithNSURL([defaults URLForKey:key])); } else if (type == "array") { - return gin::ConvertToV8(isolate(), + return gin::ConvertToV8(isolate, NSArrayToListValue([defaults arrayForKey:key])); } else if (type == "dictionary") { - return gin::ConvertToV8(isolate(), NSDictionaryToDictionaryValue( - [defaults dictionaryForKey:key])); + return gin::ConvertToV8(isolate, NSDictionaryToDictionaryValue( + [defaults dictionaryForKey:key])); } else { - return v8::Undefined(isolate()); + return v8::Undefined(isolate); } } -void SystemPreferences::RegisterDefaults(gin_helper::Arguments* args) { +void SystemPreferences::RegisterDefaults(gin::Arguments* args) { base::DictionaryValue value; if (!args->GetNext(&value)) { - args->ThrowError("Invalid userDefault data provided"); + args->ThrowError(); } else { @try { NSDictionary* dict = DictionaryValueToNSDictionary(value); for (id key in dict) { id value = [dict objectForKey:key]; if ([value isKindOfClass:[NSNull class]] || value == nil) { - args->ThrowError("Invalid userDefault data provided"); + args->ThrowError(); return; } } [[NSUserDefaults standardUserDefaults] registerDefaults:dict]; } @catch (NSException* exception) { - args->ThrowError("Invalid userDefault data provided"); + args->ThrowError(); } } } void SystemPreferences::SetUserDefault(const std::string& name, const std::string& type, - gin_helper::Arguments* args) { + gin::Arguments* args) { NSUserDefaults* defaults = [NSUserDefaults standardUserDefaults]; NSString* key = base::SysUTF8ToNSString(name); if (type == "string") { @@ -321,7 +322,8 @@ void SystemPreferences::SetUserDefault(const std::string& name, } } else if (type == "boolean") { bool value; - if (args->GetNext(&value)) { + v8::Local next = args->PeekNext(); + if (!next.IsEmpty() && next->IsBoolean() && args->GetNext(&value)) { [defaults setBool:value forKey:key]; return; } @@ -368,11 +370,13 @@ void SystemPreferences::SetUserDefault(const std::string& name, } } } else { - args->ThrowError("Invalid type: " + type); + gin_helper::ErrorThrower(args->isolate()) + .ThrowTypeError("Invalid type: " + type); return; } - args->ThrowError("Unable to convert value to: " + type); + gin_helper::ErrorThrower(args->isolate()) + .ThrowTypeError("Unable to convert value to: " + type); } std::string SystemPreferences::GetAccentColor() { @@ -572,8 +576,8 @@ std::string SystemPreferences::GetColor(gin_helper::ErrorThrower thrower, } std::string SystemPreferences::GetMediaAccessStatus( - const std::string& media_type, - gin_helper::Arguments* args) { + gin_helper::ErrorThrower thrower, + const std::string& media_type) { if (media_type == "camera") { return ConvertSystemPermission( system_media_permissions::CheckSystemVideoCapturePermission()); @@ -584,7 +588,7 @@ std::string SystemPreferences::GetMediaAccessStatus( return ConvertSystemPermission( system_media_permissions::CheckSystemScreenCapturePermission()); } else { - args->ThrowError("Invalid media type"); + thrower.ThrowError("Invalid media type"); return std::string(); } } @@ -651,13 +655,13 @@ v8::Local SystemPreferences::GetAppLevelAppearance( return v8::Null(isolate); } -void SystemPreferences::SetAppLevelAppearance(gin_helper::Arguments* args) { +void SystemPreferences::SetAppLevelAppearance(gin::Arguments* args) { if (@available(macOS 10.14, *)) { NSAppearance* appearance; if (args->GetNext(&appearance)) { [[NSApplication sharedApplication] setAppearance:appearance]; } else { - args->ThrowError("Invalid app appearance provided as first argument"); + args->ThrowError(); } } } diff --git a/shell/browser/api/electron_api_system_preferences_win.cc b/shell/browser/api/electron_api_system_preferences_win.cc index b106bd43c4a2..76fc6d079178 100644 --- a/shell/browser/api/electron_api_system_preferences_win.cc +++ b/shell/browser/api/electron_api_system_preferences_win.cc @@ -167,8 +167,8 @@ std::string SystemPreferences::GetColor(gin_helper::ErrorThrower thrower, } std::string SystemPreferences::GetMediaAccessStatus( - const std::string& media_type, - gin_helper::Arguments* args) { + gin_helper::ErrorThrower thrower, + const std::string& media_type) { if (media_type == "camera") { return ConvertDeviceAccessStatus( GetDeviceAccessStatus(DeviceClass::DeviceClass_VideoCapture)); @@ -179,7 +179,7 @@ std::string SystemPreferences::GetMediaAccessStatus( return ConvertDeviceAccessStatus( DeviceAccessStatus::DeviceAccessStatus_Allowed); } else { - args->ThrowError("Invalid media type"); + thrower.ThrowError("Invalid media type"); return std::string(); } } diff --git a/spec-main/api-system-preferences-spec.ts b/spec-main/api-system-preferences-spec.ts index ee3b5ae5c19c..15429b54de5d 100644 --- a/spec-main/api-system-preferences-spec.ts +++ b/spec-main/api-system-preferences-spec.ts @@ -53,7 +53,7 @@ describe('systemPreferences module', () => { for (const badDefault of badDefaults) { expect(() => { systemPreferences.registerDefaults(badDefault as any); - }).to.throw('Invalid userDefault data provided'); + }).to.throw('Error processing argument at index 0, conversion failure from '); } }); });