Repository: electron/electron
File: shell/browser/ui/cocoa/electron_menu_controller.mm (Objective-C++)

Code before:
}

- (BOOL)isMenuOpen {
  return isMenuOpen_;
}

- (void)menuWillOpen:(NSMenu*)menu {
  isMenuOpen_ = YES;
  [self refreshMenuTree:menu];
  if (model_)
    model_->MenuWillShow();
}

- (void)menuDidClose:(NSMenu*)menu {
  // If the menu is already closed, do nothing.
  if (!isMenuOpen_)
    return;

  bool has_close_cb = !closeCallback.is_null();

  // There are two scenarios where we should emit menu-did-close:
  // 1. It's a popup and the top level menu is closed.
  // 2. It's an application menu, and the current menu's supermenu
  //    is the top-level menu.
  if (menu != menu_) {
    if (has_close_cb || menu.supermenu != menu_)
      return;
  }

  isMenuOpen_ = NO;
  if (model_)
    model_->MenuWillClose();
  // Post async task so that itemSelected runs before the close callback
  // deletes the controller from the map which deallocates it.
  if (has_close_cb) {
    content::GetUIThreadTaskRunner({})->PostTask(FROM_HERE,
                                                 std::move(closeCallback));
  }
}

// NSSharingServiceDelegate

- (NSWindow*)sharingService:(NSSharingService*)service
    sourceWindowForShareItems:(NSArray*)items
          sharingContentScope:(NSSharingContentScope*)scope {
  // Return the current active window.
  const auto& list = electron::WindowList::GetWindows();
  for (electron::NativeWindow* window : list) {
    if (window->IsFocused())
      return window->GetNativeWindow().GetNativeNSWindow();
  }

Review comment (suggestion):
I'm not sure I understand using `has_close_cb` as a proxy for "is this a popup".

I _think_ it's because right now the [only user](https://github.com/electron/electron/blob/e601d05cbd28a2d84e0c62bd07ea67e70564a0bd/shell/browser/api/electron_api_menu_mac.mm#L160) for the close callback is a currently popup.

1. If that's the logic, we should rename the `closeCallback` property as `popupCloseCallback` to make sure future maintainers don't break this code someday by accidentally adding a close callback to a non-popup

2. If I'm misunderstanding it, then it probably needs to be spelled out more in the code comment above

Code after:
}

- (BOOL)isMenuOpen {
  return isMenuOpen_;
}

- (void)menuWillOpen:(NSMenu*)menu {
  isMenuOpen_ = YES;
  [self refreshMenuTree:menu];
  if (model_)
    model_->MenuWillShow();
}

- (void)menuDidClose:(NSMenu*)menu {
  // If the menu is already closed, do nothing.
  if (!isMenuOpen_)
    return;

  bool has_close_cb = !popupCloseCallback.is_null();

  // There are two scenarios where we should emit menu-did-close:
  // 1. It's a popup and the top level menu is closed.
  // 2. It's an application menu, and the current menu's supermenu
  //    is the top-level menu.
  if (menu != menu_) {
    if (has_close_cb || menu.supermenu != menu_)
      return;
  }

  isMenuOpen_ = NO;
  if (model_)
    model_->MenuWillClose();
  // Post async task so that itemSelected runs before the close callback
  // deletes the controller from the map which deallocates it.
  if (has_close_cb) {
    content::GetUIThreadTaskRunner({})->PostTask(FROM_HERE,
                                                 std::move(popupCloseCallback));
  }
}

// NSSharingServiceDelegate

- (NSWindow*)sharingService:(NSSharingService*)service
    sourceWindowForShareItems:(NSArray*)items
          sharingContentScope:(NSSharingContentScope*)scope {
  // Return the current active window.
  const auto& list = electron::WindowList::GetWindows();
  for (electron::NativeWindow* window : list) {
    if (window->IsFocused())
      return window->GetNativeWindow().GetNativeNSWindow();
  }