Allow Widget to pass accelerator handling to its parent.

Allow Widget to pass accelerator handling to its parent.

This enables passing of both ui::Accelerator and Mac-specific commands
from Widget to its parent regulated by Widget::InitParams::
pass_accelerator_to_parent.

Regular views accelerator handling involves overriding
FocusManagerDelegate which forwards all requests to parent's
FocusManager.

Mac-specific handling is done by overriding
UserInterfaceItemCommandHandler which forwards all request to parent's
NSWindow (if it supports CommandDispatchingWindow protocol).

BUG=603881

[alshabalin, 2016-12-27 14:43:01]

Description was changed from

==========
Allow Widget to pass accelerator handling to its parent.

This enables passing of both ui::Accelerator and Mac-specific commands
from Widget to its parent regulated by Widget::InitParams::
pass_accelerator_to_parent.

Regular views accelerator handling involves overriding
FocusManagerDelegate which forwards all requests to parent's
FocusManager.

Mac-specific handling is done by overriding
UserInterfaceItemCommandHandler which forwards all request to parent's
NSWindow (if it supports CommandDispatchingWindow protocol).

BUG=603881
==========

to

==========
Allow Widget to pass accelerator handling to its parent.

This enables passing of both ui::Accelerator and Mac-specific commands
from Widget to its parent regulated by Widget::InitParams::
pass_accelerator_to_parent.

Regular views accelerator handling involves overriding
FocusManagerDelegate which forwards all requests to parent's
FocusManager.

Mac-specific handling is done by overriding
UserInterfaceItemCommandHandler which forwards all request to parent's
NSWindow (if it supports CommandDispatchingWindow protocol).

BUG=603881
==========

[alshabalin, 2016-12-27 14:43:01]

alshabalin@yandex-team.ru changed reviewers:
+ msw@chromium.org, tapted@chromium.org

[alshabalin, 2016-12-27 14:50:40]

I've enabled accelerator passing for all BubbleDialogDelegate bubbles but it's
probably better to start doing so one by one (WebsiteSettingsPopupView, for
example, does not expect that currently active tab might change while it's
open).

Also the tests are slapped on, I can try to improve them, if you agree with the
overall approach.

[msw, 2016-12-28 19:21:12]

msw@chromium.org changed reviewers:
+ sky@chromium.org

[msw, 2016-12-28 19:21:12]

msw->sky; I'm not sure that the overall approach here is appropriate.

[tapted, 2017-01-02 23:11:49]

We need to have a discussion on a bug about this.

http://crbug.com/603881 is assigned to me, but it's a bug for the Cocoa browser
and Cocoa dialogs. It's not related to toolkit-views (it is effectively
obsolete). It can't be used for this.

For MacViews there is http://crbug.com/605374 but I closed it as WontFix. The
reason is that the closest Cocoa UI element to a Chrome "bubble" is NSPopover.
Cmd+w while an NSPopover is visible closes the popover only - it shouldn't be
passed to the parent window.

However, for other keyboard shortcuts, we might want to do something else - we
need a new bug that talks about the use-cases we need to solve. Maybe your team
has encountered a specific behaviour that feels broken? (can you file a new
bug?)

But, generally speaking, I don't think this approach is correct.

One issue is that the "parent" window is not always the browser window. For
web-modal sheets we have to make an intermediate window.

The neatest approach is probably just to allow child windows to take a reference
to the parent window's CommandHandler (see below), then modify the
implementation of BrowserWindowCommandHandler so that the lines like

Browser* browser = chrome::FindBrowserWithWindow(window);

are able to support walking up the hierarchy of parent windows until am actual
browser window is found.

https://codereview.chromium.org/2604793002/diff/1/ui/views/widget/native_widg...
File ui/views/widget/native_widget_mac.mm (right):

https://codereview.chromium.org/2604793002/diff/1/ui/views/widget/native_widg...
ui/views/widget/native_widget_mac.mm:126: base::scoped_nsobject<NSWindow>
window([ns_window retain]);
if you are just after the concrete type, you can make this a
base::scoped_nsobject<NativeWidgetMacNSWindow>

https://codereview.chromium.org/2604793002/diff/1/ui/views/widget/native_widg...
ui/views/widget/native_widget_mac.mm:129: if (params.parent &&
params.pass_accelerator_to_parent) {
I don't think we need params.pass_accelerator_to_parent . Here I'd try something
like

if (![window commandHandler] &&
    [[params.parent window]
conformsToProtocol:@protcol(CommandDispatchingWindow)) {
  id<CommandDispatchingWindow> command_window = [params.parent window];
  [window setCommandHandler:[command_window commandHandler]];
}

[alshabalin, 2017-01-09 14:04:23]

On 2017/01/02 23:11:49, tapted wrote:
> We need to have a discussion on a bug about this.
> 
> http://crbug.com/603881 is assigned to me, but it's a bug for the Cocoa
browser
> and Cocoa dialogs. It's not related to toolkit-views (it is effectively
> obsolete). It can't be used for this.
> 
> For MacViews there is http://crbug.com/605374 but I closed it as WontFix. The
> reason is that the closest Cocoa UI element to a Chrome "bubble" is NSPopover.
> Cmd+w while an NSPopover is visible closes the popover only - it shouldn't be
> passed to the parent window.
> 
> However, for other keyboard shortcuts, we might want to do something else - we
> need a new bug that talks about the use-cases we need to solve. Maybe your
team
> has encountered a specific behaviour that feels broken? (can you file a new
> bug?)

Opened a bug: crbug.com/679339
In short: inability to use browser shortcuts, when bubble gets shown without you
asking
for it can get a little annoying.

[tapted, 2017-02-01 23:56:27]

On 2017/01/09 14:04:23, alshabalin wrote:
> On 2017/01/02 23:11:49, tapted wrote:
> > We need to have a discussion on a bug about this.
> > 
> > http://crbug.com/603881 is assigned to me, but it's a bug for the Cocoa
> browser
> > and Cocoa dialogs. It's not related to toolkit-views (it is effectively
> > obsolete). It can't be used for this.
> > 
> > For MacViews there is http://crbug.com/605374 but I closed it as WontFix.
The
> > reason is that the closest Cocoa UI element to a Chrome "bubble" is
NSPopover.
> > Cmd+w while an NSPopover is visible closes the popover only - it shouldn't
be
> > passed to the parent window.
> > 
> > However, for other keyboard shortcuts, we might want to do something else -
we
> > need a new bug that talks about the use-cases we need to solve. Maybe your
> team
> > has encountered a specific behaviour that feels broken? (can you file a new
> > bug?)
> 
> Opened a bug: crbug.com/679339
> In short: inability to use browser shortcuts, when bubble gets shown without
you
> asking
> for it can get a little annoying.

I have a CL in https://codereview.chromium.org/2666523002/ which should address
http://crbug.com/603881 in a way that shares the Mac-specific command
dispatching logic used for mainMenu integration with toolkit-views bubbles. Not
much changes under ui/views -- native_widget_mac_nswindow gets a bit simpler
since some logic moves into ui/base/cocoa/command_dispatcher.mm