Repository: electron/electron
File: shell/browser/api/electron_api_menu_mac.mm (Objective-C++)

Code before:
  if (modifier_mask & NSEventModifierFlagCommand)
    modifiers |= ui::EF_COMMAND_DOWN;
  return ui::Accelerator(code, modifiers);
}

}  // namespace

namespace electron::api {

MenuMac::MenuMac(gin::Arguments* args) : Menu(args) {}

MenuMac::~MenuMac() = default;

void MenuMac::PopupAt(BaseWindow* window,
                      std::optional<WebFrameMain*> frame,
                      int x,
                      int y,
                      int positioning_item,
                      ui::mojom::MenuSourceType source_type,
                      base::OnceClosure callback) {
  NativeWindow* native_window = window->window();
  if (!native_window)
    return;

  base::WeakPtr<WebFrameMain> weak_frame;
  if (frame) {
    weak_frame = frame->GetWeakPtr();
  }

  // Make sure the Menu object would not be garbage-collected until the callback
  // has run.
  base::OnceClosure callback_with_ref = BindSelfToClosure(std::move(callback));

  auto popup = base::BindOnce(&MenuMac::PopupOnUI, weak_factory_.GetWeakPtr(),
                              native_window->GetWeakPtr(), weak_frame,
                              window->weak_map_id(), x, y, positioning_item,
                              std::move(callback_with_ref));
  base::SequencedTaskRunner::GetCurrentDefault()->PostTask(FROM_HERE,
                                                           std::move(popup));
}

v8::Local<v8::Value> Menu::GetUserAcceleratorAt(int command_id) const {
  v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
  if (![NSMenuItem usesUserKeyEquivalents])
    return v8::Null(isolate);

  auto controller = [[ElectronMenuController alloc] initWithModel:model()
                                            useDefaultAccelerator:NO];

  int command_index = GetIndexOfCommandId(command_id);
  if (command_index == -1)

Review comment (bug):
This is slightly brittle, it's possible to give the optional a `nullptr` s.t. the optional is nonempty with a null pointer and this paragraph results in a nullptr dereference

Maybe replace `std::optional<WebFrameMain*>` with just a `WebFrameMain*` that may be nullptr?

Code after:
  if (modifier_mask & NSEventModifierFlagCommand)
    modifiers |= ui::EF_COMMAND_DOWN;
  return ui::Accelerator(code, modifiers);
}

}  // namespace

namespace electron::api {

MenuMac::MenuMac(gin::Arguments* args) : Menu(args) {}

MenuMac::~MenuMac() = default;

void MenuMac::PopupAt(BaseWindow* window,
                      std::optional<WebFrameMain*> frame,
                      int x,
                      int y,
                      int positioning_item,
                      ui::mojom::MenuSourceType source_type,
                      base::OnceClosure callback) {
  NativeWindow* native_window = window->window();
  if (!native_window)
    return;

  base::WeakPtr<WebFrameMain> weak_frame;
  if (frame && frame.value()) {
    weak_frame = frame.value()->GetWeakPtr();
  }

  // Make sure the Menu object would not be garbage-collected until the callback
  // has run.
  base::OnceClosure callback_with_ref = BindSelfToClosure(std::move(callback));

  auto popup = base::BindOnce(&MenuMac::PopupOnUI, weak_factory_.GetWeakPtr(),
                              native_window->GetWeakPtr(), weak_frame,
                              window->weak_map_id(), x, y, positioning_item,
                              std::move(callback_with_ref));
  base::SequencedTaskRunner::GetCurrentDefault()->PostTask(FROM_HERE,
                                                           std::move(popup));
}

v8::Local<v8::Value> Menu::GetUserAcceleratorAt(int command_id) const {
  v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
  if (![NSMenuItem usesUserKeyEquivalents])
    return v8::Null(isolate);

  auto controller = [[ElectronMenuController alloc] initWithModel:model()
                                            useDefaultAccelerator:NO];

  int command_index = GetIndexOfCommandId(command_id);
  if (command_index == -1)