arc: Set custom icon in shelf for ARC apps.

chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.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 "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <map>
@@ skipping 40 matching lines @@
 #include "chrome/browser/prefs/browser_prefs.h"
 #include "chrome/browser/ui/app_list/app_list_syncable_service_factory.h"
 #include "chrome/browser/ui/app_list/arc/arc_app_list_prefs.h"
 #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/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"
 #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"
 #include "chrome/browser/ui/ash/launcher/launcher_controller_helper.h"
 #include "chrome/browser/ui/ash/multi_user/multi_user_util.h"
 #include "chrome/browser/ui/ash/multi_user/multi_user_window_manager.h"
 #include "chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_finder.h"
@@ skipping 2210 matching lines @@
 
   EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id()));
   EXPECT_TRUE(launcher_controller_->IsAppPinned(arc_app_id1));
   EXPECT_TRUE(launcher_controller_->IsAppPinned(extension2_->id()));
   EXPECT_TRUE(launcher_controller_->IsAppPinned(arc_app_id2));
 
   EXPECT_EQ("AppList, Chrome, App1, Fake App 1, App2, Fake App 0",
             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,
        V1AppUpdateOnUserSwitch) {
   // Create a browser item in the LauncherController.
   InitLauncherController();
 
   EXPECT_EQ(2, model_->item_count());
   {
     // Create a "windowed gmail app".
@@ skipping 1914 matching lines @@
   EXPECT_EQ(ash::SHELF_AUTO_HIDE_BEHAVIOR_ALWAYS,
             shelf_controller->auto_hide());
   EXPECT_EQ(2u, shelf_controller->auto_hide_change_count());
 
   PrefService* prefs = profile()->GetTestingPrefService();
   EXPECT_EQ("Left", prefs->GetString(prefs::kShelfAlignmentLocal));
   EXPECT_EQ("Left", prefs->GetString(prefs::kShelfAlignment));
   EXPECT_EQ("Always", prefs->GetString(prefs::kShelfAutoHideBehaviorLocal));
   EXPECT_EQ("Always", prefs->GetString(prefs::kShelfAutoHideBehavior));
 }