mash: Replace int ShelfIDs with AppLaunchID strings.

ash/shelf/shelf_window_watcher.cc

Index: ash/shelf/shelf_window_watcher.cc
diff --git a/ash/shelf/shelf_window_watcher.cc b/ash/shelf/shelf_window_watcher.cc
index 6964bb12cbeb31115fc51aa6e76aafa387609697..8042524da8a3b8a812f6541f9c43010dfe42d6f7 100644
--- a/ash/shelf/shelf_window_watcher.cc
+++ b/ash/shelf/shelf_window_watcher.cc
@@ -15,7 +15,6 @@
 #include "ash/shelf/shelf_window_watcher_item_delegate.h"
 #include "ash/shell.h"
 #include "ash/shell_port.h"
-#include "ash/wm/window_properties.h"
 #include "ash/wm/window_state.h"
 #include "ash/wm/window_state_aura.h"
 #include "ash/wm/window_util.h"
@@ -52,8 +51,8 @@ void UpdateShelfItemForWindow(ShelfItem* item, aura::Window* window) {
   else if (window->GetProperty(aura::client::kDrawAttentionKey))
     item->status = STATUS_ATTENTION;
 
-  const std::string* app_id = window->GetProperty(aura::client::kAppIdKey);
-  item->app_launch_id = app_id ? AppLaunchId(*app_id) : AppLaunchId();
+  const ShelfID* shelf_id = window->GetProperty(kShelfIDKey);
+  item->id = shelf_id ? *shelf_id : ShelfID();
 
   // Prefer app icons over window icons, they're typically larger.
   gfx::ImageSkia* image = window->GetProperty(aura::client::kAppIconKey);
@@ -108,10 +107,9 @@ void ShelfWindowWatcher::UserWindowObserver::OnWindowPropertyChanged(
     aura::Window* window,
     const void* key,
     intptr_t old) {
-  if (key == aura::client::kAppIconKey || key == aura::client::kAppIdKey ||
-      key == aura::client::kDrawAttentionKey ||
-      key == aura::client::kWindowIconKey || key == kPanelAttachedKey ||
-      key == kShelfItemTypeKey) {
+  if (key == aura::client::kAppIconKey || key == aura::client::kWindowIconKey ||

[James Cook, 2017/05/04 16:38:49]

super nit: is there anything special about the order here? if not, consider
alphabetizing

[msw, 2017/05/04 19:05:57]

I put the two icons together to wrap a bit nicer, but I can alphabetize if you
prefer. I don't see this list growing by much, if that matters.

+      key == aura::client::kDrawAttentionKey || key == kPanelAttachedKey ||
+      key == kShelfItemTypeKey || key == kShelfIDKey) {
     window_watcher_->OnUserWindowPropertyChanged(window);
   }
 }
@@ -160,24 +158,22 @@ ShelfWindowWatcher::~ShelfWindowWatcher() {
 void ShelfWindowWatcher::AddShelfItem(aura::Window* window) {
   user_windows_with_items_.insert(window);
   ShelfItem item;
-  ShelfID id = model_->next_id();
   UpdateShelfItemForWindow(&item, window);
-  window->SetProperty(kShelfIDKey, id);
-  model_->SetShelfItemDelegate(id,
+  model_->SetShelfItemDelegate(item.id,
                                base::MakeUnique<ShelfWindowWatcherItemDelegate>(
-                                   id, WmWindow::Get(window)));
+                                   item.id, WmWindow::Get(window)));
   // Panels are inserted on the left so as not to push all existing panels over.
   model_->AddAt(item.type == TYPE_APP_PANEL ? 0 : model_->item_count(), item);
 }
 
 void ShelfWindowWatcher::RemoveShelfItem(aura::Window* window) {
   user_windows_with_items_.erase(window);
-  ShelfID shelf_id = window->GetProperty(kShelfIDKey);
-  DCHECK_NE(shelf_id, kInvalidShelfID);
-  int index = model_->ItemIndexByID(shelf_id);
+  ShelfID* shelf_id = window->GetProperty(kShelfIDKey);
+  DCHECK(shelf_id);
+  DCHECK(!shelf_id->IsEmpty());
+  int index = model_->ItemIndexByID(*shelf_id);
   DCHECK_GE(index, 0);
   model_->RemoveItemAt(index);
-  window->SetProperty(kShelfIDKey, kInvalidShelfID);

[James Cook, 2017/05/04 16:38:49]

Nice that we don't have to wipe the shelf ID anymore.

[msw, 2017/05/04 19:05:57]

Agreed.

 }
 
 void ShelfWindowWatcher::OnContainerWindowDestroying(aura::Window* container) {
@@ -185,7 +181,8 @@ void ShelfWindowWatcher::OnContainerWindowDestroying(aura::Window* container) {
 }
 
 int ShelfWindowWatcher::GetShelfItemIndexForWindow(aura::Window* window) const {
-  return model_->ItemIndexByID(window->GetProperty(kShelfIDKey));
+  ShelfID* shelf_id = window->GetProperty(kShelfIDKey);
+  return shelf_id ? model_->ItemIndexByID(*shelf_id) : -1;
 }
 
 void ShelfWindowWatcher::OnUserWindowAdded(aura::Window* window) {
@@ -225,8 +222,9 @@ void ShelfWindowWatcher::OnUserWindowPropertyChanged(aura::Window* window) {
     return;
   }
 
-  // Creates a new ShelfItem for |window|.
-  AddShelfItem(window);
+  // Creates a new ShelfItem for |window| if it has a non-empty ShelfID.
+  if (window->GetProperty(kShelfIDKey))

[James Cook, 2017/05/04 16:38:49]

maybe move this up and/or into the test for TYPE_UNDEFINED?

Will we be in a good state if a buggy app unexpectedly wipes its kShelfID window
property?

[msw, 2017/05/04 19:05:57]

Yeah, I guess if the id is cleared we should probably destroy the item.

+    AddShelfItem(window);
 }
 
 void ShelfWindowWatcher::OnWindowActivated(ActivationReason reason,