Make launching apps from shelf more intuitive

Make launching apps from shelf more intuitive

In multi-display environment, each display has one copy of shelf.
Launching an app from a shelf will open a tab in the browser that has
focus, no matter which display the browser is in. This is annoying when
the focus and the shelf being clicked are in different displays.

A more intuitive way is to open the app in the same display as the shelf
being clicked.

The details of behavior changes in this CL:
- opens a new tab in the uppermost browser in the display where the shelf
is clicked if the display has one or more browsers.

- opens a new browser along with a new tab in the display where the shelf
is clicked if the display has no browser.

- takes the same behavior when the app is lauched from launcher context
menu of the shelf.

BUG=586091
TEST=browser_tests
--gtest_filter=ShelfAppBrowserTest.LaunchAppFromDisplayWithoutFocus*

Review-Url: https://codereview.chromium.org/2894743002
Cr-Commit-Position: refs/heads/master@{#475152}
Committed: https://chromium.googlesource.com/chromium/src/+/07d1c26bd8f426f51c051891d7487404f1be4115

[weidongg, 2017-05-19 01:37:40]

Description was changed from

==========
Make launching apps from shelf more intuitive

In multi-display environment, each display has one copy of shelf.
Launching an app from a shelf will open a tab in the browser that has
focus, no matter which display the browser is in. This is annoying when
the focus and the shelf being clicked are in different displays.

A more intuitive way is to open the app in the same display as the shelf
being clicked.

The details of behavior changes in this CL:
- opens a new tab in the uppermost browser in the display where the shelf
is clicked if the display has one or more browsers.

- opens a new browser along with a new tab in the display where the shelf
is clicked if the display has no browser.

- takes the same behavior when the app is lauched from launcher context
menu.

BUG=586091
TEST=browser_tests
--gtest_filter=ShelfAppBrowserTest.LaunchAppFromDisplayWithoutFocus*
==========

to

==========
Make launching apps from shelf more intuitive

In multi-display environment, each display has one copy of shelf.
Launching an app from a shelf will open a tab in the browser that has
focus, no matter which display the browser is in. This is annoying when
the focus and the shelf being clicked are in different displays.

A more intuitive way is to open the app in the same display as the shelf
being clicked.

The details of behavior changes in this CL:
- opens a new tab in the uppermost browser in the display where the shelf
is clicked if the display has one or more browsers.

- opens a new browser along with a new tab in the display where the shelf
is clicked if the display has no browser.

- takes the same behavior when the app is lauched from launcher context
menu of the shelf.

BUG=586091
TEST=browser_tests
--gtest_filter=ShelfAppBrowserTest.LaunchAppFromDisplayWithoutFocus*
==========

[weidongg, 2017-05-19 01:42:22]

weidongg@chromium.org changed reviewers:
+ benwells@chromium.org, skuhne@chromium.org, stevenjb@chromium.org

[weidongg, 2017-05-19 01:42:22]

skuhne@chromium.org: Please review changes in 

stevenjb@chromium.org: Please review changes in

[weidongg, 2017-05-19 01:44:27]

skuhne@chromium.org: Please review changes in 
ash/shelf/*

stevenjb@chromium.org: Please review changes in
chrome/browser/ui/ash/*

benwells@chromium.org: Please review changes in
chrome/browser/ui/extensions/app_*

[benwells, 2017-05-19 03:11:46]

Have you looked at fixing this at a deeper level? That is, not in ash but in
chrome generally?

Seems like displays as a concept are on all OSes, as is app launching. From
looking at launching apps on Linux it appears to be broken in different annoying
ways (it always opens a new browser), so would be nice to fix this everywhere
and make it consistent.

[weidongg, 2017-05-19 04:09:23]

On 2017/05/19 03:11:46, benwells wrote:
> Have you looked at fixing this at a deeper level? That is, not in ash but in
> chrome generally?
> 
> Seems like displays as a concept are on all OSes, as is app launching. From
> looking at launching apps on Linux it appears to be broken in different
annoying
> ways (it always opens a new browser), so would be nice to fix this everywhere
> and make it consistent.

I found OpenApplicationTab() in application_launch.cc, which seems to be right
place to start. Could I add a member variable |display_id| in the
AppLaunchParams?
And whenever an app is launched, the |display_id| is set to the id of the
display
where the app should be launched. Also I could add a new function in
browser_finder.cc
called FindTabbedBrowserInDisplay() and supply the |display_id| to find the
uppermost
browser in the display identified by the |display_id|.

Do you think this is a better idea?

[benwells, 2017-05-19 04:33:11]

On 2017/05/19 04:09:23, weidongg wrote:
> On 2017/05/19 03:11:46, benwells wrote:
> > Have you looked at fixing this at a deeper level? That is, not in ash but in
> > chrome generally?
> > 
> > Seems like displays as a concept are on all OSes, as is app launching. From
> > looking at launching apps on Linux it appears to be broken in different
> annoying
> > ways (it always opens a new browser), so would be nice to fix this
everywhere
> > and make it consistent.
> 
> I found OpenApplicationTab() in application_launch.cc, which seems to be right
> place to start. Could I add a member variable |display_id| in the
> AppLaunchParams?
> And whenever an app is launched, the |display_id| is set to the id of the
> display
> where the app should be launched. Also I could add a new function in
> browser_finder.cc
> called FindTabbedBrowserInDisplay() and supply the |display_id| to find the
> uppermost
> browser in the display identified by the |display_id|.
> 
> Do you think this is a better idea?

Yep I think that would be cleaner.

[stevenjb, 2017-05-19 16:46:14]

On 2017/05/19 04:33:11, benwells wrote:
> On 2017/05/19 04:09:23, weidongg wrote:
> > On 2017/05/19 03:11:46, benwells wrote:
> > > Have you looked at fixing this at a deeper level? That is, not in ash but
in
> > > chrome generally?
> > > 
> > > Seems like displays as a concept are on all OSes, as is app launching.
From
> > > looking at launching apps on Linux it appears to be broken in different
> > annoying
> > > ways (it always opens a new browser), so would be nice to fix this
> everywhere
> > > and make it consistent.
> > 
> > I found OpenApplicationTab() in application_launch.cc, which seems to be
right
> > place to start. Could I add a member variable |display_id| in the
> > AppLaunchParams?
> > And whenever an app is launched, the |display_id| is set to the id of the
> > display
> > where the app should be launched. Also I could add a new function in
> > browser_finder.cc
> > called FindTabbedBrowserInDisplay() and supply the |display_id| to find the
> > uppermost
> > browser in the display identified by the |display_id|.
> > 
> > Do you think this is a better idea?
> 
> Yep I think that would be cleaner.

I haven't looked at the code changes yet, but reading Ben's question and your
response, that sounds like the right approach to me as well. Even if we
initially ignore |display_id| on some platforms, having it available is not a
bad thing.

[weidongg, 2017-05-19 17:09:02]

On 2017/05/19 16:46:14, stevenjb wrote:
> On 2017/05/19 04:33:11, benwells wrote:
> > On 2017/05/19 04:09:23, weidongg wrote:
> > > On 2017/05/19 03:11:46, benwells wrote:
> > > > Have you looked at fixing this at a deeper level? That is, not in ash
but
> in
> > > > chrome generally?
> > > > 
> > > > Seems like displays as a concept are on all OSes, as is app launching.
> From
> > > > looking at launching apps on Linux it appears to be broken in different
> > > annoying
> > > > ways (it always opens a new browser), so would be nice to fix this
> > everywhere
> > > > and make it consistent.
> > > 
> > > I found OpenApplicationTab() in application_launch.cc, which seems to be
> right
> > > place to start. Could I add a member variable |display_id| in the
> > > AppLaunchParams?
> > > And whenever an app is launched, the |display_id| is set to the id of the
> > > display
> > > where the app should be launched. Also I could add a new function in
> > > browser_finder.cc
> > > called FindTabbedBrowserInDisplay() and supply the |display_id| to find
the
> > > uppermost
> > > browser in the display identified by the |display_id|.
> > > 
> > > Do you think this is a better idea?
> > 
> > Yep I think that would be cleaner.
> 
> I haven't looked at the code changes yet, but reading Ben's question and your
> response, that sounds like the right approach to me as well. Even if we
> initially ignore |display_id| on some platforms, having it available is not a
> bad thing.

Thanks, I will upload a new patch set to implement the new approach.

[weidongg, 2017-05-22 04:05:06]

The CQ bit was checked by weidongg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-22 04:05:21]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-22 04:21:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-22 04:21:28]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Mr4D (OOO till 08-26), 2017-05-22 15:02:28]

lgtm

https://codereview.chromium.org/2894743002/diff/20001/ash/shelf/wm_shelf.cc
File ash/shelf/wm_shelf.cc (right):

https://codereview.chromium.org/2894743002/diff/20001/ash/shelf/wm_shelf.cc#n...
ash/shelf/wm_shelf.cc:302: void WmShelf::ActivateShelfItemInDisplay(int
item_index, int64_t display_id) {
Maybe .. onDisplay which sounds much better here.

https://codereview.chromium.org/2894743002/diff/20001/ash/shelf/wm_shelf.h
File ash/shelf/wm_shelf.h (right):

https://codereview.chromium.org/2894743002/diff/20001/ash/shelf/wm_shelf.h#ne...
ash/shelf/wm_shelf.h:129: // in the display identified by |display_id|.
on the display ...?

[Mr4D (OOO till 08-26), 2017-05-22 15:02:58]

lgtm for the ash part that is. Please check comments.

[stevenjb, 2017-05-22 17:55:29]

https://codereview.chromium.org/2894743002/diff/20001/ash/shelf/wm_shelf.h
File ash/shelf/wm_shelf.h (right):

https://codereview.chromium.org/2894743002/diff/20001/ash/shelf/wm_shelf.h#ne...
ash/shelf/wm_shelf.h:126: static void ActivateShelfItem(int item_index);
Is this still called? Maybe note where/why we do not always call 
ActivateShelfItemInDisplay? (Or if there are not a lot of examples, maybe just
remove this and pass display::kInvalidDisplayId in those instances).

https://codereview.chromium.org/2894743002/diff/20001/chrome/browser/ui/ash/a...
File chrome/browser/ui/ash/app_list/app_list_controller_ash.cc (right):

https://codereview.chromium.org/2894743002/diff/20001/chrome/browser/ui/ash/a...
chrome/browser/ui/ash/app_list/app_list_controller_ash.cc:118: event_flags,
NULL);
Use nullptr and add a comment describing what the null argument is.

https://codereview.chromium.org/2894743002/diff/20001/chrome/browser/ui/ash/l...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc (right):

https://codereview.chromium.org/2894743002/diff/20001/chrome/browser/ui/ash/l...
chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc:415:
LaunchApp(shelf_id, source, event_flags, NULL);
nullptr /* root_window */

https://codereview.chromium.org/2894743002/diff/20001/chrome/browser/ui/ash/l...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc
(right):

https://codereview.chromium.org/2894743002/diff/20001/chrome/browser/ui/ash/l...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc:1047:
ash::LAUNCH_FROM_UNKNOWN, 0, NULL);
Use nullptr throughout.

https://codereview.chromium.org/2894743002/diff/20001/chrome/browser/ui/ash/l...
File chrome/browser/ui/ash/launcher/launcher_controller_helper.cc (right):

https://codereview.chromium.org/2894743002/diff/20001/chrome/browser/ui/ash/l...
chrome/browser/ui/ash/launcher/launcher_controller_helper.cc:219:
ash::LAUNCH_FROM_UNKNOWN, ui::EF_NONE, NULL);
nullptr

https://codereview.chromium.org/2894743002/diff/20001/chrome/browser/ui/brows...
File chrome/browser/ui/browser_finder.cc (right):

https://codereview.chromium.org/2894743002/diff/20001/chrome/browser/ui/brows...
chrome/browser/ui/browser_finder.cc:35: const int kMatchRootWindow = 1 << 3;
Align '= 1 << 3' with lines above.

https://codereview.chromium.org/2894743002/diff/20001/chrome/browser/ui/brows...
chrome/browser/ui/browser_finder.cc:120: const aura::Window* root_window) {
|match_root_window| seems redundant, can we just test 'if (root_window)' ?

https://codereview.chromium.org/2894743002/diff/20001/chrome/browser/ui/brows...
chrome/browser/ui/browser_finder.cc:166: false, NULL);
nullptr throughout

https://codereview.chromium.org/2894743002/diff/20001/chrome/browser/ui/exten...
File chrome/browser/ui/extensions/app_launch_params.h (right):

https://codereview.chromium.org/2894743002/diff/20001/chrome/browser/ui/exten...
chrome/browser/ui/extensions/app_launch_params.h:33: const aura::Window*
root_window = NULL);
nullptr

[benwells, 2017-05-23 03:49:46]

There seem to be a lot of failures related to the change.

[weidongg, 2017-05-23 16:37:46]

Sorry for the big change: In patch set 3, I replace the root window in
app_launch_params.h with display id, as using root window causes too many
platform-related problems in the trybots.

https://codereview.chromium.org/2894743002/diff/20001/ash/shelf/wm_shelf.cc
File ash/shelf/wm_shelf.cc (right):

https://codereview.chromium.org/2894743002/diff/20001/ash/shelf/wm_shelf.cc#n...
ash/shelf/wm_shelf.cc:302: void WmShelf::ActivateShelfItemInDisplay(int
item_index, int64_t display_id) {
On 2017/05/22 15:02:28, Mr4D wrote:
> Maybe .. onDisplay which sounds much better here.

Done.

https://codereview.chromium.org/2894743002/diff/20001/ash/shelf/wm_shelf.h
File ash/shelf/wm_shelf.h (right):

https://codereview.chromium.org/2894743002/diff/20001/ash/shelf/wm_shelf.h#ne...
ash/shelf/wm_shelf.h:126: static void ActivateShelfItem(int item_index);
On 2017/05/22 17:55:28, stevenjb wrote:
> Is this still called? Maybe note where/why we do not always call 
> ActivateShelfItemInDisplay? (Or if there are not a lot of examples, maybe just
> remove this and pass display::kInvalidDisplayId in those instances).

There are 40 occurrences of this function, so should I do the refactor like
removing ActivateShelfItemInDisplay and adding display_id parameter to
ActivateShelfItem?

https://codereview.chromium.org/2894743002/diff/20001/ash/shelf/wm_shelf.h#ne...
ash/shelf/wm_shelf.h:129: // in the display identified by |display_id|.
On 2017/05/22 15:02:28, Mr4D wrote:
> on the display ...?
> 

Done.

https://codereview.chromium.org/2894743002/diff/20001/chrome/browser/ui/brows...
File chrome/browser/ui/browser_finder.cc (right):

https://codereview.chromium.org/2894743002/diff/20001/chrome/browser/ui/brows...
chrome/browser/ui/browser_finder.cc:120: const aura::Window* root_window) {
On 2017/05/22 17:55:29, stevenjb wrote:
> |match_root_window| seems redundant, can we just test 'if (root_window)' ?

Done

[weidongg, 2017-05-23 16:38:06]

The CQ bit was checked by weidongg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-23 16:38:39]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-23 18:31:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-23 18:31:34]

Dry run: This issue passed the CQ dry run.

[stevenjb, 2017-05-23 21:56:43]

lgtm

https://codereview.chromium.org/2894743002/diff/40001/chrome/browser/ui/brows...
File chrome/browser/ui/browser_finder.cc (right):

https://codereview.chromium.org/2894743002/diff/40001/chrome/browser/ui/brows...
chrome/browser/ui/browser_finder.cc:99: .id() == display_id;
{}

[benwells, 2017-05-24 04:05:34]

c/b/e lgtm. You'll need an owner for browser_finder...

https://codereview.chromium.org/2894743002/diff/40001/chrome/browser/ui/exten...
File chrome/browser/ui/extensions/app_launch_params.h (right):

https://codereview.chromium.org/2894743002/diff/40001/chrome/browser/ui/exten...
chrome/browser/ui/extensions/app_launch_params.h:78: // -1 means the display
does not exist or is not set.
Nit: Use the constant display::kInvalidDisplayId here instead of -1

[weidongg, 2017-05-24 17:49:39]

weidongg@chromium.org changed reviewers:
+ pkasting@chromium.org

[weidongg, 2017-05-24 17:49:40]

Thanks for the review.

pkasting@chromium.org: Please review changes in 
chrome/browser/ui/browser_finder.*

https://codereview.chromium.org/2894743002/diff/40001/chrome/browser/ui/brows...
File chrome/browser/ui/browser_finder.cc (right):

https://codereview.chromium.org/2894743002/diff/40001/chrome/browser/ui/brows...
chrome/browser/ui/browser_finder.cc:99: .id() == display_id;
On 2017/05/23 21:56:43, stevenjb wrote:
> {}

Done.

https://codereview.chromium.org/2894743002/diff/40001/chrome/browser/ui/exten...
File chrome/browser/ui/extensions/app_launch_params.h (right):

https://codereview.chromium.org/2894743002/diff/40001/chrome/browser/ui/exten...
chrome/browser/ui/extensions/app_launch_params.h:78: // -1 means the display
does not exist or is not set.
On 2017/05/24 04:05:34, benwells wrote:
> Nit: Use the constant display::kInvalidDisplayId here instead of -1

Done.

[Peter Kasting, 2017-05-25 07:25:08]

Note: I'm behind on codereviews but will try and get to this today (Thursday May
25), after I get some sleep.

[Peter Kasting, 2017-05-26 01:25:04]

LGTM

https://codereview.chromium.org/2894743002/diff/60001/chrome/browser/ui/brows...
File chrome/browser/ui/browser_finder.cc (right):

https://codereview.chromium.org/2894743002/diff/60001/chrome/browser/ui/brows...
chrome/browser/ui/browser_finder.cc:183: display::kInvalidDisplayId);
Nit: We could reduce the number of callers restating kInvalidDisplayId values
they don't care about by using more default args above.

https://codereview.chromium.org/2894743002/diff/60001/chrome/browser/ui/brows...
File chrome/browser/ui/browser_finder.h (right):

https://codereview.chromium.org/2894743002/diff/60001/chrome/browser/ui/brows...
chrome/browser/ui/browser_finder.h:35: // display identified by |display_id|.
Maybe this should just be added as an optional (defaulted to kInvalidDisplayId)
third arg to FindTabbedBrowser()?

[weidongg, 2017-05-26 02:49:48]

Thanks for the review.

https://codereview.chromium.org/2894743002/diff/60001/chrome/browser/ui/brows...
File chrome/browser/ui/browser_finder.cc (right):

https://codereview.chromium.org/2894743002/diff/60001/chrome/browser/ui/brows...
chrome/browser/ui/browser_finder.cc:183: display::kInvalidDisplayId);
On 2017/05/26 01:25:03, Peter Kasting wrote:
> Nit: We could reduce the number of callers restating kInvalidDisplayId values
> they don't care about by using more default args above.

Done.

https://codereview.chromium.org/2894743002/diff/60001/chrome/browser/ui/brows...
File chrome/browser/ui/browser_finder.h (right):

https://codereview.chromium.org/2894743002/diff/60001/chrome/browser/ui/brows...
chrome/browser/ui/browser_finder.h:35: // display identified by |display_id|.
On 2017/05/26 01:25:03, Peter Kasting wrote:
> Maybe this should just be added as an optional (defaulted to
kInvalidDisplayId)
> third arg to FindTabbedBrowser()?

Yes, that's better. Done.

[weidongg, 2017-05-26 17:35:04]

The CQ bit was checked by weidongg@chromium.org

[weidongg, 2017-05-26 17:35:04]

The patchset sent to the CQ was uploaded after l-g-t-m from skuhne@chromium.org,
stevenjb@chromium.org, benwells@chromium.org, pkasting@chromium.org
Link to the patchset: https://codereview.chromium.org/2894743002/#ps100001
(title: "Apply fix to patch set 4: Use default value for |display_id|")

[commit-bot: I haz the power, 2017-05-26 17:35:39]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-26 17:38:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-26 17:38:57]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[weidongg, 2017-05-26 19:07:31]

The CQ bit was checked by weidongg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-26 19:08:12]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-26 19:16:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-26 19:16:18]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[weidongg, 2017-05-26 20:52:51]

The CQ bit was checked by weidongg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-26 20:53:32]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-26 21:53:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-26 21:53:06]

Dry run: This issue passed the CQ dry run.

[weidongg, 2017-05-26 21:53:53]

The CQ bit was checked by weidongg@chromium.org

[weidongg, 2017-05-26 21:53:54]

The patchset sent to the CQ was uploaded after l-g-t-m from
stevenjb@chromium.org, benwells@chromium.org, pkasting@chromium.org,
skuhne@chromium.org
Link to the patchset: https://codereview.chromium.org/2894743002/#ps140001
(title: "Affected by new revision, gclient sync and re-upload.")

[commit-bot: I haz the power, 2017-05-26 21:54:29]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-26 22:00:04]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1495835633904760,
"parent_rev": "ea289a66e6444098032ce8f5b3c0dc0da1923c25", "commit_rev":
"07d1c26bd8f426f51c051891d7487404f1be4115"}

[commit-bot: I haz the power, 2017-05-26 22:00:17]

Description was changed from

==========
Make launching apps from shelf more intuitive

In multi-display environment, each display has one copy of shelf.
Launching an app from a shelf will open a tab in the browser that has
focus, no matter which display the browser is in. This is annoying when
the focus and the shelf being clicked are in different displays.

A more intuitive way is to open the app in the same display as the shelf
being clicked.

The details of behavior changes in this CL:
- opens a new tab in the uppermost browser in the display where the shelf
is clicked if the display has one or more browsers.

- opens a new browser along with a new tab in the display where the shelf
is clicked if the display has no browser.

- takes the same behavior when the app is lauched from launcher context
menu of the shelf.

BUG=586091
TEST=browser_tests
--gtest_filter=ShelfAppBrowserTest.LaunchAppFromDisplayWithoutFocus*
==========

to

==========
Make launching apps from shelf more intuitive

In multi-display environment, each display has one copy of shelf.
Launching an app from a shelf will open a tab in the browser that has
focus, no matter which display the browser is in. This is annoying when
the focus and the shelf being clicked are in different displays.

A more intuitive way is to open the app in the same display as the shelf
being clicked.

The details of behavior changes in this CL:
- opens a new tab in the uppermost browser in the display where the shelf
is clicked if the display has one or more browsers.

- opens a new browser along with a new tab in the display where the shelf
is clicked if the display has no browser.

- takes the same behavior when the app is lauched from launcher context
menu of the shelf.

BUG=586091
TEST=browser_tests
--gtest_filter=ShelfAppBrowserTest.LaunchAppFromDisplayWithoutFocus*

Review-Url: https://codereview.chromium.org/2894743002
Cr-Commit-Position: refs/heads/master@{#475152}
Committed:
https://chromium.googlesource.com/chromium/src/+/07d1c26bd8f426f51c051891d748...
==========

[commit-bot: I haz the power, 2017-05-26 22:00:19]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/07d1c26bd8f426f51c051891d748...