refactor: use gin_helper::Dictionary::ValueOrDefault() (#47015)

refactor: use `gin_helper::Dictionary::ValueOrDefault()` (#46982)

* refactor: use ValueOrDefault() in electron_api_web_contents.cc

* refactor: use ValueOrDefault() in electron_api_url_loader.cc

* refactor: use ValueOrDefault() in electron_download_manager_delegate.cc

* refactor: use ValueOrDefault() in electron_touch_bar.mm

* refactor: use ValueOrDefault() in electron_url_loader_factory.cc

* refactor: use ValueOrDefault() in electron_browser_context.cc

* refactor: use ValueOrDefault() in electron_touch_bar.mm

* refactor: use ValueOrDefault() in blink_converter.cc

* feat: add ValueOrDefault() to PersistentDictionary

* empty commit

* refactor: use ValueOrDefault() in blink_converter.cc

* refactor: inline the rectangle base::Value::Dict

* refactor: remove has_scroll temporary

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Charles Kerr <charles@charleskerr.com>

shell/browser/api/electron_api_web_contents.cc

@@ -822,8 +822,7 @@ WebContents::WebContents(v8::Isolate* isolate,
   // Whether to enable DevTools.
   options.Get("devTools", &enable_devtools_);
 
-  bool initially_shown = true;
+  const bool initially_shown = options.ValueOrDefault(options::kShow, true);
-  options.Get(options::kShow, &initially_shown);
 
   // Obtain the session.
   std::string partition;
@@ -2332,9 +2331,7 @@ void WebContents::LoadURL(const GURL& url,
     params.load_type = content::NavigationController::LOAD_TYPE_DATA;
   }
 
-  bool reload_ignoring_cache = false;
+  if (options.ValueOrDefault("reloadIgnoringCache", false)) {
-  if (options.Get("reloadIgnoringCache", &reload_ignoring_cache) &&
-      reload_ignoring_cache) {
     params.reload_type = content::ReloadType::BYPASSING_CACHE;
   }
 
@@ -3011,13 +3008,10 @@ void WebContents::Print(gin::Arguments* args) {
   }
 
   // Set optional silent printing.
-  bool silent = false;
+  settings.Set("silent", options.ValueOrDefault("silent", false));
-  options.Get("silent", &silent);
-  settings.Set("silent", silent);
 
-  bool print_background = false;
+  settings.Set(printing::kSettingShouldPrintBackgrounds,
-  options.Get("printBackground", &print_background);
+               options.ValueOrDefault("printBackground", false));
-  settings.Set(printing::kSettingShouldPrintBackgrounds, print_background);
 
   // Set custom margin settings
   auto margins = gin_helper::Dictionary::CreateEmpty(args->isolate());
@@ -3028,20 +3022,16 @@ void WebContents::Print(gin::Arguments* args) {
     settings.Set(printing::kSettingMarginsType, static_cast<int>(margin_type));
 
     if (margin_type == printing::mojom::MarginType::kCustomMargins) {
-      base::Value::Dict custom_margins;
+      settings.Set(printing::kSettingMarginsCustom,
-      int top = 0;
+                   base::Value::Dict{}
-      margins.Get("top", &top);
+                       .Set(printing::kSettingMarginTop,
-      custom_margins.Set(printing::kSettingMarginTop, top);
+                            margins.ValueOrDefault("top", 0))
-      int bottom = 0;
+                       .Set(printing::kSettingMarginBottom,
-      margins.Get("bottom", &bottom);
+                            margins.ValueOrDefault("bottom", 0))
-      custom_margins.Set(printing::kSettingMarginBottom, bottom);
+                       .Set(printing::kSettingMarginLeft,
-      int left = 0;
+                            margins.ValueOrDefault("left", 0))
-      margins.Get("left", &left);
+                       .Set(printing::kSettingMarginRight,
-      custom_margins.Set(printing::kSettingMarginLeft, left);
+                            margins.ValueOrDefault("right", 0)));
-      int right = 0;
-      margins.Get("right", &right);
-      custom_margins.Set(printing::kSettingMarginRight, right);
-      settings.Set(printing::kSettingMarginsCustom, std::move(custom_margins));
     }
   } else {
     settings.Set(
@@ -3050,46 +3040,37 @@ void WebContents::Print(gin::Arguments* args) {
   }
 
   // Set whether to print color or greyscale
-  bool print_color = true;
+  settings.Set(printing::kSettingColor,
-  options.Get("color", &print_color);
+               static_cast<int>(options.ValueOrDefault("color", true)
-  auto const color_model = print_color ? printing::mojom::ColorModel::kColor
+                                    ? printing::mojom::ColorModel::kColor
-                                       : printing::mojom::ColorModel::kGray;
+                                    : printing::mojom::ColorModel::kGray));
-  settings.Set(printing::kSettingColor, static_cast<int>(color_model));
 
   // Is the orientation landscape or portrait.
-  bool landscape = false;
+  settings.Set(printing::kSettingLandscape,
-  options.Get("landscape", &landscape);
+               options.ValueOrDefault("landscape", false));
-  settings.Set(printing::kSettingLandscape, landscape);
 
   // We set the default to the system's default printer and only update
   // if at the Chromium level if the user overrides.
   // Printer device name as opened by the OS.
-  std::u16string device_name;
+  const auto device_name =
-  options.Get("deviceName", &device_name);
+      options.ValueOrDefault("deviceName", std::u16string{});
 
-  int scale_factor = 100;
+  settings.Set(printing::kSettingScaleFactor,
-  options.Get("scaleFactor", &scale_factor);
+               options.ValueOrDefault("scaleFactor", 100));
-  settings.Set(printing::kSettingScaleFactor, scale_factor);
 
-  int pages_per_sheet = 1;
+  settings.Set(printing::kSettingPagesPerSheet,
-  options.Get("pagesPerSheet", &pages_per_sheet);
+               options.ValueOrDefault("pagesPerSheet", 1));
-  settings.Set(printing::kSettingPagesPerSheet, pages_per_sheet);
 
   // True if the user wants to print with collate.
-  bool collate = true;
+  settings.Set(printing::kSettingCollate,
-  options.Get("collate", &collate);
+               options.ValueOrDefault("collate", true));
-  settings.Set(printing::kSettingCollate, collate);
 
   // The number of individual copies to print
-  int copies = 1;
+  settings.Set(printing::kSettingCopies, options.ValueOrDefault("copies", 1));
-  options.Get("copies", &copies);
-  settings.Set(printing::kSettingCopies, copies);
 
   // Strings to be printed as headers and footers if requested by the user.
-  std::string header;
+  const auto header = options.ValueOrDefault("header", std::string{});
-  options.Get("header", &header);
+  const auto footer = options.ValueOrDefault("footer", std::string{});
-  std::string footer;
-  options.Get("footer", &footer);
 
   if (!(header.empty() && footer.empty())) {
     settings.Set(printing::kSettingHeaderFooterEnabled, true);
@@ -3128,9 +3109,8 @@ void WebContents::Print(gin::Arguments* args) {
   }
 
   // Duplex type user wants to use.
-  printing::mojom::DuplexMode duplex_mode =
+  const auto duplex_mode = options.ValueOrDefault(
-      printing::mojom::DuplexMode::kSimplex;
+      "duplexMode", printing::mojom::DuplexMode::kSimplex);
-  options.Get("duplexMode", &duplex_mode);
   settings.Set(printing::kSettingDuplexMode, static_cast<int>(duplex_mode));
 
   base::Value::Dict media_size;
@@ -3150,14 +3130,11 @@ void WebContents::Print(gin::Arguments* args) {
   }
 
   // Set custom dots per inch (dpi)
-  gin_helper::Dictionary dpi_settings;
+  if (gin_helper::Dictionary dpi; options.Get("dpi", &dpi)) {
-  if (options.Get("dpi", &dpi_settings)) {
+    settings.Set(printing::kSettingDpiHorizontal,
-    int horizontal = 72;
+                 dpi.ValueOrDefault("horizontal", 72));
-    dpi_settings.Get("horizontal", &horizontal);
+    settings.Set(printing::kSettingDpiVertical,
-    settings.Set(printing::kSettingDpiHorizontal, horizontal);
+                 dpi.ValueOrDefault("vertical", 72));
-    int vertical = 72;
-    dpi_settings.Get("vertical", &vertical);
-    settings.Set(printing::kSettingDpiVertical, vertical);
   }
 
   print_task_runner_->PostTaskAndReplyWithResult(

shell/browser/electron_browser_context.cc

@@ -764,9 +764,9 @@ void ElectronBrowserContext::DisplayMediaDeviceChosen(
           GetAudioDesktopMediaId(request.requested_audio_device_ids));
       devices.audio_device = audio_device;
     } else if (result_dict.Get("audio", &rfh)) {
-      bool enable_local_echo = false;
+      const bool enable_local_echo =
-      result_dict.Get("enableLocalEcho", &enable_local_echo);
+          result_dict.ValueOrDefault("enableLocalEcho", false);
-      bool disable_local_echo = !enable_local_echo;
+      const bool disable_local_echo = !enable_local_echo;
       auto* web_contents = content::WebContents::FromRenderFrameHost(rfh);
       blink::MediaStreamDevice audio_device(
           request.audio_type,

shell/browser/electron_download_manager_delegate.cc

@@ -294,8 +294,7 @@ void ElectronDownloadManagerDelegate::OnDownloadSaveDialogDone(
   if (!item)
     return;
 
-  bool canceled = true;
+  const bool canceled = result.ValueOrDefault("canceled", true);
-  result.Get("canceled", &canceled);
 
   base::FilePath path;
 

shell/browser/net/electron_url_loader_factory.cc

@@ -125,8 +125,8 @@ network::mojom::URLResponseHeadPtr ToResponseHead(
     return head;
   }
 
-  int status_code = net::HTTP_OK;
+  const int status_code =
-  dict.Get("statusCode", &status_code);
+      dict.ValueOrDefault("statusCode", static_cast<int>(net::HTTP_OK));
   head->headers = base::MakeRefCounted<net::HttpResponseHeaders>(
       absl::StrFormat("HTTP/1.1 %d %s", status_code,
                       net::GetHttpReasonPhrase(

shell/browser/ui/cocoa/electron_touch_bar.mm

@@ -398,8 +398,7 @@ static NSString* const ImageScrubberItemIdentifier = @"scrubber.image.item";
     }
   }
 
-  bool enabled = true;
+  const bool enabled = settings.ValueOrDefault("enabled", true);
-  settings.Get("enabled", &enabled);
   [button setEnabled:enabled];
 }
 
@@ -501,16 +500,9 @@ static NSString* const ImageScrubberItemIdentifier = @"scrubber.image.item";
   settings.Get("label", &label);
   item.label = base::SysUTF8ToNSString(label);
 
-  int maxValue = 100;
+  item.slider.minValue = settings.ValueOrDefault("minValue", 0);
-  int minValue = 0;
+  item.slider.maxValue = settings.ValueOrDefault("maxValue", 100);
-  int value = 50;
+  item.slider.doubleValue = settings.ValueOrDefault("value", 50);
-  settings.Get("minValue", &minValue);
-  settings.Get("maxValue", &maxValue);
-  settings.Get("value", &value);
-
-  item.slider.minValue = minValue;
-  item.slider.maxValue = maxValue;
-  item.slider.doubleValue = value;
 }
 
 - (NSTouchBarItem*)makePopoverForID:(NSString*)id
@@ -540,9 +532,7 @@ static NSString* const ImageScrubberItemIdentifier = @"scrubber.image.item";
     item.collapsedRepresentationImage = image.AsNSImage();
   }
 
-  bool showCloseButton = true;
+  item.showsCloseButton = settings.ValueOrDefault("showCloseButton", true);
-  settings.Get("showCloseButton", &showCloseButton);
-  item.showsCloseButton = showCloseButton;
 
   v8::Isolate* isolate = electron::JavascriptEnvironment::GetIsolate();
   v8::HandleScope handle_scope(isolate);
@@ -670,8 +660,7 @@ static NSString* const ImageScrubberItemIdentifier = @"scrubber.image.item";
   for (size_t i = 0; i < segments.size(); ++i) {
     std::string label;
     gfx::Image image;
-    bool enabled = true;
+    const bool enabled = segments[i].ValueOrDefault("enabled", true);
-    segments[i].Get("enabled", &enabled);
     if (segments[i].Get("label", &label)) {
       [control setLabel:base::SysUTF8ToNSString(label) forSegment:i];
     } else {
@@ -686,8 +675,7 @@ static NSString* const ImageScrubberItemIdentifier = @"scrubber.image.item";
     [control setEnabled:enabled forSegment:i];
   }
 
-  int selectedIndex = 0;
+  const int selectedIndex = settings.ValueOrDefault("selectedIndex", 0);
-  settings.Get("selectedIndex", &selectedIndex);
   if (selectedIndex >= 0 && selectedIndex < control.segmentCount)
     control.selectedSegment = selectedIndex;
 }
@@ -726,8 +714,8 @@ static NSString* const ImageScrubberItemIdentifier = @"scrubber.image.item";
           withSettings:(const gin_helper::PersistentDictionary&)settings {
   NSScrubber* scrubber = item.view;
 
-  bool showsArrowButtons = false;
+  const bool showsArrowButtons =
-  settings.Get("showArrowButtons", &showsArrowButtons);
+      settings.ValueOrDefault("showArrowButtons", false);
   // The scrubber will crash if the user tries to scroll
   // and there are no items.
   if ([self numberOfItemsForScrubber:scrubber] > 0)
@@ -766,9 +754,7 @@ static NSString* const ImageScrubberItemIdentifier = @"scrubber.image.item";
     scrubber.mode = NSScrubberModeFree;
   }
 
-  bool continuous = true;
+  scrubber.continuous = settings.ValueOrDefault("continuous", true);
-  settings.Get("continuous", &continuous);
-  scrubber.continuous = continuous;
 
   [scrubber reloadData];
 }

shell/common/api/electron_api_url_loader.cc

@@ -615,9 +615,8 @@ gin::Handle<SimpleURLLoaderWrapper> SimpleURLLoaderWrapper::Create(
     }
   }
 
-  blink::mojom::FetchCacheMode cache_mode =
+  const auto cache_mode =
-      blink::mojom::FetchCacheMode::kDefault;
+      opts.ValueOrDefault("cache", blink::mojom::FetchCacheMode::kDefault);
-  opts.Get("cache", &cache_mode);
   switch (cache_mode) {
     case blink::mojom::FetchCacheMode::kNoStore:
       request->load_flags |= net::LOAD_DISABLE_CACHE;
@@ -645,8 +644,8 @@ gin::Handle<SimpleURLLoaderWrapper> SimpleURLLoaderWrapper::Create(
       break;
   }
 
-  bool use_session_cookies = false;
+  const bool use_session_cookies =
-  opts.Get("useSessionCookies", &use_session_cookies);
+      opts.ValueOrDefault("useSessionCookies", false);
   int options = network::mojom::kURLLoadOptionSniffMimeType;
   if (!credentials_specified && !use_session_cookies) {
     // This is the default case, as well as the case when credentials is not
@@ -656,9 +655,7 @@ gin::Handle<SimpleURLLoaderWrapper> SimpleURLLoaderWrapper::Create(
     options |= network::mojom::kURLLoadOptionBlockAllCookies;
   }
 
-  bool bypass_custom_protocol_handlers = false;
+  if (opts.ValueOrDefault("bypassCustomProtocolHandlers", false))
-  opts.Get("bypassCustomProtocolHandlers", &bypass_custom_protocol_handlers);
-  if (bypass_custom_protocol_handlers)
     options |= kBypassCustomProtocolHandlers;
 
   v8::Local<v8::Value> body;

shell/common/gin_converters/blink_converter.cc

@@ -369,10 +369,8 @@ bool Converter<blink::WebMouseEvent>::FromV8(v8::Isolate* isolate,
   if (!dict.Get("button", &out->button))
     out->button = blink::WebMouseEvent::Button::kLeft;
 
-  float global_x = 0.f;
+  const float global_x = dict.ValueOrDefault("globalX", 0.F);
-  float global_y = 0.f;
+  const float global_y = dict.ValueOrDefault("globalY", 0.F);
-  dict.Get("globalX", &global_x);
-  dict.Get("globalY", &global_y);
   out->SetPositionInScreen(global_x, global_y);
 
   dict.Get("movementX", &out->movement_x);
@@ -397,23 +395,19 @@ bool Converter<blink::WebMouseWheelEvent>::FromV8(
   dict.Get("accelerationRatioX", &out->acceleration_ratio_x);
   dict.Get("accelerationRatioY", &out->acceleration_ratio_y);
 
-  bool has_precise_scrolling_deltas = false;
+  const bool precise = dict.ValueOrDefault("hasPreciseScrollingDeltas", false);
-  dict.Get("hasPreciseScrollingDeltas", &has_precise_scrolling_deltas);
+  out->delta_units = precise ? ui::ScrollGranularity::kScrollByPrecisePixel
-  if (has_precise_scrolling_deltas) {
+                             : ui::ScrollGranularity::kScrollByPixel;
-    out->delta_units = ui::ScrollGranularity::kScrollByPrecisePixel;
-  } else {
-    out->delta_units = ui::ScrollGranularity::kScrollByPixel;
-  }
 
 #if defined(USE_AURA)
   // Matches the behavior of ui/events/blink/web_input_event_traits.cc:
-  bool can_scroll = true;
+  if (!dict.ValueOrDefault("canScroll", true)) {
-  if (dict.Get("canScroll", &can_scroll) && !can_scroll) {
     out->delta_units = ui::ScrollGranularity::kScrollByPage;
     out->SetModifiers(out->GetModifiers() &
                       ~blink::WebInputEvent::Modifiers::kControlKey);
   }
 #endif
+
   return true;
 }
 

shell/common/gin_helper/persistent_dictionary.h

@@ -41,6 +41,15 @@ class PersistentDictionary {
            gin::ConvertFromV8(isolate_, value, out);
   }
 
+  // Convenience function for using a default value if the
+  // specified key isn't present in the dictionary.
+  template <typename T>
+  T ValueOrDefault(const std::string_view key, T default_value) const {
+    if (auto value = T{}; Get(key, &value))
+      return value;
+    return default_value;
+  }
+
  private:
   raw_ptr<v8::Isolate> isolate_ = nullptr;
   v8::Global<v8::Object> handle_;