Browse Source

refactor: make passing empty template no-op in setMenu (#23402)

Shelley Vohr 5 years ago
parent
commit
d5f9654c12
2 changed files with 15 additions and 3 deletions
  1. 9 3
      shell/browser/api/electron_api_top_level_window.cc
  2. 6 0
      spec-main/api-menu-spec.ts

+ 9 - 3
shell/browser/api/electron_api_top_level_window.cc

@@ -683,10 +683,16 @@ void TopLevelWindow::SetMenu(v8::Isolate* isolate, v8::Local<v8::Value> value) {
       gin::V8ToString(isolate, object->GetConstructorName()) == "Menu" &&
       gin::ConvertFromV8(isolate, value, &menu) && !menu.IsEmpty()) {
     menu_.Reset(isolate, menu.ToV8());
-    window_->SetMenu(menu->model());
+
+    // We only want to update the menu if the menu has a non-zero item count,
+    // or we risk crashes.
+    if (menu->model()->GetItemCount() == 0) {
+      RemoveMenu();
+    } else {
+      window_->SetMenu(menu->model());
+    }
   } else if (value->IsNull()) {
-    menu_.Reset();
-    window_->SetMenu(nullptr);
+    RemoveMenu();
   } else {
     isolate->ThrowException(
         v8::Exception::TypeError(gin::StringToV8(isolate, "Invalid Menu")));

+ 6 - 0
spec-main/api-menu-spec.ts

@@ -92,6 +92,12 @@ describe('Menu module', function () {
       }).to.throw(/Invalid template for MenuItem: must have at least one of label, role or type/)
     })
 
+    it('throws when an non-array is passed as a template', () => {
+      expect(() => {
+        Menu.buildFromTemplate('hello' as any);
+      }).to.throw(/Invalid template for Menu: Menu template must be an array/);
+    });
+
     describe('Menu sorting and building', () => {
       describe('sorts groups', () => {
         it('does a simple sort', () => {