BrowserWindowCocoa::Show should activate the window

BrowserWindowCocoa::Show should activate the window

When a new tab/window is opened on Mac, the window is currently brought
to the front relative to other Chrome windows, but not relative to other
apps. This often goes unnoticed, since Chrome is often already the
active app, but it's particularly noticeable when clicking on a push
notification opens a new tab/window, as this window will remain
underneath other apps if Chrome wasn't already in the foreground.

This patch makes Show always Activate the window as well.

BUG=470830
TEST=manual, follow steps in https://crbug.com/470830

Committed: https://crrev.com/b13b6d7b2c7ad67c1b1937897fe3c26527eeb943
Cr-Commit-Position: refs/heads/master@{#332615}

[johnme, 2015-06-02 19:04:46]

johnme@chromium.org changed reviewers:
+ avi@chromium.org

[johnme, 2015-06-02 19:04:47]

Hey Avi, this patch seems to be a relatively easy fix for
https://crbug.com/470830 on Mac. PTAL, and if it looks good do you think it
might be safe enough to merge to m44?

[Avi (use Gerrit), 2015-06-02 19:21:37]

On 2015/06/02 19:04:47, johnme wrote:
> Hey Avi, this patch seems to be a relatively easy fix for
> https://crbug.com/470830 on Mac. PTAL, and if it looks good do you think it
> might be safe enough to merge to m44?

That's a pretty simple fix. Is forcing the window to the front always the right
thing to do for Show()?

[johnme, 2015-06-03 13:34:50]

On 2015/06/02 19:21:37, Avi wrote:
> On 2015/06/02 19:04:47, johnme wrote:
> > Hey Avi, this patch seems to be a relatively easy fix for
> > https://crbug.com/470830 on Mac. PTAL, and if it looks good do you think it
> > might be safe enough to merge to m44?
> 
> That's a pretty simple fix. Is forcing the window to the front always the
right
> thing to do for Show()?

How should I go about answering this? Show() already called
makeKeyAndOrderFront, so forcing the window in front of other Chrome windows is
not a change in behavior. However it's hard to know if every code path expects
Show() to steal focus from other apps.

What's the general philosophy around stealing focus on OS X? I noticed that
(unlike in Ubuntu Unity), on OS X any website can steal focus by calling
alert(); you can test this by running:

    setTimeout(function() { alert('hi'); } , 3000);

in DevTools and switching to another app. Is that intentional, or is that a bug
that we should fix once I finish plumbing user_gesture in
https://codereview.chromium.org/1158523002/ ?

In case it helps, I've uploaded the call hierarchy of ui::BaseWindow::Show
(which BrowserWindowCocoa overrides) to http://output.jsbin.com/daqiqi/quiet
(you can expand at least two levels; it's based on Chrome OS Grok, so not fully
accurate).

[Avi (use Gerrit), 2015-06-03 14:40:00]

Re our discussion, this patch LGTM for 44 to solve the immediate problem.

You're already plumbing the user_gesture bool in other patches, so that looks
like a better solution for the future.

As for alert, in chat you suggested that we might want to back off to using
-requestUserAttention:, which also SGTM.

[johnme, 2015-06-03 14:47:01]

The CQ bit was checked by johnme@chromium.org

[commit-bot: I haz the power, 2015-06-03 14:48:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1166643003/1

[commit-bot: I haz the power, 2015-06-03 15:39:11]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-06-03 15:40:04]

Patchset 1 (id:??) landed as
https://crrev.com/b13b6d7b2c7ad67c1b1937897fe3c26527eeb943
Cr-Commit-Position: refs/heads/master@{#332615}

[Andre, 2015-09-18 21:53:40]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/1359483002/ by andresantoso@chromium.org.

The reason for reverting is: Caused regression: http://crbug.com/500038, and
http://crbug.com/496786.