arc: Set custom icon in shelf for ARC apps.

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

Index: chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc
diff --git a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc
index 9e55ff8348cf817dcce6cec635815268762ffc3e..9a12ff88d7110836a57da327b51cf0d306d15fca 100644
--- a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc
+++ b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc
@@ -58,6 +58,8 @@
 #include "chrome/browser/ui/ash/chrome_launcher_prefs.h"
 #include "chrome/browser/ui/ash/launcher/app_window_launcher_controller.h"
 #include "chrome/browser/ui/ash/launcher/arc_app_deferred_launcher_controller.h"
+#include "chrome/browser/ui/ash/launcher/arc_app_window.h"
+#include "chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.h"
 #include "chrome/browser/ui/ash/launcher/browser_status_monitor.h"
 #include "chrome/browser/ui/ash/launcher/chrome_launcher_controller_util.h"
 #include "chrome/browser/ui/ash/launcher/extension_app_window_launcher_item_controller.h"
@@ -2288,6 +2290,72 @@ TEST_P(ChromeLauncherControllerWithArcTest, ArcAppPinOptOutOptIn) {
             GetPinnedAppStatus());
 }
 
+TEST_P(ChromeLauncherControllerWithArcTest, ArcCustomAppIcon) {
+  InitLauncherController();
+
+  ArcAppIcon::DisableSafeDecodingForTesting();
+  ArcAppWindow::DisableSafeIconDecodingForTesting();

[msw, 2017/05/18 18:27:00]

Would it be reasonable for ArcAppWindow to check the setting on ArcAppIcon, and
just have one of these flags, instead of needing to set both?

[khmel, 2017/05/18 20:27:52]

Added getter to ArcAppIcon

+
+  const std::string arc_app_id = ArcAppTest::GetAppId(arc_test_.fake_apps()[0]);
+  SendListOfArcApps();
+
+  std::string png_data;

[msw, 2017/05/18 18:27:00]

nit: add more comments to explain what these first groupings of statements do?

[khmel, 2017/05/18 20:27:52]

Done.

+  EXPECT_TRUE(arc_test_.app_instance()->GenerateAndSendIcon(
+      arc_test_.fake_apps()[0], arc::mojom::ScaleFactor::SCALE_FACTOR_300P,
+      &png_data));
+  EXPECT_FALSE(png_data.empty());
+  std::string invalid_png_data("aaaaaa");
+
+  EXPECT_FALSE(launcher_controller_->GetItem(ash::ShelfID(arc_app_id)));
+  std::string window_app_id1("org.chromium.arc.1");
+  std::string window_app_id2("org.chromium.arc.2");
+  views::Widget* window1 = CreateArcWindow(window_app_id1);
+  ASSERT_TRUE(window1 && window1->GetNativeWindow());
+  arc_test_.app_instance()->SendTaskCreated(1, arc_test_.fake_apps()[0],
+                                            std::string());
+
+  views::Widget* window2 = CreateArcWindow(window_app_id2);
+  ASSERT_TRUE(window2 && window2->GetNativeWindow());
+  arc_test_.app_instance()->SendTaskCreated(2, arc_test_.fake_apps()[0],
+                                            std::string());
+  EXPECT_TRUE(launcher_controller_->GetItem(ash::ShelfID(arc_app_id)));
+  ash::ShelfItemDelegate* item_delegate =
+      model_->GetShelfItemDelegate(ash::ShelfID(arc_app_id));
+  ASSERT_TRUE(item_delegate);
+
+  // No custom icon set. Acitivating windows should not change icon.
+  EXPECT_FALSE(item_delegate->image_set_by_controller());
+  window1->Activate();
+  EXPECT_FALSE(item_delegate->image_set_by_controller());
+  window2->Activate();
+  EXPECT_FALSE(item_delegate->image_set_by_controller());
+
+  // Set custom icon on active item. Icon should be change to custom.

[msw, 2017/05/18 18:27:00]

nit: 'be changed' or just 'should change'

[khmel, 2017/05/18 20:27:52]

Done.

+  arc_test_.app_instance()->SendTaskDescription(2, std::string(), png_data);
+  EXPECT_TRUE(item_delegate->image_set_by_controller());
+
+  // Switch back to the item without custom icon. Icon should be changed to
+  // default.
+  window1->Activate();
+  EXPECT_FALSE(item_delegate->image_set_by_controller());
+  // Test that setting of invalid icon resets the custom icon.

[msw, 2017/05/18 18:27:00]

nit: 'setting an'

[khmel, 2017/05/18 20:27:52]

Done.

+  arc_test_.app_instance()->SendTaskDescription(1, std::string(), png_data);
+  EXPECT_TRUE(item_delegate->image_set_by_controller());
+  arc_test_.app_instance()->SendTaskDescription(1, std::string(),
+                                                invalid_png_data);
+  EXPECT_FALSE(item_delegate->image_set_by_controller());
+
+  // Check window removing with active custom icon.

[msw, 2017/05/18 18:27:00]

nit: move comment down to where you're actually removing a window and change
this comment to what's actually happening in the next two lines.

[khmel, 2017/05/18 20:27:52]

Done.

+  arc_test_.app_instance()->SendTaskDescription(1, std::string(), png_data);
+  EXPECT_TRUE(item_delegate->image_set_by_controller());
+  // Reseting custom icon of inactive window doesn't reset shefl icon.

[msw, 2017/05/18 18:27:00]

nit: "shelf"

[khmel, 2017/05/18 20:27:52]

Done.

+  arc_test_.app_instance()->SendTaskDescription(2, std::string(),
+                                                std::string());
+  EXPECT_TRUE(item_delegate->image_set_by_controller());
+  window1->CloseNow();
+  EXPECT_FALSE(item_delegate->image_set_by_controller());
+}
+
 // Check that with multi profile V1 apps are properly added / removed from the
 // shelf.
 TEST_F(MultiProfileMultiBrowserShelfLayoutChromeLauncherControllerTest,