Add mash shelf application id support.

ash/mus/shelf_delegate_mus.cc

Index: ash/mus/shelf_delegate_mus.cc
diff --git a/ash/mus/shelf_delegate_mus.cc b/ash/mus/shelf_delegate_mus.cc
index 8bb11ad68fac4ad7108fce21c4fafa8d8b9d5ae1..b2ed23004703a1cfee9650c53b16ea705f687148 100644
--- a/ash/mus/shelf_delegate_mus.cc
+++ b/ash/mus/shelf_delegate_mus.cc
@@ -10,6 +10,7 @@
 #include "ash/shelf/shelf_item_delegate.h"
 #include "ash/shelf/shelf_item_delegate_manager.h"
 #include "ash/shelf/shelf_layout_manager.h"
+#include "ash/shelf/shelf_menu_model.h"
 #include "ash/shelf/shelf_model.h"
 #include "ash/shelf/shelf_types.h"
 #include "ash/shelf/shelf_widget.h"
@@ -37,38 +38,86 @@ namespace {
 // A ShelfItemDelegate used for pinned items and open user windows.
 class ShelfItemDelegateMus : public ShelfItemDelegate {
  public:
-  ShelfItemDelegateMus(uint32_t window_id,
-                       const base::string16& title,
-                       UserWindowController* user_window_controller)
-      : window_id_(window_id),
-        title_(title),
-        user_window_controller_(user_window_controller) {}
+  explicit ShelfItemDelegateMus(UserWindowController* user_window_controller)
+      : user_window_controller_(user_window_controller) {}
   ~ShelfItemDelegateMus() override {}
 
-  void UpdateTitle(const base::string16& new_title) { title_ = new_title; }
+  bool pinned() const { return pinned_; }
+  void set_pinned(bool pinned) { pinned_ = pinned; }
+
+  void AddWindow(uint32_t id, const base::string16& title) {
+    DCHECK(!windows_.count(id));
+    windows_.insert(std::make_pair(id, title));
+  }
+  void RemoveWindow(uint32_t id) { windows_.erase(id); }
+  void SetWindowTitle(uint32_t id, const base::string16& title) {
+    DCHECK(windows_.count(id));
+    windows_[id] = title;
+  }
+  const std::map<uint32_t, base::string16>& windows() const { return windows_; }
+
+  const base::string16& title() { return title_; }
+  void set_title(const base::string16& title) { title_ = title; }
 
  private:
+  // This application menu model for ShelfItemDelegateMus lists open windows.
+  class ShelfMenuModelMus : public ShelfMenuModel,
+                            public ui::SimpleMenuModel::Delegate {
+   public:
+    explicit ShelfMenuModelMus(ShelfItemDelegateMus* item_delegate)
+        : ShelfMenuModel(this), item_delegate_(item_delegate) {
+      AddSeparator(ui::SPACING_SEPARATOR);
+      AddItem(0, item_delegate_->GetTitle());
+      AddSeparator(ui::SPACING_SEPARATOR);
+      for (auto window : item_delegate_->windows())

[sky, 2016/04/22 15:51:19]

const auto&

[msw, 2016/04/22 19:02:17]

Done.

+        AddItem(window.first, window.second);
+      AddSeparator(ui::SPACING_SEPARATOR);
+    }
+    ~ShelfMenuModelMus() override {}
+
+    // ShelfMenuModel:
+    bool IsCommandActive(int command_id) const override { return false; }
+
+    // ui::SimpleMenuModel::Delegate:
+    bool IsCommandIdChecked(int command_id) const override { return false; }
+    bool IsCommandIdEnabled(int command_id) const override {
+      return command_id > 0;
+    }
+    bool GetAcceleratorForCommandId(int command_id,
+                                    ui::Accelerator* accelerator) override {
+      return false;
+    }
+    void ExecuteCommand(int command_id, int event_flags) override {
+      item_delegate_->user_window_controller_->FocusUserWindow(command_id);
+    }
+
+   private:
+    ShelfItemDelegateMus* item_delegate_;
+
+    DISALLOW_COPY_AND_ASSIGN(ShelfMenuModelMus);
+  };
+
   // ShelfItemDelegate:
   ShelfItemDelegate::PerformedAction ItemSelected(
       const ui::Event& event) override {
-    if (window_id_ != 0) {
-      user_window_controller_->FocusUserWindow(window_id_);
+    if (windows_.size() == 1) {
+      user_window_controller_->FocusUserWindow(windows_.begin()->first);
       return kExistingWindowActivated;
     }
-    NOTIMPLEMENTED();
     return kNoAction;
   }
 
-  base::string16 GetTitle() override { return title_; }
+  base::string16 GetTitle() override {
+    return windows_.empty() ? title_ : windows_.begin()->second;
+  }
 
   bool CanPin() const override {
     NOTIMPLEMENTED();
-    return false;
+    return true;
   }
 
   ShelfMenuModel* CreateApplicationMenu(int event_flags) override {
-    NOTIMPLEMENTED();
-    return nullptr;
+    return new ShelfMenuModelMus(this);
   }
 
   bool IsDraggable() override {
@@ -80,14 +129,20 @@ class ShelfItemDelegateMus : public ShelfItemDelegate {
 
   void Close() override { NOTIMPLEMENTED(); }
 
-  // TODO(msw): Support multiple open windows per button.
-  uint32_t window_id_;
+  bool pinned_ = false;
+  std::map<uint32_t, base::string16> windows_;

[sky, 2016/04/22 15:51:19]

This name is mildly confusing as as it's really window id to title. I'm ok with
windows, but at least document what the mapping is.

[msw, 2016/04/22 19:02:17]

Renamed, clarity in identifier names is better than explanatory comments.

   base::string16 title_;
   UserWindowController* user_window_controller_;
 
   DISALLOW_COPY_AND_ASSIGN(ShelfItemDelegateMus);
 };
 
+ShelfItemDelegateMus* GetShelfItemDelegate(ShelfID shelf_id) {
+  return static_cast<ShelfItemDelegateMus*>(
+      Shell::GetInstance()->shelf_item_delegate_manager()->GetShelfItemDelegate(
+          shelf_id));
+}
+
 // Returns an icon image from an SkBitmap, or the default shelf icon
 // image if the bitmap is empty. Assumes the bitmap is a 1x icon.
 // TODO(jamescook): Support other scale factors.
@@ -210,31 +265,42 @@ void ShelfDelegateMus::SetAutoHideBehavior(
                                                  Shell::GetPrimaryRootWindow());
 }
 
-void ShelfDelegateMus::AddItem(
+void ShelfDelegateMus::PinItem(
     mash::shelf::mojom::ShelfItemPtr item,
     mash::shelf::mojom::ShelfItemDelegateAssociatedPtrInfo delegate) {
+  std::string app_id(item->app_id.To<std::string>());
+  if (app_id_to_shelf_id_.count(app_id)) {
+    ShelfID shelf_id = app_id_to_shelf_id_[app_id];
+    ShelfItemDelegateMus* item_delegate = GetShelfItemDelegate(shelf_id);
+    item_delegate->set_pinned(true);
+    return;
+  }
+
+  ShelfID shelf_id = model_->next_id();
+  app_id_to_shelf_id_.insert(std::make_pair(app_id, shelf_id));
+
   ShelfItem shelf_item;
   shelf_item.type = TYPE_APP_SHORTCUT;
   shelf_item.status = STATUS_CLOSED;
   shelf_item.image = GetShelfIconFromBitmap(item->image.To<SkBitmap>());
-
-  std::string item_id(item->id.To<std::string>());
-  if (app_id_to_shelf_id_.count(item_id))
-    return;
-  ShelfID shelf_id = model_->next_id();
-  app_id_to_shelf_id_.insert(std::make_pair(item_id, shelf_id));
   model_->Add(shelf_item);
 
-  std::unique_ptr<ShelfItemDelegateMus> item_delegate(new ShelfItemDelegateMus(
-      0, item->title.To<base::string16>(), user_window_controller_.get()));
+  std::unique_ptr<ShelfItemDelegateMus> item_delegate(
+      new ShelfItemDelegateMus(user_window_controller_.get()));
+  item_delegate->set_pinned(true);
+  item_delegate->set_title(item->app_title.To<base::string16>());
   Shell::GetInstance()->shelf_item_delegate_manager()->SetShelfItemDelegate(
       shelf_id, std::move(item_delegate));
 }
 
-void ShelfDelegateMus::RemoveItem(const mojo::String& id) {
-  std::string item_id(id.To<std::string>());
-  DCHECK(app_id_to_shelf_id_.count(item_id));
-  model_->RemoveItemAt(model_->ItemIndexByID(app_id_to_shelf_id_[item_id]));
+void ShelfDelegateMus::UnpinItem(const mojo::String& app_id) {
+  DCHECK(app_id_to_shelf_id_.count(app_id.To<std::string>()));

[sky, 2016/04/22 15:51:19]

Don't trust a client to do the right thing with mojoms. In other words ignore
the client if the app_id is bogus (and possibly log).

Also, do we want to allow one client to pin/unpin another clients items?

[msw, 2016/04/22 19:02:17]

Added early return. I think this is probably okay for now; I don't anticipate
bad mojo actors in the chromium codebase making their way into the shelf
controller interface (ie. we don't protect against
Shell::GetInstance()->shelf_model()->RemoveItemAt(index)). If we do want some
added security, maybe we could require a ShelfItemDelegateAssociatedPtrInfo
(like PinItem) and require that the it be the same object? (not sure if that's
actually possible)

+  ShelfID shelf_id = app_id_to_shelf_id_[app_id.To<std::string>()];
+  ShelfItemDelegateMus* item_delegate = GetShelfItemDelegate(shelf_id);
+  DCHECK(item_delegate->pinned());
+  item_delegate->set_pinned(false);
+  if (item_delegate->windows().empty())
+    model_->RemoveItemAt(model_->ItemIndexByID(shelf_id));
 }
 
 void ShelfDelegateMus::OnUserWindowObserverAdded(
@@ -245,27 +311,47 @@ void ShelfDelegateMus::OnUserWindowObserverAdded(
 
 void ShelfDelegateMus::OnUserWindowAdded(
     mash::wm::mojom::UserWindowPtr user_window) {
-  ShelfItem item;
-  item.type = TYPE_PLATFORM_APP;
-  item.status = user_window->window_has_focus ? STATUS_ACTIVE : STATUS_RUNNING;
-  item.image = GetShelfIconFromSerializedBitmap(user_window->window_app_icon);
+  DCHECK(!window_id_to_shelf_id_.count(user_window->window_id));
+
+  std::string app_id(user_window->window_app_id.To<std::string>());
+  if (app_id_to_shelf_id_.count(app_id)) {
+    ShelfID shelf_id = app_id_to_shelf_id_[app_id];
+    window_id_to_shelf_id_.insert(
+        std::make_pair(user_window->window_id, shelf_id));
+
+    ShelfItemDelegateMus* item_delegate = GetShelfItemDelegate(shelf_id);
+    item_delegate->AddWindow(user_window->window_id,
+                             user_window->window_title.To<base::string16>());
+    return;
+  }
 
   ShelfID shelf_id = model_->next_id();
   window_id_to_shelf_id_.insert(
       std::make_pair(user_window->window_id, shelf_id));
+  app_id_to_shelf_id_.insert(std::make_pair(app_id, shelf_id));
+
+  ShelfItem item;
+  item.type = TYPE_PLATFORM_APP;
+  item.status = user_window->window_has_focus ? STATUS_ACTIVE : STATUS_RUNNING;
+  item.image = GetShelfIconFromSerializedBitmap(user_window->window_app_icon);
   model_->Add(item);
 
-  std::unique_ptr<ShelfItemDelegate> item_delegate(new ShelfItemDelegateMus(
-      user_window->window_id, user_window->window_title.To<base::string16>(),
-      user_window_controller_.get()));
+  std::unique_ptr<ShelfItemDelegateMus> item_delegate(
+      new ShelfItemDelegateMus(user_window_controller_.get()));
+  item_delegate->AddWindow(user_window->window_id,
+                           user_window->window_title.To<base::string16>());
   Shell::GetInstance()->shelf_item_delegate_manager()->SetShelfItemDelegate(
       shelf_id, std::move(item_delegate));
 }
 
 void ShelfDelegateMus::OnUserWindowRemoved(uint32_t window_id) {
   DCHECK(window_id_to_shelf_id_.count(window_id));
-  model_->RemoveItemAt(
-      model_->ItemIndexByID(window_id_to_shelf_id_[window_id]));
+  ShelfID shelf_id = window_id_to_shelf_id_[window_id];
+  ShelfItemDelegateMus* item_delegate = GetShelfItemDelegate(shelf_id);
+  item_delegate->RemoveWindow(window_id);
+  window_id_to_shelf_id_.erase(window_id);
+  if (item_delegate->windows().empty() && !item_delegate->pinned())
+    model_->RemoveItemAt(model_->ItemIndexByID(shelf_id));
 }
 
 void ShelfDelegateMus::OnUserWindowTitleChanged(
@@ -273,12 +359,8 @@ void ShelfDelegateMus::OnUserWindowTitleChanged(
     const mojo::String& window_title) {
   DCHECK(window_id_to_shelf_id_.count(window_id));
   ShelfID shelf_id = window_id_to_shelf_id_[window_id];
-  ShelfItemDelegateManager* manager =
-      Shell::GetInstance()->shelf_item_delegate_manager();
-  ShelfItemDelegate* delegate = manager->GetShelfItemDelegate(shelf_id);
-  DCHECK(delegate);
-  static_cast<ShelfItemDelegateMus*>(delegate)->UpdateTitle(
-      window_title.To<base::string16>());
+  ShelfItemDelegateMus* item_delegate = GetShelfItemDelegate(shelf_id);
+  item_delegate->SetWindowTitle(window_id, window_title.To<base::string16>());
 
   // There's nothing in the ShelfItem that needs to be updated. But we still
   // need to update the ShelfModel so that the observers can pick up any