fix: about panel crash (#37373)

* fix: about panel is a base::Value::Dict * nix this test for a diff PR
electron · Feb 28, 2023 · 2e03bdb · 2e03bdb
1 parent 3a5ae28
commit 2e03bdb
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 29 deletions.
diff --git a/shell/browser/browser.h b/shell/browser/browser.h
@@ -363,7 +363,7 @@ class Browser : public WindowListObserver {
   base::Time last_dock_show_;
 #endif
 
-  base::Value about_panel_options_;
+  base::Value::Dict about_panel_options_;
 
 #if BUILDFLAG(IS_WIN)
   void UpdateBadgeContents(HWND hwnd,

diff --git a/shell/browser/browser_linux.cc b/shell/browser/browser_linux.cc
@@ -162,31 +162,25 @@ bool Browser::IsEmojiPanelSupported() {
 void Browser::ShowAboutPanel() {
   const auto& opts = about_panel_options_;
 
-  if (!opts.is_dict()) {
-    LOG(WARNING) << "Called showAboutPanel(), but didn't use "
-                    "setAboutPanelSettings() first";
-    return;
-  }
-
   GtkWidget* dialogWidget = gtk_about_dialog_new();
   GtkAboutDialog* dialog = GTK_ABOUT_DIALOG(dialogWidget);
 
   const std::string* str;
-  const base::Value* val;
+  const base::Value::List* list;
 
-  if ((str = opts.FindStringKey("applicationName"))) {
+  if ((str = opts.FindString("applicationName"))) {
     gtk_about_dialog_set_program_name(dialog, str->c_str());
   }
-  if ((str = opts.FindStringKey("applicationVersion"))) {
+  if ((str = opts.FindString("applicationVersion"))) {
     gtk_about_dialog_set_version(dialog, str->c_str());
   }
-  if ((str = opts.FindStringKey("copyright"))) {
+  if ((str = opts.FindString("copyright"))) {
     gtk_about_dialog_set_copyright(dialog, str->c_str());
   }
-  if ((str = opts.FindStringKey("website"))) {
+  if ((str = opts.FindString("website"))) {
     gtk_about_dialog_set_website(dialog, str->c_str());
   }
-  if ((str = opts.FindStringKey("iconPath"))) {
+  if ((str = opts.FindString("iconPath"))) {
     GError* error = nullptr;
     constexpr int width = 64;   // width of about panel icon in pixels
     constexpr int height = 64;  // height of about panel icon in pixels
@@ -203,9 +197,9 @@ void Browser::ShowAboutPanel() {
     }
   }
 
-  if ((val = opts.FindListKey("authors"))) {
+  if ((list = opts.FindList("authors"))) {
     std::vector<const char*> cstrs;
-    for (const auto& authorVal : val->GetList()) {
+    for (const auto& authorVal : *list) {
       if (authorVal.is_string()) {
         cstrs.push_back(authorVal.GetString().c_str());
       }
@@ -223,7 +217,7 @@ void Browser::ShowAboutPanel() {
 }
 
 void Browser::SetAboutPanelOptions(base::Value::Dict options) {
-  about_panel_options_ = base::Value(std::move(options));
+  about_panel_options_ = std::move(options);
 }
 
 }  // namespace electron
diff --git a/shell/browser/browser_mac.mm b/shell/browser/browser_mac.mm
@@ -513,8 +513,7 @@ bool CheckLoginItemStatus(bool* is_hidden) {
 }
 
 void Browser::ShowAboutPanel() {
-  NSDictionary* options =
-      DictionaryValueToNSDictionary(about_panel_options_.GetDict());
+  NSDictionary* options = DictionaryValueToNSDictionary(about_panel_options_);
 
   // Credits must be a NSAttributedString instead of NSString
   NSString* credits = (NSString*)options[@"Credits"];
@@ -537,13 +536,13 @@ bool CheckLoginItemStatus(bool* is_hidden) {
 }
 
 void Browser::SetAboutPanelOptions(base::Value::Dict options) {
-  about_panel_options_.GetDict().clear();
+  about_panel_options_.clear();
 
   for (const auto pair : options) {
     std::string key = pair.first;
     if (!key.empty() && pair.second.is_string()) {
       key[0] = base::ToUpperASCII(key[0]);
-      about_panel_options_.GetDict().Set(key, pair.second.Clone());
+      about_panel_options_.Set(key, pair.second.Clone());
     }
   }
 }

diff --git a/shell/browser/browser_win.cc b/shell/browser/browser_win.cc
@@ -734,30 +734,29 @@ void Browser::ShowEmojiPanel() {
 }
 
 void Browser::ShowAboutPanel() {
-  base::Value dict(base::Value::Type::DICTIONARY);
+  base::Value::Dict dict;
   std::string aboutMessage = "";
   gfx::ImageSkia image;
 
   // grab defaults from Windows .EXE file
   std::unique_ptr<FileVersionInfo> exe_info = FetchFileVersionInfo();
-  dict.SetStringKey("applicationName", exe_info->file_description());
-  dict.SetStringKey("applicationVersion", exe_info->product_version());
+  dict.Set("applicationName", exe_info->file_description());
+  dict.Set("applicationVersion", exe_info->product_version());
 
-  if (about_panel_options_.is_dict()) {
-    dict.MergeDictionary(&about_panel_options_);
-  }
+  // Merge user-provided options, overwriting any of the above
+  dict.Merge(about_panel_options_.Clone());
 
   std::vector<std::string> stringOptions = {
       "applicationName", "applicationVersion", "copyright", "credits"};
 
   const std::string* str;
   for (std::string opt : stringOptions) {
-    if ((str = dict.FindStringKey(opt))) {
+    if ((str = dict.FindString(opt))) {
       aboutMessage.append(*str).append("\r\n");
     }
   }
 
-  if ((str = dict.FindStringKey("iconPath"))) {
+  if ((str = dict.FindString("iconPath"))) {
     base::FilePath path = base::FilePath::FromUTF8Unsafe(*str);
     electron::util::PopulateImageSkiaRepsFromPath(&image, path);
   }
@@ -770,7 +769,7 @@ void Browser::ShowAboutPanel() {
 }
 
 void Browser::SetAboutPanelOptions(base::Value::Dict options) {
-  about_panel_options_ = base::Value(std::move(options));
+  about_panel_options_ = std::move(options);
 }
 
 }  // namespace electron
diff --git a/spec/api-app-spec.ts b/spec/api-app-spec.ts
@@ -1859,6 +1859,15 @@ describe('app module', () => {
       })).to.eventually.be.rejectedWith(/ERR_NAME_NOT_RESOLVED/);
     });
   });
+
+  describe('about panel', () => {
+    it('app.setAboutPanelOptions() does not crash', () => {
+      app.setAboutPanelOptions({
+        applicationName: 'electron!!',
+        version: '1.2.3'
+      });
+    });
+  });
 });
 
 describe('default behavior', () => {