Add simple mash context menu support.

Add simple mash context menu support.

Add ash/mus/context_menu_mus.* for mash context menus.
(populates local ash items like ash_shell_with_content)
Tangential ShelfView refactoring (can be split).

BUG=557406
TEST="chrome --mash" context menus appear.
R=sky@chromium.org

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

[msw, 2016-03-02 22:46:57]

Description was changed from

==========
Add simple mash context menu support.

BUG=557406
TEST="chrome --mash"
==========

to

==========
Add simple mash context menu support.

TODO: Cite menu bug ( http://crbug.com/548441 )?

BUG=557406
TEST="chrome --mash"
==========

[msw, 2016-03-03 21:36:44]

Patchset #5 (id:80001) has been deleted

[msw, 2016-03-17 22:20:58]

Description was changed from

==========
Add simple mash context menu support.

TODO: Cite menu bug ( http://crbug.com/548441 )?

BUG=557406
TEST="chrome --mash"
==========

to

==========
Add simple mash context menu support.

TODO: Cite menu bug ( http://crbug.com/548441 )?

Depends on https://codereview.chromium.org/1812013003

BUG=557406
TEST="chrome --mash"
==========

[msw, 2016-03-18 01:19:17]

Patchset #8 (id:160001) has been deleted

[msw, 2016-03-18 01:19:28]

Patchset #6 (id:120001) has been deleted

[msw, 2016-03-18 01:19:41]

Patchset #7 (id:180001) has been deleted

[msw, 2016-03-18 01:19:54]

Patchset #5 (id:100001) has been deleted

[msw, 2016-03-18 01:20:35]

Patchset #5 (id:140001) has been deleted

[msw, 2016-03-18 01:21:05]

Patchset #10 (id:300001) has been deleted

[msw, 2016-03-18 01:21:16]

Patchset #10 (id:320001) has been deleted

[msw, 2016-03-18 01:21:25]

Patchset #10 (id:340001) has been deleted

[msw, 2016-03-18 01:21:43]

Patchset #4 (id:60001) has been deleted

[msw, 2016-03-18 01:21:55]

Patchset #3 (id:40001) has been deleted

[msw, 2016-03-18 01:22:05]

Patchset #8 (id:360001) has been deleted

[msw, 2016-03-18 01:22:18]

Patchset #7 (id:280001) has been deleted

[msw, 2016-03-18 01:22:27]

Patchset #6 (id:260001) has been deleted

[msw, 2016-03-18 19:28:15]

Description was changed from

==========
Add simple mash context menu support.

TODO: Cite menu bug ( http://crbug.com/548441 )?

Depends on https://codereview.chromium.org/1812013003

BUG=557406
TEST="chrome --mash"
==========

to

==========
Add simple mash context menu support.

Add ash/mus/context_menu_mus.* for mash context menus.
(populates local ash items and remote Chrome items)

Add mash/shelf/public/interfaces/context_menu.mojom.
(supports Chrome delegation of ash context menus)

Add chrome/browser/ui/ash/context_menu.*
Supplies current profile info and test menu items.

Tangential ShelfView refactoring (can be split).

BUG=557406
TEST="chrome --mash" context menus appear.
R=sky@chromium.org
==========

[msw, 2016-03-18 19:28:15]

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

[msw, 2016-03-18 19:29:07]

Hey Scott, please take a look, thanks!

[msw, 2016-03-18 19:30:49]

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

[commit-bot: I haz the power, 2016-03-18 19:31:46]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1760743002/400001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1760743002/400001

[commit-bot: I haz the power, 2016-03-18 19:37:25]

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

[commit-bot: I haz the power, 2016-03-18 19:37:26]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_compile_dbg_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[msw, 2016-03-18 19:48:33]

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

[msw, 2016-03-18 19:48:40]

Patchset #7 (id:400001) has been deleted

[commit-bot: I haz the power, 2016-03-18 19:49:20]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1760743002/420001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1760743002/420001

[commit-bot: I haz the power, 2016-03-18 20:17:05]

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

[commit-bot: I haz the power, 2016-03-18 20:17:07]

Dry run: Try jobs failed on following builders:
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)

[sky, 2016-03-18 20:30:57]

https://codereview.chromium.org/1760743002/diff/420001/ash/mus/context_menu_m...
File ash/mus/context_menu_mus.cc (right):

https://codereview.chromium.org/1760743002/diff/420001/ash/mus/context_menu_m...
ash/mus/context_menu_mus.cc:42: context_menu_.WaitForIncomingResponse();
We shouldn't block here. There are two options for how to deal with this:
1. Show the menu with the parts you know immediately and when you get back the
callback add them.
2. Wait for the callback then show the menu.

Both have there merits.

https://codereview.chromium.org/1760743002/diff/420001/ash/mus/context_menu_m...
File ash/mus/context_menu_mus.h (right):

https://codereview.chromium.org/1760743002/diff/420001/ash/mus/context_menu_m...
ash/mus/context_menu_mus.h:22: namespace sysui {
This shouldn't be in the sysui namespace (namespace is based on directory). I
would leave it in ash namespace only.

https://codereview.chromium.org/1760743002/diff/420001/ash/shelf/shelf_view.cc
File ash/shelf/shelf_view.cc (right):

https://codereview.chromium.org/1760743002/diff/420001/ash/shelf/shelf_view.c...
ash/shelf/shelf_view.cc:1797: base::AutoReset<ShelfID>
reseter(&context_menu_id_, item ? item->id : 0);
You shouldn't use AutoReset here. It's possible for this to be deleted around
1846. Maybe make ShowMenu return a status?

https://codereview.chromium.org/1760743002/diff/420001/chrome/browser/ui/ash/...
File chrome/browser/ui/ash/context_menu.cc (right):

https://codereview.chromium.org/1760743002/diff/420001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/context_menu.cc:34: test_item->command_id = 990;
I'm going to assume all this is temporary and not comment on it.

https://codereview.chromium.org/1760743002/diff/420001/mash/shelf/public/inte...
File mash/shelf/public/interfaces/context_menu.mojom (right):

https://codereview.chromium.org/1760743002/diff/420001/mash/shelf/public/inte...
mash/shelf/public/interfaces/context_menu.mojom:7: struct ContextMenuItem {
I have a feeling we're going to need this structure in other places. How about
moving to mash/common/public/interfaces/context_menu_item.mojom?

[sky, 2016-03-18 20:31:34]

https://codereview.chromium.org/1760743002/diff/420001/mash/shelf/public/inte...
File mash/shelf/public/interfaces/context_menu.mojom (right):

https://codereview.chromium.org/1760743002/diff/420001/mash/shelf/public/inte...
mash/shelf/public/interfaces/context_menu.mojom:7: struct ContextMenuItem {
On 2016/03/18 20:30:57, sky wrote:
> I have a feeling we're going to need this structure in other places. How about
> moving to mash/common/public/interfaces/context_menu_item.mojom?

Ignore me on this one. While I think we'll need this for other code, we can move
it when that happens.

[msw, 2016-03-18 22:07:16]

Patchset #8 (id:440001) has been deleted

[msw, 2016-03-18 22:07:58]

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

[commit-bot: I haz the power, 2016-03-18 22:08:19]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1760743002/460001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1760743002/460001

[msw, 2016-03-18 22:10:13]

Description was changed from

==========
Add simple mash context menu support.

Add ash/mus/context_menu_mus.* for mash context menus.
(populates local ash items and remote Chrome items)

Add mash/shelf/public/interfaces/context_menu.mojom.
(supports Chrome delegation of ash context menus)

Add chrome/browser/ui/ash/context_menu.*
Supplies current profile info and test menu items.

Tangential ShelfView refactoring (can be split).

BUG=557406
TEST="chrome --mash" context menus appear.
R=sky@chromium.org
==========

to

==========
Add simple mash context menu support.

Add ash/mus/context_menu_mus.* for mash context menus.
(populates local ash items like ash_shell_with_content)
Tangential ShelfView refactoring (can be split).

BUG=557406
TEST="chrome --mash" context menus appear.
R=sky@chromium.org
==========

[msw, 2016-03-18 22:10:31]

Please take another look; thanks!

https://codereview.chromium.org/1760743002/diff/420001/ash/mus/context_menu_m...
File ash/mus/context_menu_mus.cc (right):

https://codereview.chromium.org/1760743002/diff/420001/ash/mus/context_menu_m...
ash/mus/context_menu_mus.cc:42: context_menu_.WaitForIncomingResponse();
On 2016/03/18 20:30:57, sky wrote:
> We shouldn't block here. There are two options for how to deal with this:
> 1. Show the menu with the parts you know immediately and when you get back the
> callback add them.
> 2. Wait for the callback then show the menu.
> 
> Both have there merits.

Adding a menu without Chrome hooks for now, as discussed.

https://codereview.chromium.org/1760743002/diff/420001/ash/mus/context_menu_m...
File ash/mus/context_menu_mus.h (right):

https://codereview.chromium.org/1760743002/diff/420001/ash/mus/context_menu_m...
ash/mus/context_menu_mus.h:22: namespace sysui {
On 2016/03/18 20:30:57, sky wrote:
> This shouldn't be in the sysui namespace (namespace is based on directory). I
> would leave it in ash namespace only.

Done.

https://codereview.chromium.org/1760743002/diff/420001/ash/shelf/shelf_view.cc
File ash/shelf/shelf_view.cc (right):

https://codereview.chromium.org/1760743002/diff/420001/ash/shelf/shelf_view.c...
ash/shelf/shelf_view.cc:1797: base::AutoReset<ShelfID>
reseter(&context_menu_id_, item ? item->id : 0);
On 2016/03/18 20:30:57, sky wrote:
> You shouldn't use AutoReset here. It's possible for this to be deleted around
> 1846. Maybe make ShowMenu return a status?

AFAICT, we don't need the AutoReset and can just set context_menu_id_ back to
zero within ShowMenu. (works when right-clicking two items consecutively)

[commit-bot: I haz the power, 2016-03-19 00:26:10]

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

[commit-bot: I haz the power, 2016-03-19 00:26:11]

Dry run: This issue passed the CQ dry run.

[sky, 2016-03-21 16:55:57]

LGTM

https://codereview.chromium.org/1760743002/diff/460001/ash/shelf/shelf_view.cc
File ash/shelf/shelf_view.cc (right):

https://codereview.chromium.org/1760743002/diff/460001/ash/shelf/shelf_view.c...
ash/shelf/shelf_view.cc:1842: if (!got_deleted) {
I don't understand why we have this branch at all. If MENU_DELETED returns true,
then this should have been deleted. We can worry about that separately I guess.

[msw, 2016-03-21 17:05:47]

https://codereview.chromium.org/1760743002/diff/460001/ash/shelf/shelf_view.cc
File ash/shelf/shelf_view.cc (right):

https://codereview.chromium.org/1760743002/diff/460001/ash/shelf/shelf_view.c...
ash/shelf/shelf_view.cc:1842: if (!got_deleted) {
On 2016/03/21 16:55:57, sky wrote:
> I don't understand why we have this branch at all. If MENU_DELETED returns
true,
> then this should have been deleted. We can worry about that separately I
guess.

Acknowledged.

[msw, 2016-03-21 17:05:54]

The CQ bit was checked by msw@chromium.org

[commit-bot: I haz the power, 2016-03-21 17:06:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1760743002/460001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1760743002/460001

[commit-bot: I haz the power, 2016-03-21 18:08:34]

Description was changed from

==========
Add simple mash context menu support.

Add ash/mus/context_menu_mus.* for mash context menus.
(populates local ash items like ash_shell_with_content)
Tangential ShelfView refactoring (can be split).

BUG=557406
TEST="chrome --mash" context menus appear.
R=sky@chromium.org
==========

to

==========
Add simple mash context menu support.

Add ash/mus/context_menu_mus.* for mash context menus.
(populates local ash items like ash_shell_with_content)
Tangential ShelfView refactoring (can be split).

BUG=557406
TEST="chrome --mash" context menus appear.
R=sky@chromium.org
==========

[commit-bot: I haz the power, 2016-03-21 18:08:35]

Committed patchset #8 (id:460001)

[commit-bot: I haz the power, 2016-03-21 18:10:04]

Description was changed from

==========
Add simple mash context menu support.

Add ash/mus/context_menu_mus.* for mash context menus.
(populates local ash items like ash_shell_with_content)
Tangential ShelfView refactoring (can be split).

BUG=557406
TEST="chrome --mash" context menus appear.
R=sky@chromium.org
==========

to

==========
Add simple mash context menu support.

Add ash/mus/context_menu_mus.* for mash context menus.
(populates local ash items like ash_shell_with_content)
Tangential ShelfView refactoring (can be split).

BUG=557406
TEST="chrome --mash" context menus appear.
R=sky@chromium.org

Committed: https://crrev.com/bc1cee6f1db69579e72287bdd1aca442ca904ec2
Cr-Commit-Position: refs/heads/master@{#382326}
==========

[commit-bot: I haz the power, 2016-03-21 18:10:06]

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