refactor: reduce scope of temporaries when getting dictionary values (#47831)

refactor: reduce scale of temporaries when getting dictionary values

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Charles Kerr <charles@charleskerr.com>
This commit is contained in:
trop[bot] 2025-07-21 10:26:59 -04:00 committed by GitHub
parent ef9212a112
commit 4a81ae7954
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 48 additions and 88 deletions

View file

@ -130,19 +130,17 @@ NativeWindow::~NativeWindow() {
void NativeWindow::InitFromOptions(const gin_helper::Dictionary& options) { void NativeWindow::InitFromOptions(const gin_helper::Dictionary& options) {
// Setup window from options. // Setup window from options.
int x = -1, y = -1; if (int x, y; options.Get(options::kX, &x) && options.Get(options::kY, &y)) {
bool center; SetPosition(gfx::Point{x, y});
if (options.Get(options::kX, &x) && options.Get(options::kY, &y)) {
SetPosition(gfx::Point(x, y));
#if BUILDFLAG(IS_WIN) #if BUILDFLAG(IS_WIN)
// FIXME(felixrieseberg): Dirty, dirty workaround for // FIXME(felixrieseberg): Dirty, dirty workaround for
// https://github.com/electron/electron/issues/10862 // https://github.com/electron/electron/issues/10862
// Somehow, we need to call `SetBounds` twice to get // Somehow, we need to call `SetBounds` twice to get
// usable results. The root cause is still unknown. // usable results. The root cause is still unknown.
SetPosition(gfx::Point(x, y)); SetPosition(gfx::Point{x, y});
#endif #endif
} else if (options.Get(options::kCenter, &center) && center) { } else if (bool center; options.Get(options::kCenter, &center) && center) {
Center(); Center();
} }
@ -181,27 +179,21 @@ void NativeWindow::InitFromOptions(const gin_helper::Dictionary& options) {
SetSizeConstraints(size_constraints); SetSizeConstraints(size_constraints);
} }
#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_LINUX) #if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_LINUX)
bool closable; if (bool val; options.Get(options::kClosable, &val))
if (options.Get(options::kClosable, &closable)) { SetClosable(val);
SetClosable(closable);
}
#endif #endif
bool movable;
if (options.Get(options::kMovable, &movable)) { if (bool val; options.Get(options::kMovable, &val))
SetMovable(movable); SetMovable(val);
}
bool has_shadow; if (bool val; options.Get(options::kHasShadow, &val))
if (options.Get(options::kHasShadow, &has_shadow)) { SetHasShadow(val);
SetHasShadow(has_shadow);
} if (double val; options.Get(options::kOpacity, &val))
double opacity; SetOpacity(val);
if (options.Get(options::kOpacity, &opacity)) {
SetOpacity(opacity); if (bool val; options.Get(options::kAlwaysOnTop, &val) && val)
}
bool top;
if (options.Get(options::kAlwaysOnTop, &top) && top) {
SetAlwaysOnTop(ui::ZOrderLevel::kFloatingWindow); SetAlwaysOnTop(ui::ZOrderLevel::kFloatingWindow);
}
bool fullscreenable = true; bool fullscreenable = true;
bool fullscreen = false; bool fullscreen = false;
@ -218,29 +210,21 @@ void NativeWindow::InitFromOptions(const gin_helper::Dictionary& options) {
if (fullscreen) if (fullscreen)
SetFullScreen(true); SetFullScreen(true);
bool resizable; if (bool val; options.Get(options::kResizable, &val))
if (options.Get(options::kResizable, &resizable)) { SetResizable(val);
SetResizable(resizable);
} if (bool val; options.Get(options::kSkipTaskbar, &val))
SetSkipTaskbar(val);
if (bool val; options.Get(options::kKiosk, &val) && val)
SetKiosk(val);
bool skip;
if (options.Get(options::kSkipTaskbar, &skip)) {
SetSkipTaskbar(skip);
}
bool kiosk;
if (options.Get(options::kKiosk, &kiosk) && kiosk) {
SetKiosk(kiosk);
}
#if BUILDFLAG(IS_MAC) #if BUILDFLAG(IS_MAC)
std::string type; if (std::string val; options.Get(options::kVibrancyType, &val))
if (options.Get(options::kVibrancyType, &type)) { SetVibrancy(val, 0);
SetVibrancy(type, 0);
}
#elif BUILDFLAG(IS_WIN) #elif BUILDFLAG(IS_WIN)
std::string material; if (std::string val; options.Get(options::kBackgroundMaterial, &val))
if (options.Get(options::kBackgroundMaterial, &material)) { SetBackgroundMaterial(val);
SetBackgroundMaterial(material);
}
#endif #endif
SkColor background_color = SK_ColorWHITE; SkColor background_color = SK_ColorWHITE;
@ -251,9 +235,7 @@ void NativeWindow::InitFromOptions(const gin_helper::Dictionary& options) {
} }
SetBackgroundColor(background_color); SetBackgroundColor(background_color);
std::string title(Browser::Get()->GetName()); SetTitle(options.ValueOrDefault(options::kTitle, Browser::Get()->GetName()));
options.Get(options::kTitle, &title);
SetTitle(title);
// Then show it. // Then show it.
if (options.ValueOrDefault(options::kShow, true)) if (options.ValueOrDefault(options::kShow, true))

View file

@ -238,8 +238,7 @@ NativeWindowMac::NativeWindowMac(const gin_helper::Dictionary& options,
[window_ setLevel:NSFloatingWindowLevel]; [window_ setLevel:NSFloatingWindowLevel];
} }
bool focusable; if (bool val; options.Get(options::kFocusable, &val) && !val)
if (options.Get(options::kFocusable, &focusable) && !focusable)
[window_ setDisableKeyOrMainWindow:YES]; [window_ setDisableKeyOrMainWindow:YES];
if (transparent() || !has_frame()) { if (transparent() || !has_frame()) {
@ -284,12 +283,10 @@ NativeWindowMac::NativeWindowMac(const gin_helper::Dictionary& options,
// NOTE(@mlaurencin) Spec requirements can be found here: // NOTE(@mlaurencin) Spec requirements can be found here:
// https://developer.mozilla.org/en-US/docs/Web/API/Window/open#width // https://developer.mozilla.org/en-US/docs/Web/API/Window/open#width
constexpr int kMinSizeReqdBySpec = 100; constexpr int kMinSizeReqdBySpec = 100;
int inner_width = 0; const int inner_width = options.ValueOrDefault(options::kinnerWidth, 0);
int inner_height = 0; const int inner_height = options.ValueOrDefault(options::kinnerHeight, 0);
bool use_content_size = bool use_content_size =
options.ValueOrDefault(options::kUseContentSize, false); options.ValueOrDefault(options::kUseContentSize, false);
options.Get(options::kinnerWidth, &inner_width);
options.Get(options::kinnerHeight, &inner_height);
if (inner_width || inner_height) { if (inner_width || inner_height) {
use_content_size = true; use_content_size = true;
if (inner_width) if (inner_width)

View file

@ -196,9 +196,8 @@ NativeWindowViews::NativeWindowViews(const gin_helper::Dictionary& options,
: NativeWindow(options, parent) { : NativeWindow(options, parent) {
options.Get(options::kTitle, &title_); options.Get(options::kTitle, &title_);
bool menu_bar_autohide; if (bool val; options.Get(options::kAutoHideMenuBar, &val))
if (options.Get(options::kAutoHideMenuBar, &menu_bar_autohide)) root_view_.SetAutoHideMenuBar(val);
root_view_.SetAutoHideMenuBar(menu_bar_autohide);
#if BUILDFLAG(IS_WIN) #if BUILDFLAG(IS_WIN)
// On Windows we rely on the CanResize() to indicate whether window can be // On Windows we rely on the CanResize() to indicate whether window can be
@ -215,35 +214,20 @@ NativeWindowViews::NativeWindowViews(const gin_helper::Dictionary& options,
overlay_button_color_ = color_utils::GetSysSkColor(COLOR_BTNFACE); overlay_button_color_ = color_utils::GetSysSkColor(COLOR_BTNFACE);
overlay_symbol_color_ = color_utils::GetSysSkColor(COLOR_BTNTEXT); overlay_symbol_color_ = color_utils::GetSysSkColor(COLOR_BTNTEXT);
bool accent_color = true; if (std::string str; options.Get(options::kAccentColor, &str)) {
std::string accent_color_string; accent_color_ = ParseCSSColor(str);
if (options.Get(options::kAccentColor, &accent_color_string)) { } else if (bool flag; options.Get(options::kAccentColor, &flag)) {
accent_color_ = ParseCSSColor(accent_color_string); accent_color_ = flag;
} else if (options.Get(options::kAccentColor, &accent_color)) {
accent_color_ = accent_color;
} }
#endif #endif
v8::Local<v8::Value> titlebar_overlay; if (gin_helper::Dictionary od; options.Get(options::ktitleBarOverlay, &od)) {
if (options.Get(options::ktitleBarOverlay, &titlebar_overlay) && if (std::string val; od.Get(options::kOverlayButtonColor, &val)) {
titlebar_overlay->IsObject()) { bool success = content::ParseCssColorString(val, &overlay_button_color_);
auto titlebar_overlay_obj =
gin_helper::Dictionary::CreateEmpty(options.isolate());
options.Get(options::ktitleBarOverlay, &titlebar_overlay_obj);
std::string overlay_color_string;
if (titlebar_overlay_obj.Get(options::kOverlayButtonColor,
&overlay_color_string)) {
bool success = content::ParseCssColorString(overlay_color_string,
&overlay_button_color_);
DCHECK(success); DCHECK(success);
} }
if (std::string val; od.Get(options::kOverlaySymbolColor, &val)) {
std::string overlay_symbol_color_string; bool success = content::ParseCssColorString(val, &overlay_symbol_color_);
if (titlebar_overlay_obj.Get(options::kOverlaySymbolColor,
&overlay_symbol_color_string)) {
bool success = content::ParseCssColorString(overlay_symbol_color_string,
&overlay_symbol_color_);
DCHECK(success); DCHECK(success);
} }
} }
@ -289,8 +273,7 @@ NativeWindowViews::NativeWindowViews(const gin_helper::Dictionary& options,
if (IsTranslucent() && !has_frame()) if (IsTranslucent() && !has_frame())
params.shadow_type = InitParams::ShadowType::kNone; params.shadow_type = InitParams::ShadowType::kNone;
bool focusable; if (bool val; options.Get(options::kFocusable, &val) && !val)
if (options.Get(options::kFocusable, &focusable) && !focusable)
params.activatable = InitParams::Activatable::kNo; params.activatable = InitParams::Activatable::kNo;
#if BUILDFLAG(IS_WIN) #if BUILDFLAG(IS_WIN)
@ -413,11 +396,9 @@ NativeWindowViews::NativeWindowViews(const gin_helper::Dictionary& options,
// NOTE(@mlaurencin) Spec requirements can be found here: // NOTE(@mlaurencin) Spec requirements can be found here:
// https://developer.mozilla.org/en-US/docs/Web/API/Window/open#width // https://developer.mozilla.org/en-US/docs/Web/API/Window/open#width
int kMinSizeReqdBySpec = 100; constexpr int kMinSizeReqdBySpec = 100;
int inner_width = 0; const int inner_width = options.ValueOrDefault(options::kinnerWidth, 0);
int inner_height = 0; const int inner_height = options.ValueOrDefault(options::kinnerHeight, 0);
options.Get(options::kinnerWidth, &inner_width);
options.Get(options::kinnerHeight, &inner_height);
if (inner_width || inner_height) { if (inner_width || inner_height) {
use_content_size_ = true; use_content_size_ = true;
if (inner_width) if (inner_width)