refactor: return null when passing empty menu templates (#23403)

electron · May 5, 2020 · 2362fa0 · 2362fa0
1 parent 35a4db0
commit 2362fa0
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 3 deletions.
diff --git a/shell/browser/api/atom_api_top_level_window.cc b/shell/browser/api/atom_api_top_level_window.cc
@@ -661,10 +661,16 @@ void TopLevelWindow::SetMenu(v8::Isolate* isolate, v8::Local<v8::Value> value) {
       gin::V8ToString(isolate, object->GetConstructorName()) == "Menu" &&
       mate::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(mate::StringToV8(isolate, "Invalid Menu")));

diff --git a/spec-main/api-menu-spec.ts b/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', () => {