mash: Replace int ShelfIDs with AppLaunchID strings.

ash/shelf/shelf_window_watcher.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ash/shelf/shelf_window_watcher.h"
 
 #include <memory>
 #include <utility>
 
 #include "ash/public/cpp/config.h"
 #include "ash/public/cpp/shell_window_ids.h"
 #include "ash/public/cpp/window_properties.h"
 #include "ash/shelf/shelf_constants.h"
 #include "ash/shelf/shelf_model.h"
 #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"
 #include "ash/wm_window.h"
 #include "ui/aura/client/aura_constants.h"
 #include "ui/aura/window.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
 #include "ui/resources/grit/ui_resources.h"
@@ skipping 16 matching lines @@
 // Update the ShelfItem from relevant window properties.
 void UpdateShelfItemForWindow(ShelfItem* item, aura::Window* window) {
   item->type = GetShelfItemType(window);
 
   item->status = STATUS_RUNNING;
   if (wm::IsActiveWindow(window))
     item->status = STATUS_ACTIVE;
   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);
   if (!image || image->isNull())
     image = window->GetProperty(aura::client::kWindowIconKey);
   if (!image || image->isNull()) {
     ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
     item->image = rb.GetImageNamed(IDR_DEFAULT_FAVICON_32).AsImageSkia();
   } else {
     item->image = *image;
@@ skipping 34 matching lines @@
 ShelfWindowWatcher::UserWindowObserver::UserWindowObserver(
     ShelfWindowWatcher* window_watcher)
     : window_watcher_(window_watcher) {}
 
 ShelfWindowWatcher::UserWindowObserver::~UserWindowObserver() {}
 
 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);
   }
 }
 
 void ShelfWindowWatcher::UserWindowObserver::OnWindowDestroying(
     aura::Window* window) {
   window_watcher_->OnUserWindowDestroying(window);
 }
 
 void ShelfWindowWatcher::UserWindowObserver::OnWindowVisibilityChanged(
@@ skipping 28 matching lines @@
 }
 
 ShelfWindowWatcher::~ShelfWindowWatcher() {
   display::Screen::GetScreen()->RemoveObserver(this);
   Shell::Get()->activation_client()->RemoveObserver(this);
 }
 
 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) {
   observed_container_windows_.Remove(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) {
   // The window may already be tracked from a prior display or parent container.
   if (observed_user_windows_.IsObserving(window))
     return;
 
   observed_user_windows_.Add(window);
 
   // Add, update, or remove a ShelfItem for |window|, as needed.
@@ skipping 19 matching lines @@
 
   // Update an existing ShelfItem for |window| when a property has changed.
   int index = GetShelfItemIndexForWindow(window);
   if (index > 0) {
     ShelfItem item = model_->items()[index];
     UpdateShelfItemForWindow(&item, window);
     model_->Set(index, item);
     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,
                                            aura::Window* gained_active,
                                            aura::Window* lost_active) {
   if (gained_active && user_windows_with_items_.count(gained_active) > 0)
     OnUserWindowPropertyChanged(gained_active);
   if (lost_active && user_windows_with_items_.count(lost_active) > 0)
     OnUserWindowPropertyChanged(lost_active);
 }
@@ skipping 21 matching lines @@
   }
 }
 
 void ShelfWindowWatcher::OnDisplayRemoved(const display::Display& old_display) {
 }
 
 void ShelfWindowWatcher::OnDisplayMetricsChanged(const display::Display&,
                                                  uint32_t) {}
 
 }  // namespace ash