Add basic Chrome interaction with the mash shelf.

Add basic Chrome interaction with the mash shelf.

Based on the "Mash Shelf Interface Proposal" doc:
https://docs.google.com/document/d/1FSSAxodmRbaeiHI2H-2WNfbVXPLHLcTbcThvln0bKNw

Add shelf.mojom with necessary interfaces and structs.
Make ShelfDelegateMus implement mash::shelf::mojom::Shelf.
Make SysUIApplication (mojo:ash_sysui) vend Shelf interfaces.

Init mash from ChromeBrowserMainExtraPartsAsh::PostProfileInit.
Add a test shortcut item to a fake application from Chrome.
Store local chrome prefs when mash shelf properties change.
Include required pref-related consts/files in USE_AURA.

Extract IsRunningInMash helper function.
Extract launcher preference conversion helper functions.

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

TODO: Support multiple displays with a shelf per display.
TODO: Support complex prefs in ChromeMashShelfController.
TODO: Link up user window and pinned shelf items.
TODO: Add more NOTIMPLEMENTED() functionality...

BUG=557406
TEST=Mash shelf has a new test item, stores prefs locally.
R=sky@chromium.org,jamescook@chromium.org,tomhudson@google.com

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

[msw, 2016-03-31 20:59:59]

Patchset #2 (id:20001) has been deleted

[msw, 2016-04-05 21:23:51]

Patchset #6 (id:120001) has been deleted

[msw, 2016-04-05 21:24:12]

Patchset #6 (id:140001) has been deleted

[msw, 2016-04-05 21:34:49]

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

[commit-bot: I haz the power, 2016-04-05 21:35:20]

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

[msw, 2016-04-05 21:36:05]

Description was changed from

==========
Add basic Chrome interaction with the mash shelf.

TODO: File a chrome<->shelf bug.

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

to

==========
Add basic Chrome interaction with the mash shelf.

Based on the "Mash Shelf Interface Proposal" doc:
https://docs.google.com/document/d/1FSSAxodmRbaeiHI2H-2WNfbVXPLHLcTbcThvln0bKNw

Add shelf.mojom with necessary interfaces and structs.
Make ShelfDelegateMus implement mash::shelf::mojom::Shelf.
Make SysUIApplication (mojo:ash_sysui) vend Shelf interfaces.

Init mash from ChromeBrowserMainExtraPartsAsh::PostProfileInit.
Add a test shortcut item to a fake application from Chrome.
Store local chrome prefs when mash shelf properties change.

Extract IsRunningInMash helper function.
Extract launcher preference conversion helper functions.

TODO: Support multiple displays with a shelf per display.
TODO: Support complex prefs in ChromeMashShelfController.
TODO: Link up user window and pinned shelf items.
TODO: Add more NOTIMPLEMENTED() functionality...

BUG=557406
TEST=Mash shelf has a new test item, stores prefs locally.
R=sky@chromium.org
==========

[msw, 2016-04-05 21:36:06]

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

[commit-bot: I haz the power, 2016-04-05 21:51:16]

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

[commit-bot: I haz the power, 2016-04-05 21:51:17]

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

[msw, 2016-04-05 22:07:20]

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

[commit-bot: I haz the power, 2016-04-05 22:08:00]

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

[commit-bot: I haz the power, 2016-04-05 22:24:35]

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

[commit-bot: I haz the power, 2016-04-05 22:24:36]

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

[msw, 2016-04-05 23:26:45]

Description was changed from

==========
Add basic Chrome interaction with the mash shelf.

Based on the "Mash Shelf Interface Proposal" doc:
https://docs.google.com/document/d/1FSSAxodmRbaeiHI2H-2WNfbVXPLHLcTbcThvln0bKNw

Add shelf.mojom with necessary interfaces and structs.
Make ShelfDelegateMus implement mash::shelf::mojom::Shelf.
Make SysUIApplication (mojo:ash_sysui) vend Shelf interfaces.

Init mash from ChromeBrowserMainExtraPartsAsh::PostProfileInit.
Add a test shortcut item to a fake application from Chrome.
Store local chrome prefs when mash shelf properties change.

Extract IsRunningInMash helper function.
Extract launcher preference conversion helper functions.

TODO: Support multiple displays with a shelf per display.
TODO: Support complex prefs in ChromeMashShelfController.
TODO: Link up user window and pinned shelf items.
TODO: Add more NOTIMPLEMENTED() functionality...

BUG=557406
TEST=Mash shelf has a new test item, stores prefs locally.
R=sky@chromium.org
==========

to

==========
Add basic Chrome interaction with the mash shelf.

Based on the "Mash Shelf Interface Proposal" doc:
https://docs.google.com/document/d/1FSSAxodmRbaeiHI2H-2WNfbVXPLHLcTbcThvln0bKNw

Add shelf.mojom with necessary interfaces and structs.
Make ShelfDelegateMus implement mash::shelf::mojom::Shelf.
Make SysUIApplication (mojo:ash_sysui) vend Shelf interfaces.

Init mash from ChromeBrowserMainExtraPartsAsh::PostProfileInit.
Add a test shortcut item to a fake application from Chrome.
Store local chrome prefs when mash shelf properties change.
Include required pref-related consts/files in USE_AURA.

Extract IsRunningInMash helper function.
Extract launcher preference conversion helper functions.

TODO: Support multiple displays with a shelf per display.
TODO: Support complex prefs in ChromeMashShelfController.
TODO: Link up user window and pinned shelf items.
TODO: Add more NOTIMPLEMENTED() functionality...

BUG=557406
TEST=Mash shelf has a new test item, stores prefs locally.
R=sky@chromium.org
==========

[msw, 2016-04-05 23:26:52]

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

[msw, 2016-04-05 23:27:11]

Hey Scott, please take a look; thanks!

[commit-bot: I haz the power, 2016-04-05 23:27:26]

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

[commit-bot: I haz the power, 2016-04-05 23:42:07]

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

[commit-bot: I haz the power, 2016-04-05 23:42:08]

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

[sky, 2016-04-06 02:57:45]

https://codereview.chromium.org/1839223003/diff/200001/mash/shelf/public/inte...
File mash/shelf/public/interfaces/shelf.mojom (right):

https://codereview.chromium.org/1839223003/diff/200001/mash/shelf/public/inte...
mash/shelf/public/interfaces/shelf.mojom:21: interface ShelfObserver {
When the observer is added seems like you should call OnAlignment/OnAutoHide*.
Otherwise, how does the observer know the state?

https://codereview.chromium.org/1839223003/diff/200001/mash/shelf/public/inte...
mash/shelf/public/interfaces/shelf.mojom:27: interface ShelfItemDelegate {
Why do you need 'id' in each of these functions? Isn't the delegate associated
with one and only one item, whose id won't change?

https://codereview.chromium.org/1839223003/diff/200001/mash/shelf/public/inte...
mash/shelf/public/interfaces/shelf.mojom:27: interface ShelfItemDelegate {
I'm nervous that all the delegates are different pipes. An implication of that
is ordering is not guaranteed. Would it make sense to have all the delegates
associated with the shelf, and perhaps the ShelfObserver associated with the
shelf too?

https://codereview.chromium.org/1839223003/diff/200001/mash/shelf/public/inte...
mash/shelf/public/interfaces/shelf.mojom:32: ItemRemoved(string id);
If this is called when the item is unpinned can it be called ItemUnpinned? Might
the item become pinned again?

https://codereview.chromium.org/1839223003/diff/200001/mash/shelf/public/inte...
mash/shelf/public/interfaces/shelf.mojom:35: ItemReordered(string id, uint32
order);
Document what 'order' means.

[James Cook, 2016-04-06 04:29:52]

Mike, if you'd like me to refrain from drive-by's, or if you'd prefer me to ask
questions some other way, please let me know.

https://codereview.chromium.org/1839223003/diff/200001/ash/mus/shelf_delegate...
File ash/mus/shelf_delegate_mus.h (right):

https://codereview.chromium.org/1839223003/diff/200001/ash/mus/shelf_delegate...
ash/mus/shelf_delegate_mus.h:23: public mash::shelf::mojom::Shelf,
optional drive-by comment: I find it kind of confusing that ShelfDelegateMus
both is-a ShelfDelegate and is-a Shelf. The latter is namespaced, but it still
feels odd to me. It leads to some references to "Shelf" becoming "ash::Shelf"
despite being in namespace ash, whereas things like ShelfID don't need to be
namespaced.

I wonder if it would be clearer to have something the interface be something
like mash::shelf::mojom::ShelfController or ShelfManager. Another approach might
be to avoid inheritance and make ShelfDelegateMus have a member
mash::shelf::mojom::Shelf.

Again, just an optional drive-by, but WDYT?

https://codereview.chromium.org/1839223003/diff/200001/chrome/browser/ui/ash/...
File chrome/browser/ui/ash/launcher/chrome_mash_shelf_controller.cc (right):

https://codereview.chromium.org/1839223003/diff/200001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/launcher/chrome_mash_shelf_controller.cc:18:
ChromeMashShelfController* ChromeMashShelfController::instance_ = NULL;
drive-by nit: nullptr

https://codereview.chromium.org/1839223003/diff/200001/chrome/browser/ui/view...
File chrome/browser/ui/views/ash/chrome_browser_main_extra_parts_ash.cc (right):

https://codereview.chromium.org/1839223003/diff/200001/chrome/browser/ui/view...
chrome/browser/ui/views/ash/chrome_browser_main_extra_parts_ash.cc:37:
ash::Shell::GetInstance()->CreateShelf();
Hooray for correct (auto) formatting! :-)

https://codereview.chromium.org/1839223003/diff/200001/chrome/common/pref_nam...
File chrome/common/pref_names.h (right):

https://codereview.chromium.org/1839223003/diff/200001/chrome/common/pref_nam...
chrome/common/pref_names.h:706: #if defined(USE_AURA)
Possibly silly drive-by question: Why does all this need to move into USE_AURA
instead of living in USE_ASH? I see the source files moving between groups in
gyp/gn, but I don't understand why. Why does chrome_mash_shelf_controller.cc
compile using USE_AURA but not USE_ASH? I must have missed something. Maybe I've
forgotten how USE_AURA and USE_ASH interact.

[msw, 2016-04-07 20:23:21]

Description was changed from

==========
Add basic Chrome interaction with the mash shelf.

Based on the "Mash Shelf Interface Proposal" doc:
https://docs.google.com/document/d/1FSSAxodmRbaeiHI2H-2WNfbVXPLHLcTbcThvln0bKNw

Add shelf.mojom with necessary interfaces and structs.
Make ShelfDelegateMus implement mash::shelf::mojom::Shelf.
Make SysUIApplication (mojo:ash_sysui) vend Shelf interfaces.

Init mash from ChromeBrowserMainExtraPartsAsh::PostProfileInit.
Add a test shortcut item to a fake application from Chrome.
Store local chrome prefs when mash shelf properties change.
Include required pref-related consts/files in USE_AURA.

Extract IsRunningInMash helper function.
Extract launcher preference conversion helper functions.

TODO: Support multiple displays with a shelf per display.
TODO: Support complex prefs in ChromeMashShelfController.
TODO: Link up user window and pinned shelf items.
TODO: Add more NOTIMPLEMENTED() functionality...

BUG=557406
TEST=Mash shelf has a new test item, stores prefs locally.
R=sky@chromium.org
==========

to

==========
Add basic Chrome interaction with the mash shelf.

Based on the "Mash Shelf Interface Proposal" doc:
https://docs.google.com/document/d/1FSSAxodmRbaeiHI2H-2WNfbVXPLHLcTbcThvln0bKNw

Add shelf.mojom with necessary interfaces and structs.
Make ShelfDelegateMus implement mash::shelf::mojom::Shelf.
Make SysUIApplication (mojo:ash_sysui) vend Shelf interfaces.

Init mash from ChromeBrowserMainExtraPartsAsh::PostProfileInit.
Add a test shortcut item to a fake application from Chrome.
Store local chrome prefs when mash shelf properties change.
Include required pref-related consts/files in USE_AURA.

Extract IsRunningInMash helper function.
Extract launcher preference conversion helper functions.

Add AssociatedInterfacePtrSet to parallel InterfacePtrSet.

TODO: Support multiple displays with a shelf per display.
TODO: Support complex prefs in ChromeMashShelfController.
TODO: Link up user window and pinned shelf items.
TODO: Add more NOTIMPLEMENTED() functionality...

BUG=557406
TEST=Mash shelf has a new test item, stores prefs locally.
R=sky@chromium.org
==========

[msw, 2016-04-07 20:31:38]

Patchset #9 (id:220001) has been deleted

[msw, 2016-04-07 20:32:05]

Description was changed from

==========
Add basic Chrome interaction with the mash shelf.

Based on the "Mash Shelf Interface Proposal" doc:
https://docs.google.com/document/d/1FSSAxodmRbaeiHI2H-2WNfbVXPLHLcTbcThvln0bKNw

Add shelf.mojom with necessary interfaces and structs.
Make ShelfDelegateMus implement mash::shelf::mojom::Shelf.
Make SysUIApplication (mojo:ash_sysui) vend Shelf interfaces.

Init mash from ChromeBrowserMainExtraPartsAsh::PostProfileInit.
Add a test shortcut item to a fake application from Chrome.
Store local chrome prefs when mash shelf properties change.
Include required pref-related consts/files in USE_AURA.

Extract IsRunningInMash helper function.
Extract launcher preference conversion helper functions.

Add AssociatedInterfacePtrSet to parallel InterfacePtrSet.

TODO: Support multiple displays with a shelf per display.
TODO: Support complex prefs in ChromeMashShelfController.
TODO: Link up user window and pinned shelf items.
TODO: Add more NOTIMPLEMENTED() functionality...

BUG=557406
TEST=Mash shelf has a new test item, stores prefs locally.
R=sky@chromium.org
==========

to

==========
Add basic Chrome interaction with the mash shelf.

Based on the "Mash Shelf Interface Proposal" doc:
https://docs.google.com/document/d/1FSSAxodmRbaeiHI2H-2WNfbVXPLHLcTbcThvln0bKNw

Add shelf.mojom with necessary interfaces and structs.
Make ShelfDelegateMus implement mash::shelf::mojom::Shelf.
Make SysUIApplication (mojo:ash_sysui) vend Shelf interfaces.

Init mash from ChromeBrowserMainExtraPartsAsh::PostProfileInit.
Add a test shortcut item to a fake application from Chrome.
Store local chrome prefs when mash shelf properties change.
Include required pref-related consts/files in USE_AURA.

Extract IsRunningInMash helper function.
Extract launcher preference conversion helper functions.

Add AssociatedInterfacePtrSet to parallel InterfacePtrSet.

TODO: Support multiple displays with a shelf per display.
TODO: Support complex prefs in ChromeMashShelfController.
TODO: Link up user window and pinned shelf items.
TODO: Add more NOTIMPLEMENTED() functionality...

BUG=557406
TEST=Mash shelf has a new test item, stores prefs locally.
R=sky@chromium.org,jamescook@chromium.org
==========

[msw, 2016-04-07 20:32:06]

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

[msw, 2016-04-07 20:32:27]

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

[commit-bot: I haz the power, 2016-04-07 20:33:36]

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

[msw, 2016-04-07 20:34:58]

Please take another look; thanks! James, your review is greatly appreciated;
drive-by or not, I'll never turn down another set of trained eyes on my code! We
should probably work together more to reduce Scott's review burden anyway.

https://codereview.chromium.org/1839223003/diff/200001/ash/mus/shelf_delegate...
File ash/mus/shelf_delegate_mus.h (right):

https://codereview.chromium.org/1839223003/diff/200001/ash/mus/shelf_delegate...
ash/mus/shelf_delegate_mus.h:23: public mash::shelf::mojom::Shelf,
On 2016/04/06 04:29:52, James Cook wrote:
> optional drive-by comment: I find it kind of confusing that ShelfDelegateMus
> both is-a ShelfDelegate and is-a Shelf. The latter is namespaced, but it still
> feels odd to me. It leads to some references to "Shelf" becoming "ash::Shelf"
> despite being in namespace ash, whereas things like ShelfID don't need to be
> namespaced.
> 
> I wonder if it would be clearer to have something the interface be something
> like mash::shelf::mojom::ShelfController or ShelfManager. Another approach
might
> be to avoid inheritance and make ShelfDelegateMus have a member
> mash::shelf::mojom::Shelf.
> 
> Again, just an optional drive-by, but WDYT?

ShelfController (or ShelfManager) sgtm, otherwise I'd make ash::Shelf itself
implement this interface, but then it'd be per-display (in non-unified mode).

https://codereview.chromium.org/1839223003/diff/200001/chrome/browser/ui/ash/...
File chrome/browser/ui/ash/launcher/chrome_mash_shelf_controller.cc (right):

https://codereview.chromium.org/1839223003/diff/200001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/launcher/chrome_mash_shelf_controller.cc:18:
ChromeMashShelfController* ChromeMashShelfController::instance_ = NULL;
On 2016/04/06 04:29:52, James Cook wrote:
> drive-by nit: nullptr

Done.

https://codereview.chromium.org/1839223003/diff/200001/chrome/common/pref_nam...
File chrome/common/pref_names.h (right):

https://codereview.chromium.org/1839223003/diff/200001/chrome/common/pref_nam...
chrome/common/pref_names.h:706: #if defined(USE_AURA)
On 2016/04/06 04:29:52, James Cook wrote:
> Possibly silly drive-by question: Why does all this need to move into USE_AURA
> instead of living in USE_ASH? I see the source files moving between groups in
> gyp/gn, but I don't understand why. Why does chrome_mash_shelf_controller.cc
> compile using USE_AURA but not USE_ASH? I must have missed something. Maybe
I've
> forgotten how USE_AURA and USE_ASH interact.

afaik, we want to be able to build mash w/o USE_ASH for ease of development.
I think we to remove USE_ASH and just rely on OS_CHROMEOS and runtime flags.

https://codereview.chromium.org/1839223003/diff/200001/mash/shelf/public/inte...
File mash/shelf/public/interfaces/shelf.mojom (right):

https://codereview.chromium.org/1839223003/diff/200001/mash/shelf/public/inte...
mash/shelf/public/interfaces/shelf.mojom:21: interface ShelfObserver {
On 2016/04/06 02:57:45, sky wrote:
> When the observer is added seems like you should call OnAlignment/OnAutoHide*.
> Otherwise, how does the observer know the state?

Done. FYI: we now rely on the ordering of (1) chrome setting the initial values
from prefs, and then (2) adding itself as an observer (so it doesn't get
notified of the default/uninitialized ash state and try to save that as a user
preference). Luckily that's how ChromeMashShelfController::Init works already.

https://codereview.chromium.org/1839223003/diff/200001/mash/shelf/public/inte...
mash/shelf/public/interfaces/shelf.mojom:27: interface ShelfItemDelegate {
On 2016/04/06 02:57:45, sky wrote:
> Why do you need 'id' in each of these functions? Isn't the delegate associated
> with one and only one item, whose id won't change?

I had planned that a single ShelfItemDelegate might exist for multiple shelf
items, but that's probably not necessary and not how the existing
LauncherItemController works, so I changed this around.

https://codereview.chromium.org/1839223003/diff/200001/mash/shelf/public/inte...
mash/shelf/public/interfaces/shelf.mojom:27: interface ShelfItemDelegate {
On 2016/04/06 02:57:45, sky wrote:
> I'm nervous that all the delegates are different pipes. An implication of that
> is ordering is not guaranteed. Would it make sense to have all the delegates
> associated with the shelf, and perhaps the ShelfObserver associated with the
> shelf too?

OKay, I associated observers and item delegates with the shelf controller
interface. I had to add AssociatedInterfacePtrSet (copypasta of
InterfacePtrSet); hope it's all right!

https://codereview.chromium.org/1839223003/diff/200001/mash/shelf/public/inte...
mash/shelf/public/interfaces/shelf.mojom:32: ItemRemoved(string id);
On 2016/04/06 02:57:45, sky wrote:
> If this is called when the item is unpinned can it be called ItemUnpinned?
Might
> the item become pinned again?

Yes and good call.

https://codereview.chromium.org/1839223003/diff/200001/mash/shelf/public/inte...
mash/shelf/public/interfaces/shelf.mojom:35: ItemReordered(string id, uint32
order);
On 2016/04/06 02:57:45, sky wrote:
> Document what 'order' means. 

Done.

[commit-bot: I haz the power, 2016-04-07 20:51:35]

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

[commit-bot: I haz the power, 2016-04-07 20:51:37]

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

[sky, 2016-04-07 22:02:46]

https://codereview.chromium.org/1839223003/diff/200001/mash/shelf/public/inte...
File mash/shelf/public/interfaces/shelf.mojom (right):

https://codereview.chromium.org/1839223003/diff/200001/mash/shelf/public/inte...
mash/shelf/public/interfaces/shelf.mojom:21: interface ShelfObserver {
On 2016/04/07 20:34:57, msw wrote:
> On 2016/04/06 02:57:45, sky wrote:
> > When the observer is added seems like you should call
OnAlignment/OnAutoHide*.
> > Otherwise, how does the observer know the state?
> 
> Done. FYI: we now rely on the ordering of (1) chrome setting the initial
values
> from prefs, and then (2) adding itself as an observer (so it doesn't get
> notified of the default/uninitialized ash state and try to save that as a user
> preference). Luckily that's how ChromeMashShelfController::Init works already.

Be sure and document this.

https://codereview.chromium.org/1839223003/diff/240001/ash/mus/shelf_delegate...
File ash/mus/shelf_delegate_mus.cc (right):

https://codereview.chromium.org/1839223003/diff/240001/ash/mus/shelf_delegate...
ash/mus/shelf_delegate_mus.cc:45: void set_window_id(uint32_t window_id) {
window_id_ = window_id; }
Document when this changes (maybe it should DCHECK_EQ(0, window_id_)) too.

https://codereview.chromium.org/1839223003/diff/240001/ash/mus/shelf_delegate...
ash/mus/shelf_delegate_mus.cc:191: Shelf* shelf = Shelf::ForPrimaryDisplay();
Do we support shelfs for different displays? If we do, AddObserver should take a
display.

https://codereview.chromium.org/1839223003/diff/240001/ash/mus/shelf_delegate...
ash/mus/shelf_delegate_mus.cc:220: std::string
item_id(item->id.To<std::string>());
Ignore request if item_id is in app_id_to_shelf_id_?

https://codereview.chromium.org/1839223003/diff/240001/mojo/public/cpp/bindin...
File mojo/public/cpp/bindings/associated_interface_ptr_set.h (right):

https://codereview.chromium.org/1839223003/diff/240001/mojo/public/cpp/bindin...
mojo/public/cpp/bindings/associated_interface_ptr_set.h:13: #include
"mojo/public/cpp/bindings/associated_interface_ptr.h"
Can you separate this change out and add Yuzhu? I recommend renaming
InterfacePtrSet to PtrSet and make it take the type in the template arg.
InterfacePtrSet then becomes:

using InterfacePtrSet<Interface> = PtrSet<InterfacePtr<Interface>>;

and AssociatedInterfacePtrSet:

using AssociatedInterfacePtrSet<Interface> =
PtrSet<AssociatedInterfacePtr<Interface>>;

Hopefully that works.

[msw, 2016-04-08 21:20:26]

Description was changed from

==========
Add basic Chrome interaction with the mash shelf.

Based on the "Mash Shelf Interface Proposal" doc:
https://docs.google.com/document/d/1FSSAxodmRbaeiHI2H-2WNfbVXPLHLcTbcThvln0bKNw

Add shelf.mojom with necessary interfaces and structs.
Make ShelfDelegateMus implement mash::shelf::mojom::Shelf.
Make SysUIApplication (mojo:ash_sysui) vend Shelf interfaces.

Init mash from ChromeBrowserMainExtraPartsAsh::PostProfileInit.
Add a test shortcut item to a fake application from Chrome.
Store local chrome prefs when mash shelf properties change.
Include required pref-related consts/files in USE_AURA.

Extract IsRunningInMash helper function.
Extract launcher preference conversion helper functions.

Add AssociatedInterfacePtrSet to parallel InterfacePtrSet.

TODO: Support multiple displays with a shelf per display.
TODO: Support complex prefs in ChromeMashShelfController.
TODO: Link up user window and pinned shelf items.
TODO: Add more NOTIMPLEMENTED() functionality...

BUG=557406
TEST=Mash shelf has a new test item, stores prefs locally.
R=sky@chromium.org,jamescook@chromium.org
==========

to

==========
Add basic Chrome interaction with the mash shelf.

Based on the "Mash Shelf Interface Proposal" doc:
https://docs.google.com/document/d/1FSSAxodmRbaeiHI2H-2WNfbVXPLHLcTbcThvln0bKNw

Add shelf.mojom with necessary interfaces and structs.
Make ShelfDelegateMus implement mash::shelf::mojom::Shelf.
Make SysUIApplication (mojo:ash_sysui) vend Shelf interfaces.

Init mash from ChromeBrowserMainExtraPartsAsh::PostProfileInit.
Add a test shortcut item to a fake application from Chrome.
Store local chrome prefs when mash shelf properties change.
Include required pref-related consts/files in USE_AURA.

Extract IsRunningInMash helper function.
Extract launcher preference conversion helper functions.

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

TODO: Support multiple displays with a shelf per display.
TODO: Support complex prefs in ChromeMashShelfController.
TODO: Link up user window and pinned shelf items.
TODO: Add more NOTIMPLEMENTED() functionality...

BUG=557406
TEST=Mash shelf has a new test item, stores prefs locally.
R=sky@chromium.org,jamescook@chromium.org
==========

[msw, 2016-04-08 21:20:48]

Please take another look; thanks!

https://codereview.chromium.org/1839223003/diff/240001/ash/mus/shelf_delegate...
File ash/mus/shelf_delegate_mus.cc (right):

https://codereview.chromium.org/1839223003/diff/240001/ash/mus/shelf_delegate...
ash/mus/shelf_delegate_mus.cc:45: void set_window_id(uint32_t window_id) {
window_id_ = window_id; }
On 2016/04/07 22:02:46, sky wrote:
> Document when this changes (maybe it should DCHECK_EQ(0, window_id_)) too.

Moved back to ctor. (this is super temporary; we need multi-window support)

https://codereview.chromium.org/1839223003/diff/240001/ash/mus/shelf_delegate...
ash/mus/shelf_delegate_mus.cc:191: Shelf* shelf = Shelf::ForPrimaryDisplay();
On 2016/04/07 22:02:46, sky wrote:
> Do we support shelfs for different displays? If we do, AddObserver should take
a
> display.

I asked Robert "Does mash support some test way to open multiple displays?" He
replied: "I'd say no. Mus doesn't know how to draw to multiple displays." I plan
to add multiple display support in a follow-up, since I can't really test it
now; added a TODO to the mojom.

https://codereview.chromium.org/1839223003/diff/240001/ash/mus/shelf_delegate...
ash/mus/shelf_delegate_mus.cc:220: std::string
item_id(item->id.To<std::string>());
On 2016/04/07 22:02:46, sky wrote:
> Ignore request if item_id is in app_id_to_shelf_id_?

Done.

https://codereview.chromium.org/1839223003/diff/240001/mojo/public/cpp/bindin...
File mojo/public/cpp/bindings/associated_interface_ptr_set.h (right):

https://codereview.chromium.org/1839223003/diff/240001/mojo/public/cpp/bindin...
mojo/public/cpp/bindings/associated_interface_ptr_set.h:13: #include
"mojo/public/cpp/bindings/associated_interface_ptr.h"
On 2016/04/07 22:02:46, sky wrote:
> Can you separate this change out and add Yuzhu? I recommend renaming
> InterfacePtrSet to PtrSet and make it take the type in the template arg.
> InterfacePtrSet then becomes:
> 
> using InterfacePtrSet<Interface> = PtrSet<InterfacePtr<Interface>>;
> 
> and AssociatedInterfacePtrSet:
> 
> using AssociatedInterfacePtrSet<Interface> =
> PtrSet<AssociatedInterfacePtr<Interface>>;
> 
> Hopefully that works.

Done: https://codereview.chromium.org/1872903003

[msw, 2016-04-08 21:20:55]

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

[commit-bot: I haz the power, 2016-04-08 21:21:08]

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

[commit-bot: I haz the power, 2016-04-08 21:32:28]

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

[commit-bot: I haz the power, 2016-04-08 21:32:29]

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

[msw, 2016-04-08 21:51:46]

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

[commit-bot: I haz the power, 2016-04-08 21:52:24]

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

[commit-bot: I haz the power, 2016-04-08 22:03:09]

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

[commit-bot: I haz the power, 2016-04-08 22:03:10]

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

[sky, 2016-04-08 23:08:24]

LGTM

https://codereview.chromium.org/1839223003/diff/240001/ash/mus/shelf_delegate...
File ash/mus/shelf_delegate_mus.cc (right):

https://codereview.chromium.org/1839223003/diff/240001/ash/mus/shelf_delegate...
ash/mus/shelf_delegate_mus.cc:191: Shelf* shelf = Shelf::ForPrimaryDisplay();
On 2016/04/08 21:20:48, msw wrote:
> On 2016/04/07 22:02:46, sky wrote:
> > Do we support shelfs for different displays? If we do, AddObserver should
take
> a
> > display.
> 
> I asked Robert "Does mash support some test way to open multiple displays?" He
> replied: "I'd say no. Mus doesn't know how to draw to multiple displays." I
plan
> to add multiple display support in a follow-up, since I can't really test it
> now; added a TODO to the mojom.

Mus itself supports multiple displays. But I'm fine with coming back to this.

[msw, 2016-04-08 23:26:04]

Description was changed from

==========
Add basic Chrome interaction with the mash shelf.

Based on the "Mash Shelf Interface Proposal" doc:
https://docs.google.com/document/d/1FSSAxodmRbaeiHI2H-2WNfbVXPLHLcTbcThvln0bKNw

Add shelf.mojom with necessary interfaces and structs.
Make ShelfDelegateMus implement mash::shelf::mojom::Shelf.
Make SysUIApplication (mojo:ash_sysui) vend Shelf interfaces.

Init mash from ChromeBrowserMainExtraPartsAsh::PostProfileInit.
Add a test shortcut item to a fake application from Chrome.
Store local chrome prefs when mash shelf properties change.
Include required pref-related consts/files in USE_AURA.

Extract IsRunningInMash helper function.
Extract launcher preference conversion helper functions.

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

TODO: Support multiple displays with a shelf per display.
TODO: Support complex prefs in ChromeMashShelfController.
TODO: Link up user window and pinned shelf items.
TODO: Add more NOTIMPLEMENTED() functionality...

BUG=557406
TEST=Mash shelf has a new test item, stores prefs locally.
R=sky@chromium.org,jamescook@chromium.org
==========

to

==========
Add basic Chrome interaction with the mash shelf.

Based on the "Mash Shelf Interface Proposal" doc:
https://docs.google.com/document/d/1FSSAxodmRbaeiHI2H-2WNfbVXPLHLcTbcThvln0bKNw

Add shelf.mojom with necessary interfaces and structs.
Make ShelfDelegateMus implement mash::shelf::mojom::Shelf.
Make SysUIApplication (mojo:ash_sysui) vend Shelf interfaces.

Init mash from ChromeBrowserMainExtraPartsAsh::PostProfileInit.
Add a test shortcut item to a fake application from Chrome.
Store local chrome prefs when mash shelf properties change.
Include required pref-related consts/files in USE_AURA.

Extract IsRunningInMash helper function.
Extract launcher preference conversion helper functions.

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

TODO: Support multiple displays with a shelf per display.
TODO: Support complex prefs in ChromeMashShelfController.
TODO: Link up user window and pinned shelf items.
TODO: Add more NOTIMPLEMENTED() functionality...

BUG=557406
TEST=Mash shelf has a new test item, stores prefs locally.
R=sky@chromium.org,jamescook@chromium.org
TBR=tomhudson@google.com
==========

[msw, 2016-04-08 23:26:04]

msw@chromium.org changed reviewers:
+ tomhudson@google.com

[msw, 2016-04-08 23:28:15]

+TBR tomhudson for new users of skia/public[/interfaces].

[James Cook, 2016-04-11 15:00:46]

jamescook@chromium.org changed reviewers:
- jamescook@chromium.org

[msw, 2016-04-11 18:07:54]

The CQ bit was checked by msw@chromium.org

[msw, 2016-04-11 18:07:55]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1839223003/#ps300001
(title: "Leave +skia/public in chrome/browser/DEPS")

[commit-bot: I haz the power, 2016-04-11 18:08:20]

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

[msw, 2016-04-11 18:08:55]

Patchset #12 (id:300001) has been deleted

[msw, 2016-04-11 18:11:07]

The CQ bit was checked by msw@chromium.org

[msw, 2016-04-11 18:11:07]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1839223003/#ps320001
(title: "Leave +skia/public in chrome/browser/DEPS")

[commit-bot: I haz the power, 2016-04-11 18:11:18]

Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_gn...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)

[commit-bot: I haz the power, 2016-04-11 18:11:40]

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

[commit-bot: I haz the power, 2016-04-11 19:15:04]

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

[commit-bot: I haz the power, 2016-04-11 19:15:09]

Failed to apply patch for ash/mus/BUILD.gn:
While running git apply --index -3 -p1;
  error: patch failed: ash/mus/BUILD.gn:40
  Falling back to three-way merge...
  Applied patch to 'ash/mus/BUILD.gn' with conflicts.
  U ash/mus/BUILD.gn

Patch:       ash/mus/BUILD.gn
Index: ash/mus/BUILD.gn
diff --git a/ash/mus/BUILD.gn b/ash/mus/BUILD.gn
index
d86ab228dd3a9660986c0891ad3176caff3e74f6..eeae61c92ea89f226d9a93f69ede7e2c3cf4c0e9
100644
--- a/ash/mus/BUILD.gn
+++ b/ash/mus/BUILD.gn
@@ -33,6 +33,7 @@ source_set("lib") {
     "//components/mus/public/interfaces",
     "//components/user_manager",
     "//device/bluetooth",
+    "//mash/shelf/public/interfaces",
     "//mash/wm/public/interfaces",
     "//mojo/common:common_base",
     "//mojo/converters/geometry",
@@ -40,6 +41,7 @@ source_set("lib") {
     "//mojo/services/tracing/public/cpp",
     "//mojo/shell/public/cpp",
     "//skia",
+    "//skia/public",
     "//ui/aura",
     "//ui/events",
     "//ui/events/devices",

[commit-bot: I haz the power, 2016-04-11 19:15:33]

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

[tomhudson, 2016-04-11 19:19:50]

+skia/public/ MOJOM LGTM.

At some time in the next couple of quarters we may push to change this from
using SkBitmap to SkImage if they're going to be immutable, but we've got bigger
fish to fry for now.

[msw, 2016-04-11 19:29:16]

Description was changed from

==========
Add basic Chrome interaction with the mash shelf.

Based on the "Mash Shelf Interface Proposal" doc:
https://docs.google.com/document/d/1FSSAxodmRbaeiHI2H-2WNfbVXPLHLcTbcThvln0bKNw

Add shelf.mojom with necessary interfaces and structs.
Make ShelfDelegateMus implement mash::shelf::mojom::Shelf.
Make SysUIApplication (mojo:ash_sysui) vend Shelf interfaces.

Init mash from ChromeBrowserMainExtraPartsAsh::PostProfileInit.
Add a test shortcut item to a fake application from Chrome.
Store local chrome prefs when mash shelf properties change.
Include required pref-related consts/files in USE_AURA.

Extract IsRunningInMash helper function.
Extract launcher preference conversion helper functions.

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

TODO: Support multiple displays with a shelf per display.
TODO: Support complex prefs in ChromeMashShelfController.
TODO: Link up user window and pinned shelf items.
TODO: Add more NOTIMPLEMENTED() functionality...

BUG=557406
TEST=Mash shelf has a new test item, stores prefs locally.
R=sky@chromium.org,jamescook@chromium.org
TBR=tomhudson@google.com
==========

to

==========
Add basic Chrome interaction with the mash shelf.

Based on the "Mash Shelf Interface Proposal" doc:
https://docs.google.com/document/d/1FSSAxodmRbaeiHI2H-2WNfbVXPLHLcTbcThvln0bKNw

Add shelf.mojom with necessary interfaces and structs.
Make ShelfDelegateMus implement mash::shelf::mojom::Shelf.
Make SysUIApplication (mojo:ash_sysui) vend Shelf interfaces.

Init mash from ChromeBrowserMainExtraPartsAsh::PostProfileInit.
Add a test shortcut item to a fake application from Chrome.
Store local chrome prefs when mash shelf properties change.
Include required pref-related consts/files in USE_AURA.

Extract IsRunningInMash helper function.
Extract launcher preference conversion helper functions.

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

TODO: Support multiple displays with a shelf per display.
TODO: Support complex prefs in ChromeMashShelfController.
TODO: Link up user window and pinned shelf items.
TODO: Add more NOTIMPLEMENTED() functionality...

BUG=557406
TEST=Mash shelf has a new test item, stores prefs locally.
R=sky@chromium.org,jamescook@chromium.org,tomhudson@google.com
==========

[msw, 2016-04-11 19:29:31]

The CQ bit was checked by msw@chromium.org

[msw, 2016-04-11 19:29:31]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org,
tomhudson@google.com
Link to the patchset: https://codereview.chromium.org/1839223003/#ps340001
(title: "Sync and rebase.")

[commit-bot: I haz the power, 2016-04-11 19:30:10]

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

[commit-bot: I haz the power, 2016-04-11 21:18:02]

Description was changed from

==========
Add basic Chrome interaction with the mash shelf.

Based on the "Mash Shelf Interface Proposal" doc:
https://docs.google.com/document/d/1FSSAxodmRbaeiHI2H-2WNfbVXPLHLcTbcThvln0bKNw

Add shelf.mojom with necessary interfaces and structs.
Make ShelfDelegateMus implement mash::shelf::mojom::Shelf.
Make SysUIApplication (mojo:ash_sysui) vend Shelf interfaces.

Init mash from ChromeBrowserMainExtraPartsAsh::PostProfileInit.
Add a test shortcut item to a fake application from Chrome.
Store local chrome prefs when mash shelf properties change.
Include required pref-related consts/files in USE_AURA.

Extract IsRunningInMash helper function.
Extract launcher preference conversion helper functions.

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

TODO: Support multiple displays with a shelf per display.
TODO: Support complex prefs in ChromeMashShelfController.
TODO: Link up user window and pinned shelf items.
TODO: Add more NOTIMPLEMENTED() functionality...

BUG=557406
TEST=Mash shelf has a new test item, stores prefs locally.
R=sky@chromium.org,jamescook@chromium.org,tomhudson@google.com
==========

to

==========
Add basic Chrome interaction with the mash shelf.

Based on the "Mash Shelf Interface Proposal" doc:
https://docs.google.com/document/d/1FSSAxodmRbaeiHI2H-2WNfbVXPLHLcTbcThvln0bKNw

Add shelf.mojom with necessary interfaces and structs.
Make ShelfDelegateMus implement mash::shelf::mojom::Shelf.
Make SysUIApplication (mojo:ash_sysui) vend Shelf interfaces.

Init mash from ChromeBrowserMainExtraPartsAsh::PostProfileInit.
Add a test shortcut item to a fake application from Chrome.
Store local chrome prefs when mash shelf properties change.
Include required pref-related consts/files in USE_AURA.

Extract IsRunningInMash helper function.
Extract launcher preference conversion helper functions.

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

TODO: Support multiple displays with a shelf per display.
TODO: Support complex prefs in ChromeMashShelfController.
TODO: Link up user window and pinned shelf items.
TODO: Add more NOTIMPLEMENTED() functionality...

BUG=557406
TEST=Mash shelf has a new test item, stores prefs locally.
R=sky@chromium.org,jamescook@chromium.org,tomhudson@google.com
==========

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

Committed patchset #13 (id:340001)

[commit-bot: I haz the power, 2016-04-11 21:18:57]

Description was changed from

==========
Add basic Chrome interaction with the mash shelf.

Based on the "Mash Shelf Interface Proposal" doc:
https://docs.google.com/document/d/1FSSAxodmRbaeiHI2H-2WNfbVXPLHLcTbcThvln0bKNw

Add shelf.mojom with necessary interfaces and structs.
Make ShelfDelegateMus implement mash::shelf::mojom::Shelf.
Make SysUIApplication (mojo:ash_sysui) vend Shelf interfaces.

Init mash from ChromeBrowserMainExtraPartsAsh::PostProfileInit.
Add a test shortcut item to a fake application from Chrome.
Store local chrome prefs when mash shelf properties change.
Include required pref-related consts/files in USE_AURA.

Extract IsRunningInMash helper function.
Extract launcher preference conversion helper functions.

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

TODO: Support multiple displays with a shelf per display.
TODO: Support complex prefs in ChromeMashShelfController.
TODO: Link up user window and pinned shelf items.
TODO: Add more NOTIMPLEMENTED() functionality...

BUG=557406
TEST=Mash shelf has a new test item, stores prefs locally.
R=sky@chromium.org,jamescook@chromium.org,tomhudson@google.com
==========

to

==========
Add basic Chrome interaction with the mash shelf.

Based on the "Mash Shelf Interface Proposal" doc:
https://docs.google.com/document/d/1FSSAxodmRbaeiHI2H-2WNfbVXPLHLcTbcThvln0bKNw

Add shelf.mojom with necessary interfaces and structs.
Make ShelfDelegateMus implement mash::shelf::mojom::Shelf.
Make SysUIApplication (mojo:ash_sysui) vend Shelf interfaces.

Init mash from ChromeBrowserMainExtraPartsAsh::PostProfileInit.
Add a test shortcut item to a fake application from Chrome.
Store local chrome prefs when mash shelf properties change.
Include required pref-related consts/files in USE_AURA.

Extract IsRunningInMash helper function.
Extract launcher preference conversion helper functions.

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

TODO: Support multiple displays with a shelf per display.
TODO: Support complex prefs in ChromeMashShelfController.
TODO: Link up user window and pinned shelf items.
TODO: Add more NOTIMPLEMENTED() functionality...

BUG=557406
TEST=Mash shelf has a new test item, stores prefs locally.
R=sky@chromium.org,jamescook@chromium.org,tomhudson@google.com

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

[commit-bot: I haz the power, 2016-04-11 21:18:59]

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