Merge ShelfItemDelegate::ItemSelected & LauncherItemDelegate::Activate.

Merge ShelfItemDelegate::ItemSelected & LauncherItemDelegate::Activate.

These functions have nearly identical purposes; merge them.
(app-list used Activate via ChromeLauncherControllerImpl::ActivateApp)

Split up the ui::Event argument into constituent pieces.
(this will help when converting the function to support mojo)

Move ash::LaunchSource (in src/chrome) -> ash::ShelfLaunchSource.
Move ash::ShelfItemDelegate::PerformedAction -> ash::ShelfAction.
(also update its kConstantValueNaming to ENUM_VALUE_NAMING)

Add ash::wm::MoveWindowToDisplay helper; minor refactor; add test.

BUG=557406
TEST=No behavior changes selecting/activating shelf or app-list items.
R=jamescook@chromium.org

Review-Url: https://codereview.chromium.org/2696073002
Cr-Commit-Position: refs/heads/master@{#451072}
Committed: https://chromium.googlesource.com/chromium/src/+/c30c27728fcf046088ca6c0da51bda06e3a5ce63

[msw, 2017-02-14 21:49:37]

Description was changed from

==========
Merge ShelfItemDelegate::ItemSelected & LauncherItemDelegate::Activate.

BUG=557406
==========

to

==========
Merge ShelfItemDelegate::ItemSelected & LauncherItemDelegate::Activate.

These functions have nearly identical purposes; merge them.
(app-list used Activate via ChromeLauncherControllerImpl::ActivateApp)

Split up the ui::Event argument into constituent pieces.
(this will help when converting the function to support mojo)

Move ash::LaunchSource (in src/chrome) -> ash::ShelfLaunchSource.
Move ash::ShelfItemDelegate::PerformedAction -> ash::ShelfAction.
TODO: Update kConstantValueNaming to ENUM_VALUE_NAMING.

Add ash::wm::MoveWindowToDisplay helper; refactor similar helper.

TODO(msw): BLOCKERS FROM LANDING!!!
TODO: Fix selecting Files from app-list w/ open files&chrome windows.
(if chrome is active, it doesn't switch to the Files window...)
TODO: Fix selecting chrome/files item w/ multiple open windows.
(activates a window *and* shows the application list)

BUG=557406
TEST=No behavior changes selecting/activating shelf or app-list items.
R=jamescook@chromium.org
==========

[msw, 2017-02-14 21:49:37]

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

[msw, 2017-02-14 21:54:12]

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

[commit-bot: I haz the power, 2017-02-14 21:55:20]

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-02-14 22:49:19]

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

[commit-bot: I haz the power, 2017-02-14 22:49:20]

Dry run: This issue passed the CQ dry run.

[msw, 2017-02-14 22:56:51]

Description was changed from

==========
Merge ShelfItemDelegate::ItemSelected & LauncherItemDelegate::Activate.

These functions have nearly identical purposes; merge them.
(app-list used Activate via ChromeLauncherControllerImpl::ActivateApp)

Split up the ui::Event argument into constituent pieces.
(this will help when converting the function to support mojo)

Move ash::LaunchSource (in src/chrome) -> ash::ShelfLaunchSource.
Move ash::ShelfItemDelegate::PerformedAction -> ash::ShelfAction.
TODO: Update kConstantValueNaming to ENUM_VALUE_NAMING.

Add ash::wm::MoveWindowToDisplay helper; refactor similar helper.

TODO(msw): BLOCKERS FROM LANDING!!!
TODO: Fix selecting Files from app-list w/ open files&chrome windows.
(if chrome is active, it doesn't switch to the Files window...)
TODO: Fix selecting chrome/files item w/ multiple open windows.
(activates a window *and* shows the application list)

BUG=557406
TEST=No behavior changes selecting/activating shelf or app-list items.
R=jamescook@chromium.org
==========

to

==========
Merge ShelfItemDelegate::ItemSelected & LauncherItemDelegate::Activate.

These functions have nearly identical purposes; merge them.
(app-list used Activate via ChromeLauncherControllerImpl::ActivateApp)

Split up the ui::Event argument into constituent pieces.
(this will help when converting the function to support mojo)

Move ash::LaunchSource (in src/chrome) -> ash::ShelfLaunchSource.
Move ash::ShelfItemDelegate::PerformedAction -> ash::ShelfAction.
TODO: Update kConstantValueNaming to ENUM_VALUE_NAMING.

Add ash::wm::MoveWindowToDisplay helper; refactor similar helper.

BUG=557406
TEST=No behavior changes selecting/activating shelf or app-list items.
R=jamescook@chromium.org
==========

[msw, 2017-02-14 22:59:51]

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

[msw, 2017-02-14 23:00:02]

Hey James, please take a look; thanks!

[commit-bot: I haz the power, 2017-02-14 23:00:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[James Cook, 2017-02-15 00:44:40]

I really like collapsing the functions! I'm concerned about the default values
for the parameters in the new combined function -- it confuses me that lots of
code passes the primary display id seemingly as a default value. If you need
defaults I would go with kInvalidDisplayId and let the underlying code do the
right thing when it isn't provided.

https://codereview.chromium.org/2696073002/diff/20001/ash/common/shelf/app_li...
File ash/common/shelf/app_list_shelf_item_delegate.cc (right):

https://codereview.chromium.org/2696073002/diff/20001/ash/common/shelf/app_li...
ash/common/shelf/app_list_shelf_item_delegate.cc:40: return
ash::kAppListMenuShown;
ash:: not needed?

Also, if you have to touch the majority of the lines that have kCamelCaseNames
is now a good time to switch to BETTER_NAMES and/or enum class?

https://codereview.chromium.org/2696073002/diff/20001/ash/common/shelf/shelf_...
File ash/common/shelf/shelf_item_delegate.h (right):

https://codereview.chromium.org/2696073002/diff/20001/ash/common/shelf/shelf_...
ash/common/shelf/shelf_item_delegate.h:21: // relevant display id, and the
source of the selection are sometimes known.
uh, "sometimes known"? Maybe tell the reader which fields are optional and which
aren't? And document what the default values should be?

https://codereview.chromium.org/2696073002/diff/20001/ash/common/shelf/shelf_...
ash/common/shelf/shelf_item_delegate.h:26: int64_t display_id,
You could also provide two versions, one of which does not take display_id, and
require that it handle that case correctly.

https://codereview.chromium.org/2696073002/diff/20001/ash/common/shelf/shelf_...
File ash/common/shelf/shelf_view.cc (right):

https://codereview.chromium.org/2696073002/diff/20001/ash/common/shelf/shelf_...
ash/common/shelf/shelf_view.cc:490: LAUNCH_FROM_UNKNOWN);
It's kind of weird that this is "LAUNCH_FROM_UNKNOWN" since you know everything
about the launch. Maybe the enum should pick up a value for FROM_SHELF, or maybe
UNKNOWN should be DEFAULT or NON_APP_LIST?

https://codereview.chromium.org/2696073002/diff/20001/ash/public/cpp/shelf_ty...
File ash/public/cpp/shelf_types.h (right):

https://codereview.chromium.org/2696073002/diff/20001/ash/public/cpp/shelf_ty...
ash/public/cpp/shelf_types.h:60: LAUNCH_FROM_UNKNOWN,
Can you document each of these while you're here?

https://codereview.chromium.org/2696073002/diff/20001/ash/wm/window_util.cc
File ash/wm/window_util.cc (right):

https://codereview.chromium.org/2696073002/diff/20001/ash/wm/window_util.cc#n...
ash/wm/window_util.cc:36: // Move the |window| to the target |root|, using the
corresponding container.
nit: document return value ("True if it actually moved"?)

https://codereview.chromium.org/2696073002/diff/20001/ash/wm/window_util.cc#n...
ash/wm/window_util.cc:85: bool MoveWindowToDisplay(aura::Window* window, int64_t
display_id) {
DCHECK(window) and that display_id is valid?

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

https://codereview.chromium.org/2696073002/diff/20001/chrome/browser/ui/ash/l...
chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.cc:120:
source, event_flags);
Is this change needed?

https://codereview.chromium.org/2696073002/diff/20001/chrome/browser/ui/ash/l...
File chrome/browser/ui/ash/launcher/app_window_launcher_item_controller.cc
(left):

https://codereview.chromium.org/2696073002/diff/20001/chrome/browser/ui/ash/l...
chrome/browser/ui/ash/launcher/app_window_launcher_item_controller.cc:79: }
I'm a little lost. Where did the functionality provided by this function end up?
Was it duplicated somewhere and isn't needed any more?

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

https://codereview.chromium.org/2696073002/diff/20001/chrome/browser/ui/ash/l...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl.cc:450: int64_t
display_id = display::Screen::GetScreen()->GetPrimaryDisplay().id();
Is it correct to assume the primary display here? Should this be passing an
invalid display ID to mean "I don't know"?

If it's OK here to assume the primary display I would comment why.

[commit-bot: I haz the power, 2017-02-15 02:18:50]

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

[commit-bot: I haz the power, 2017-02-15 02:18:51]

Dry run: This issue passed the CQ dry run.

[msw, 2017-02-15 19:58:18]

Description was changed from

==========
Merge ShelfItemDelegate::ItemSelected & LauncherItemDelegate::Activate.

These functions have nearly identical purposes; merge them.
(app-list used Activate via ChromeLauncherControllerImpl::ActivateApp)

Split up the ui::Event argument into constituent pieces.
(this will help when converting the function to support mojo)

Move ash::LaunchSource (in src/chrome) -> ash::ShelfLaunchSource.
Move ash::ShelfItemDelegate::PerformedAction -> ash::ShelfAction.
TODO: Update kConstantValueNaming to ENUM_VALUE_NAMING.

Add ash::wm::MoveWindowToDisplay helper; refactor similar helper.

BUG=557406
TEST=No behavior changes selecting/activating shelf or app-list items.
R=jamescook@chromium.org
==========

to

==========
Merge ShelfItemDelegate::ItemSelected & LauncherItemDelegate::Activate.

These functions have nearly identical purposes; merge them.
(app-list used Activate via ChromeLauncherControllerImpl::ActivateApp)

Split up the ui::Event argument into constituent pieces.
(this will help when converting the function to support mojo)

Move ash::LaunchSource (in src/chrome) -> ash::ShelfLaunchSource.
Move ash::ShelfItemDelegate::PerformedAction -> ash::ShelfAction.
(also update its kConstantValueNaming to ENUM_VALUE_NAMING)

Add ash::wm::MoveWindowToDisplay helper; minor refactor; add test.

BUG=557406
TEST=No behavior changes selecting/activating shelf or app-list items.
R=jamescook@chromium.org
==========

[msw, 2017-02-15 19:58:27]

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

[commit-bot: I haz the power, 2017-02-15 19:59:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[msw, 2017-02-15 19:59:18]

Please take another look, thanks! (I also added a test)

https://codereview.chromium.org/2696073002/diff/20001/ash/common/shelf/app_li...
File ash/common/shelf/app_list_shelf_item_delegate.cc (right):

https://codereview.chromium.org/2696073002/diff/20001/ash/common/shelf/app_li...
ash/common/shelf/app_list_shelf_item_delegate.cc:40: return
ash::kAppListMenuShown;
On 2017/02/15 00:44:39, James Cook wrote:
> ash:: not needed?
> 
> Also, if you have to touch the majority of the lines that have kCamelCaseNames
> is now a good time to switch to BETTER_NAMES and/or enum class?

Done.

https://codereview.chromium.org/2696073002/diff/20001/ash/common/shelf/shelf_...
File ash/common/shelf/shelf_item_delegate.h (right):

https://codereview.chromium.org/2696073002/diff/20001/ash/common/shelf/shelf_...
ash/common/shelf/shelf_item_delegate.h:21: // relevant display id, and the
source of the selection are sometimes known.
On 2017/02/15 00:44:39, James Cook wrote:
> uh, "sometimes known"? Maybe tell the reader which fields are optional and
which
> aren't? And document what the default values should be?

Heh, I didn't want to enumerate the various codepaths that call this function
and the different levels of context that they have. No fields are strictly
necessary, but when the information is available, it should be provided, since
some shelf item delegate implementations will vary the behavior based on the
arguments. I've revised the comment with suggested defaults.

https://codereview.chromium.org/2696073002/diff/20001/ash/common/shelf/shelf_...
ash/common/shelf/shelf_item_delegate.h:26: int64_t display_id,
On 2017/02/15 00:44:39, James Cook wrote:
> You could also provide two versions, one of which does not take display_id,
and
> require that it handle that case correctly.

I'd rather not overload this function; I'm hoping to merge it with
GetAppMenuItems and convert it to mojo asap.

https://codereview.chromium.org/2696073002/diff/20001/ash/common/shelf/shelf_...
File ash/common/shelf/shelf_view.cc (right):

https://codereview.chromium.org/2696073002/diff/20001/ash/common/shelf/shelf_...
ash/common/shelf/shelf_view.cc:490: LAUNCH_FROM_UNKNOWN);
On 2017/02/15 00:44:40, James Cook wrote:
> It's kind of weird that this is "LAUNCH_FROM_UNKNOWN" since you know
everything
> about the launch. Maybe the enum should pick up a value for FROM_SHELF, or
maybe
> UNKNOWN should be DEFAULT or NON_APP_LIST?

It's mostly used for logging at the moment, I'd rather not add new values. Even
if I did add a new value, I wouldn't be adding any new behavior making use of
that value; that would be beyond the scope of this CL. I can rename the value if
you wish, but I don't think that offers any real benefit for the time being.
(All that said, I agree; it's weird, overly complex w/corresponding
AppListSource, and under-utilized. Like most ash code, it's not exactly optimal)

https://codereview.chromium.org/2696073002/diff/20001/ash/public/cpp/shelf_ty...
File ash/public/cpp/shelf_types.h (right):

https://codereview.chromium.org/2696073002/diff/20001/ash/public/cpp/shelf_ty...
ash/public/cpp/shelf_types.h:60: LAUNCH_FROM_UNKNOWN,
On 2017/02/15 00:44:40, James Cook wrote:
> Can you document each of these while you're here?

Done.

https://codereview.chromium.org/2696073002/diff/20001/ash/wm/window_util.cc
File ash/wm/window_util.cc (right):

https://codereview.chromium.org/2696073002/diff/20001/ash/wm/window_util.cc#n...
ash/wm/window_util.cc:36: // Move the |window| to the target |root|, using the
corresponding container.
On 2017/02/15 00:44:40, James Cook wrote:
> nit: document return value ("True if it actually moved"?)

Done.

https://codereview.chromium.org/2696073002/diff/20001/ash/wm/window_util.cc#n...
ash/wm/window_util.cc:85: bool MoveWindowToDisplay(aura::Window* window, int64_t
display_id) {
On 2017/02/15 00:44:40, James Cook wrote:
> DCHECK(window) and that display_id is valid?

Added DCHECK(window), but left it returning false for invalid display ids.

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

https://codereview.chromium.org/2696073002/diff/20001/chrome/browser/ui/ash/l...
chrome/browser/ui/ash/launcher/app_shortcut_launcher_item_controller.cc:120:
source, event_flags);
On 2017/02/15 00:44:40, James Cook wrote:
> Is this change needed?

Reverted; no, afaict, this would have no behavioral difference at the moment. A
separate subclass, BrowserShortcutLauncherItemController uses the CTRL key
modifier flag to create new browser windows (instead of activating an existing
one, etc.). I *may* need to make a similar change in a follow up CL to merge
ItemSelected and GetAppMenuItems, but perhaps that also won't matter for some
time. It also just seems silly to disregard the flags available here.

https://codereview.chromium.org/2696073002/diff/20001/chrome/browser/ui/ash/l...
File chrome/browser/ui/ash/launcher/app_window_launcher_item_controller.cc
(left):

https://codereview.chromium.org/2696073002/diff/20001/chrome/browser/ui/ash/l...
chrome/browser/ui/ash/launcher/app_window_launcher_item_controller.cc:79: }
On 2017/02/15 00:44:40, James Cook wrote:
> I'm a little lost. Where did the functionality provided by this function end
up?
> Was it duplicated somewhere and isn't needed any more?

It's nearly equivalent to ItemSelected; and I can't discern any behavior
differences here via manual testing, so it's not really needed; afaict.

That might be partly because shelf button presses via keyboard deliver
ET_KEY_PRESSED, not ET_KEY_RELEASED... That change could be quite old (I vaguely
remember hearing about changes like that back in the day). I could follow up
with a patch to remove ActivateOrAdvanceToNextAppWindow or re-enable it by
checking for either key event type... (Issue 614451 might make that moot) wdyt?

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

https://codereview.chromium.org/2696073002/diff/20001/chrome/browser/ui/ash/l...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl.cc:450: int64_t
display_id = display::Screen::GetScreen()->GetPrimaryDisplay().id();
On 2017/02/15 00:44:40, James Cook wrote:
> Is it correct to assume the primary display here? Should this be passing an
> invalid display ID to mean "I don't know"?
> 
> If it's OK here to assume the primary display I would comment why.

Good idea, I've replaced assumptions of primary with invalid ids.
FYI, it's only used for moving app panels to the active display.

[commit-bot: I haz the power, 2017-02-15 21:37:15]

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

[commit-bot: I haz the power, 2017-02-15 21:37:16]

Dry run: This issue passed the CQ dry run.

[James Cook, 2017-02-15 23:06:12]

LGTM modulo offline discussion about overloading ItemSelected().

https://codereview.chromium.org/2696073002/diff/40001/ash/wm/window_util_unit...
File ash/wm/window_util_unittest.cc (right):

https://codereview.chromium.org/2696073002/diff/40001/ash/wm/window_util_unit...
ash/wm/window_util_unittest.cc:146: }
Thanks for adding test coverage.

[James Cook, 2017-02-16 00:08:19]

LGTM

https://codereview.chromium.org/2696073002/diff/60001/ash/common/shelf/shelf_...
File ash/common/shelf/shelf_item_delegate.h (right):

https://codereview.chromium.org/2696073002/diff/60001/ash/common/shelf/shelf_...
ash/common/shelf/shelf_item_delegate.h:26: ShelfAction
ItemSelectedBySource(ShelfLaunchSource source);
nit: I would put this above the comment, or below ItemSelected(), with its own
comment.

[msw, 2017-02-16 00:12:20]

comments addressed; landing. Thanks!

https://codereview.chromium.org/2696073002/diff/40001/ash/wm/window_util_unit...
File ash/wm/window_util_unittest.cc (right):

https://codereview.chromium.org/2696073002/diff/40001/ash/wm/window_util_unit...
ash/wm/window_util_unittest.cc:146: }
On 2017/02/15 23:06:12, James Cook wrote:
> Thanks for adding test coverage.

Acknowledged.

https://codereview.chromium.org/2696073002/diff/60001/ash/common/shelf/shelf_...
File ash/common/shelf/shelf_item_delegate.h (right):

https://codereview.chromium.org/2696073002/diff/60001/ash/common/shelf/shelf_...
ash/common/shelf/shelf_item_delegate.h:26: ShelfAction
ItemSelectedBySource(ShelfLaunchSource source);
On 2017/02/16 00:08:18, James Cook wrote:
> nit: I would put this above the comment, or below ItemSelected(), with its own
> comment.

Done.

[msw, 2017-02-16 00:12:27]

The CQ bit was checked by msw@chromium.org

[msw, 2017-02-16 00:12:27]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamescook@chromium.org
Link to the patchset: https://codereview.chromium.org/2696073002/#ps80001
(title: "nit")

[commit-bot: I haz the power, 2017-02-16 00:13:53]

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-02-16 02:16:53]

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

[commit-bot: I haz the power, 2017-02-16 02:16:55]

Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)

[msw, 2017-02-16 02:18:55]

The CQ bit was checked by msw@chromium.org

[commit-bot: I haz the power, 2017-02-16 02:20:21]

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-02-16 02:51:41]

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

[commit-bot: I haz the power, 2017-02-16 02:51:43]

Exceeded global retry quota

[msw, 2017-02-16 04:31:53]

The CQ bit was checked by msw@chromium.org

[commit-bot: I haz the power, 2017-02-16 04:33:36]

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-02-16 08:44:57]

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

[commit-bot: I haz the power, 2017-02-16 08:44:58]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[msw, 2017-02-16 17:44:30]

The CQ bit was checked by msw@chromium.org

[commit-bot: I haz the power, 2017-02-16 17:45:13]

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-02-16 20:08:22]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1487267070185460,
"parent_rev": "601dc9f2edb0f9410d3c79a459a56ef38ba68cf1", "commit_rev":
"c30c27728fcf046088ca6c0da51bda06e3a5ce63"}

[commit-bot: I haz the power, 2017-02-16 20:09:17]

Description was changed from

==========
Merge ShelfItemDelegate::ItemSelected & LauncherItemDelegate::Activate.

These functions have nearly identical purposes; merge them.
(app-list used Activate via ChromeLauncherControllerImpl::ActivateApp)

Split up the ui::Event argument into constituent pieces.
(this will help when converting the function to support mojo)

Move ash::LaunchSource (in src/chrome) -> ash::ShelfLaunchSource.
Move ash::ShelfItemDelegate::PerformedAction -> ash::ShelfAction.
(also update its kConstantValueNaming to ENUM_VALUE_NAMING)

Add ash::wm::MoveWindowToDisplay helper; minor refactor; add test.

BUG=557406
TEST=No behavior changes selecting/activating shelf or app-list items.
R=jamescook@chromium.org
==========

to

==========
Merge ShelfItemDelegate::ItemSelected & LauncherItemDelegate::Activate.

These functions have nearly identical purposes; merge them.
(app-list used Activate via ChromeLauncherControllerImpl::ActivateApp)

Split up the ui::Event argument into constituent pieces.
(this will help when converting the function to support mojo)

Move ash::LaunchSource (in src/chrome) -> ash::ShelfLaunchSource.
Move ash::ShelfItemDelegate::PerformedAction -> ash::ShelfAction.
(also update its kConstantValueNaming to ENUM_VALUE_NAMING)

Add ash::wm::MoveWindowToDisplay helper; minor refactor; add test.

BUG=557406
TEST=No behavior changes selecting/activating shelf or app-list items.
R=jamescook@chromium.org

Review-Url: https://codereview.chromium.org/2696073002
Cr-Commit-Position: refs/heads/master@{#451072}
Committed:
https://chromium.googlesource.com/chromium/src/+/c30c27728fcf046088ca6c0da51b...
==========

[commit-bot: I haz the power, 2017-02-16 20:09:18]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/c30c27728fcf046088ca6c0da51b...