[Mac] Remove BrowserCommandExecutor protocol.

chrome/browser/ui/cocoa/chrome_event_processing_window.mm

Index: chrome/browser/ui/cocoa/chrome_event_processing_window.mm
diff --git a/chrome/browser/ui/cocoa/chrome_event_processing_window.mm b/chrome/browser/ui/cocoa/chrome_event_processing_window.mm
index a493d84f39544721d760bf2574103854f17db62c..4b0400ddcd1e3c620d73fe8176358241f7c914a5 100644
--- a/chrome/browser/ui/cocoa/chrome_event_processing_window.mm
+++ b/chrome/browser/ui/cocoa/chrome_event_processing_window.mm
@@ -6,7 +6,8 @@
 
 #include "base/logging.h"
 #include "chrome/browser/global_keyboard_shortcuts_mac.h"
-#import "chrome/browser/ui/cocoa/browser_command_executor.h"
+#include "chrome/browser/ui/browser_commands.h"
+#include "chrome/browser/ui/browser_finder.h"
 #import "chrome/browser/ui/cocoa/browser_window_controller_private.h"
 #import "chrome/browser/ui/cocoa/tabs/tab_strip_controller.h"
 #import "content/public/browser/render_widget_host_view_mac_base.h"
@@ -15,13 +16,14 @@ typedef int (*KeyToCommandMapper)(bool, bool, bool, bool, int, unichar);
 
 @interface ChromeEventProcessingWindow ()
 // Duplicate the given key event, but changing the associated window.
-- (NSEvent*)keyEventForWindow:(NSWindow*)window fromKeyEvent:(NSEvent*)event;
++ (NSEvent*)keyEventForWindow:(NSWindow*)window fromKeyEvent:(NSEvent*)event;

[tapted, 2015/07/27 04:58:57]

Should this be a function just declared in an anonymous namespace?

Or - probably better.. should it just be something like

- (NSEvent*)adoptOtherWindowKeyEvent:(NSEvent*)event;

since I think the only call is

[self keyEventForWindow:self ..

[jackhou1, 2015/07/28 05:19:45]

Made it anonymous.

This will be factored out into ChromeCommandDispatchDelegate, so the window will
be passed in.

 @end
 
 @implementation ChromeEventProcessingWindow
 
-- (BOOL)handleExtraKeyboardShortcut:(NSEvent*)event fromTable:
-    (KeyToCommandMapper)commandForKeyboardShortcut {
+- (BOOL)handleExtraKeyboardShortcut:(NSEvent*)event

[tapted, 2015/07/27 04:58:57]

Can this just be a function in an anonymous namespace now? (but also see below)

[jackhou1, 2015/07/28 05:19:45]

Done.

+                          fromTable:
+                              (KeyToCommandMapper)commandForKeyboardShortcut {
   // Extract info from |event|.
   NSUInteger modifers = [event modifierFlags];
   const bool cmdKey = modifers & NSCommandKeyMask;
@@ -34,15 +36,16 @@ typedef int (*KeyToCommandMapper)(bool, bool, bool, bool, int, unichar);
   int cmdNum = commandForKeyboardShortcut(cmdKey, shiftKey, cntrlKey, optKey,
       keyCode, keyChar);
 
-  if (cmdNum != -1) {
-    id executor = [self delegate];
-    // A bit of sanity.
-    DCHECK([executor conformsToProtocol:@protocol(BrowserCommandExecutor)]);
-    DCHECK([executor respondsToSelector:@selector(executeCommand:)]);
-    [executor executeCommand:cmdNum];
-    return YES;
-  }
-  return NO;
+  if (cmdNum == -1)
+    return NO;
+
+  // Only handle event if this is a browser window.
+  Browser* browser = chrome::FindBrowserWithWindow(event.window);

[tapted, 2015/07/27 04:58:57]

Pretty sure that it's not guaranteed that event.window == self here. (e.g.
[PermissionBubbleWindow performKeyEquivalent:] passes [self parentWindow])

Probably keeping this an instance method, and passing `self` instead of
event.window would be closer to the prior code.

[jackhou1, 2015/07/28 05:19:45]

This is also code that we want to factor out, so changed to passing in the
desired window.

+  if (!browser)
+    return NO;
+
+  chrome::ExecuteCommand(browser, cmdNum);
+  return YES;
 }
 
 - (BOOL)handleExtraWindowKeyboardShortcut:(NSEvent*)event {
@@ -86,11 +89,6 @@ typedef int (*KeyToCommandMapper)(bool, bool, bool, bool, int, unichar);
   if ([r conformsToProtocol:@protocol(RenderWidgetHostViewMacBase)])
     return [r performKeyEquivalent:event];
 
-  // If the delegate does not implement the BrowserCommandExecutor protocol,
-  // then we don't need to handle browser specific shortcut keys.
-  if (![[self delegate] conformsToProtocol:@protocol(BrowserCommandExecutor)])
-    return [super performKeyEquivalent:event];
-
   // Handle per-window shortcuts like cmd-1, but do not handle browser-level
   // shortcuts like cmd-left (else, cmd-left would do history navigation even
   // if e.g. the Omnibox has focus).
@@ -122,7 +120,8 @@ typedef int (*KeyToCommandMapper)(bool, bool, bool, bool, int, unichar);
   // rare case, we synthesize a new key event so that its associate window
   // (number) is our own.
   if ([event window] != self)

[tapted, 2015/07/27 04:58:57]

nit: needs curlies (if it stays this way)

[jackhou1, 2015/07/28 05:19:45]

Done.

-    event = [self keyEventForWindow:self fromKeyEvent:event];
+    event =
+        [ChromeEventProcessingWindow keyEventForWindow:self fromKeyEvent:event];
 
   // Redispatch the event.
   eventHandled_ = YES;
@@ -143,14 +142,16 @@ typedef int (*KeyToCommandMapper)(bool, bool, bool, bool, int, unichar);
     eventHandled_ = NO;
 }
 
-- (NSEvent*)keyEventForWindow:(NSWindow*)window fromKeyEvent:(NSEvent*)event {
+// Private interface.
+
++ (NSEvent*)keyEventForWindow:(NSWindow*)window fromKeyEvent:(NSEvent*)event {
   NSEventType eventType = [event type];
 
   // Convert the event's location from the original window's coordinates into
   // our own.
   NSPoint eventLoc = [event locationInWindow];
   eventLoc = [[event window] convertBaseToScreen:eventLoc];
-  eventLoc = [self convertScreenToBase:eventLoc];
+  eventLoc = [window convertScreenToBase:eventLoc];
 
   // Various things *only* apply to key down/up.
   BOOL eventIsARepeat = NO;