cros: Add more unit tests for internal app window

This cl adds more ChromeLauncherControllerTest to test the internal app
window creation, close, and change of window property.

Bug: 824437
Test: New ChromeLauncherControllerTest.
Change-Id: I914b2aa931cfe8c3f845c1017b73025848fe29b3
Reviewed-on: https://chromium-review.googlesource.com/1023180
Commit-Queue: Tao Wu <wutao@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553380}

chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc

@@ -20,6 +20,8 @@
 #include "ash/public/cpp/shelf_item_delegate.h"
 #include "ash/public/cpp/shelf_model.h"
 #include "ash/public/cpp/shelf_model_observer.h"
+#include "ash/public/cpp/shelf_types.h"
+#include "ash/public/cpp/window_properties.h"
 #include "ash/shelf/shelf_application_menu_model.h"
 #include "ash/shelf/shelf_constants.h"
 #include "ash/shelf/shelf_controller.h"
@@ -48,6 +50,7 @@
 #include "chrome/browser/ui/app_list/arc/arc_app_test.h"
 #include "chrome/browser/ui/app_list/arc/arc_app_utils.h"
 #include "chrome/browser/ui/app_list/arc/arc_default_app_list.h"
+#include "chrome/browser/ui/app_list/internal_app/internal_app_metadata.h"
 #include "chrome/browser/ui/apps/chrome_app_delegate.h"
 #include "chrome/browser/ui/ash/chrome_launcher_prefs.h"
 #include "chrome/browser/ui/ash/launcher/app_window_launcher_controller.h"
@@ -341,6 +344,23 @@ SkBitmap GetLastItemImage(TestShelfController* shelf_controller) {
   return *bitmap;
 }
 
+// Creates a window with TYPE_APP shelf item type and the given app_id.
+views::Widget* CreateShelfAppWindow(const std::string& app_id) {
+  views::Widget::InitParams params(views::Widget::InitParams::TYPE_WINDOW);
+  params.bounds = gfx::Rect(5, 5, 20, 20);
+  views::Widget* widget = new views::Widget();
+  widget->Init(params);
+
+  aura::Window* window = widget->GetNativeWindow();
+  const ash::ShelfID shelf_id(app_id);
+  window->SetProperty(ash::kShelfIDKey, new std::string(shelf_id.Serialize()));
+  window->SetProperty<int>(ash::kShelfItemTypeKey, ash::TYPE_APP);
+
+  widget->Show();
+  widget->Activate();
+  return widget;
+}
+
 }  // namespace
 
 // A test ChromeLauncherController subclass that uses TestShelfController.
@@ -4242,25 +4262,92 @@ TEST_F(ChromeLauncherControllerTest, ShelfItemImageSync) {
 }
 
 // Test the Settings can be pinned and unpinned.
-TEST_F(ChromeLauncherControllerTest, PinUnpinInternalApp) {
+TEST_F(ChromeLauncherControllerTest, InternalAppPinUnpin) {
   InitLauncherController();
   // The model should only contain the browser shortcut, app list and back
   // button items.
   EXPECT_EQ(3, model_->item_count());
-  EXPECT_FALSE(
-      launcher_controller_->IsAppPinned(app_list::kInternalAppIdSettings));
+
+  const std::string app_id = app_list::kInternalAppIdSettings;
+  EXPECT_FALSE(launcher_controller_->IsAppPinned(app_id));
 
   // Pin Settings.
-  launcher_controller_->PinAppWithID(app_list::kInternalAppIdSettings);
+  launcher_controller_->PinAppWithID(app_id);
   EXPECT_EQ(4, model_->item_count());
   EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[3].type);
   EXPECT_EQ(ash::STATUS_CLOSED, model_->items()[3].status);
-  EXPECT_TRUE(
-      launcher_controller_->IsAppPinned(app_list::kInternalAppIdSettings));
+  EXPECT_TRUE(launcher_controller_->IsAppPinned(app_id));
 
   // Unpin Settings.
-  launcher_controller_->UnpinAppWithID(app_list::kInternalAppIdSettings);
+  launcher_controller_->UnpinAppWithID(app_id);
   EXPECT_EQ(3, model_->item_count());
-  EXPECT_FALSE(
-      launcher_controller_->IsAppPinned(app_list::kInternalAppIdSettings));
+  EXPECT_FALSE(launcher_controller_->IsAppPinned(app_id));
+}
+
+// Test that internal app can be added and removed on shelf.
+TEST_F(ChromeLauncherControllerTest, InternalAppWindowRecreation) {
+  InitLauncherController();
+
+  // Only test the first internal app. The others should be the same.
+  const auto& internal_app = app_list::GetInternalAppList().front();
+  const std::string app_id = internal_app.app_id;
+  const ash::ShelfID shelf_id(app_id);
+  EXPECT_FALSE(launcher_controller_->GetItem(shelf_id));
+
+  views::Widget* internal_app_window = CreateShelfAppWindow(app_id);
+  ASSERT_TRUE(internal_app_window);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(launcher_controller_->GetItem(shelf_id));
+
+  internal_app_window->Close();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(launcher_controller_->GetItem(shelf_id));
+
+  // Create and close again.
+  internal_app_window = CreateShelfAppWindow(app_id);
+  ASSERT_TRUE(internal_app_window);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(launcher_controller_->GetItem(shelf_id));
+
+  internal_app_window->Close();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(launcher_controller_->GetItem(shelf_id));
+}
+
+// Test that internal app can be added and removed by SetProperty of
+// ash::kShelfIDKey.
+TEST_F(ChromeLauncherControllerTest, InternalAppWindowPropertyChanged) {
+  InitLauncherController();
+
+  // Only test the first internal app. The others should be the same.
+  const auto& internal_app = app_list::GetInternalAppList().front();
+  std::string app_id;
+  ash::ShelfID shelf_id;
+  EXPECT_FALSE(launcher_controller_->GetItem(shelf_id));
+
+  // Set an empty ash::kShelfIDKey.
+  views::Widget* internal_app_window = CreateShelfAppWindow(app_id);
+  ASSERT_TRUE(internal_app_window);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(launcher_controller_->GetItem(shelf_id));
+
+  // Set an invalid ash::kShelfIDKey.
+  app_id = "An invalid internal app id";
+  shelf_id = ash::ShelfID(app_id);
+  internal_app_window->GetNativeWindow()->SetProperty(
+      ash::kShelfIDKey, new std::string(shelf_id.Serialize()));
+  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(launcher_controller_->GetItem(shelf_id));
+
+  // Set a valid ash::kShelfIDKey.
+  app_id = internal_app.app_id;
+  shelf_id = ash::ShelfID(app_id);
+  internal_app_window->GetNativeWindow()->SetProperty(
+      ash::kShelfIDKey, new std::string(shelf_id.Serialize()));
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(launcher_controller_->GetItem(shelf_id));
+
+  internal_app_window->Close();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(launcher_controller_->GetItem(shelf_id));
 }

chrome/browser/ui/ash/launcher/internal_app_window_shelf_controller.cc

@@ -59,7 +59,7 @@ void InternalAppWindowShelfController::OnWindowPropertyChanged(
   ash::ShelfID old_shelf_id =
       ash::ShelfID::Deserialize(reinterpret_cast<std::string*>(old));
   if (!old_shelf_id.IsNull() && app_list::IsInternalApp(old_shelf_id.app_id))
-    DelegetAppWindow(old_shelf_id);
+    DeleteAppWindow(old_shelf_id);
 
   ash::ShelfID shelf_id =
       ash::ShelfID::Deserialize(window->GetProperty(ash::kShelfIDKey));
@@ -91,7 +91,7 @@ void InternalAppWindowShelfController::OnWindowDestroying(
 
   ash::ShelfID shelf_id =
       ash::ShelfID::Deserialize(window->GetProperty(ash::kShelfIDKey));
-  if (!DelegetAppWindow(shelf_id))
+  if (!DeleteAppWindow(shelf_id))
     return;
 
   // Check if we may close controller now, at this point we can safely remove
@@ -149,7 +149,7 @@ void InternalAppWindowShelfController::UnregisterAppWindow(
   app_window->SetController(nullptr);
 }
 
-bool InternalAppWindowShelfController::DelegetAppWindow(
+bool InternalAppWindowShelfController::DeleteAppWindow(
     const ash::ShelfID& shelf_id) {
   auto app_window_it = shelf_id_to_app_window_.find(shelf_id);
   if (app_window_it == shelf_id_to_app_window_.end())

chrome/browser/ui/ash/launcher/internal_app_window_shelf_controller.h

@@ -54,8 +54,8 @@ class InternalAppWindowShelfController : public AppWindowLauncherController,
   void UnregisterAppWindow(AppWindowBase* app_window);
 
   // Deletes an AppWindow.
-  // Returns true if an AppWindow of |shelf_id| exits, otherwise returns false.
-  bool DelegetAppWindow(const ash::ShelfID& shelf_id);
+  // Returns true if an AppWindow of |shelf_id| exists, otherwise returns false.
+  bool DeleteAppWindow(const ash::ShelfID& shelf_id);
 
   // AppWindowLauncherController:
   AppWindowLauncherItemController* ControllerForWindow(