Fix the flaky browser test LauncherPlatformAppBrowserTest.SetIcon

AppService is used to load icons for windows. AppService uses async
mojom. So it might be not very appropriate to test with update number
when we have the custom icon, because if the icon is updated fast from
AppService, then it might be updated 4 times, however if slow, then
there are 3 updates only, becuase it has got the customer icon, and the
icon update from AppService could be skipped.

Modify the test case LauncherPlatformAppBrowserTest.SetIcon to check
the final image. If the custom icon is set, check the window's icon is
set as the custom icon.

BUG=1115533

Change-Id: Ief034842fef9360a49a504effb54cf51c8d4c51a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2364156Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Nancy Wang <nancylingwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800144}

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

@@ -887,6 +887,9 @@ IN_PROC_BROWSER_TEST_F(LauncherPlatformAppBrowserTest, SetIcon) {
 
   gfx::ImageSkia image_skia;
   if (base::FeatureList::IsEnabled(features::kAppServiceAdaptiveIcon)) {
+    // Only when the kAppServiceAdaptiveIcon feature is enabled, AppService is
+    // called to load icons for windows. So the image checking is available when
+    // the kAppServiceAdaptiveIcon feature is enabled.
     int32_t size_hint_in_dip = 48;
     image_skia = app_service_test().LoadAppIconBlocking(
         apps::mojom::AppType::kExtension, extension->id(), size_hint_in_dip);
@@ -928,18 +931,28 @@ IN_PROC_BROWSER_TEST_F(LauncherPlatformAppBrowserTest, SetIcon) {
   ready_listener.Reset();
   // Custom icon update.
   test_observer.WaitForIconUpdate();
+  if (base::FeatureList::IsEnabled(features::kAppServiceAdaptiveIcon)) {
+    EXPECT_FALSE(app_service_test().AreIconImageEqual(
+        image_skia, test_observer.last_app_icon()));
+  }
+  gfx::ImageSkia custome_icon = test_observer.last_app_icon();
 
   // Create shelf window with custom icon on init.
   EXPECT_TRUE(ready_listener.WaitUntilSatisfied());
   ready_listener.Reply("createShelfWindowWithCustomIcon");
   ready_listener.Reset();
+  int update_number;
   if (base::FeatureList::IsEnabled(features::kAppServiceAdaptiveIcon)) {
     // Default app icon + extension icon + AppServiceProxy load icon + custom
-    // icon updates.
-    test_observer.WaitForIconUpdates(4);
+    // icon updates. Ensure the custom icon is set as the window's icon.
+    test_observer.WaitForIconUpdates(custome_icon);
+    EXPECT_TRUE(app_service_test().AreIconImageEqual(
+        custome_icon, test_observer.last_app_icon()));
+    update_number = test_observer.icon_updates();
   } else {
     // Default app icon + extension icon + custom icon updates.
     test_observer.WaitForIconUpdates(3);
+    update_number = test_observer.icon_updates();
   }
 
   const gfx::ImageSkia app_item_custom_image = test_observer.last_app_icon();
@@ -972,13 +985,9 @@ IN_PROC_BROWSER_TEST_F(LauncherPlatformAppBrowserTest, SetIcon) {
 
   // No more icon updates.
   base::RunLoop().RunUntilIdle();
-  if (base::FeatureList::IsEnabled(features::kAppServiceAdaptiveIcon)) {
-    // 3 + 3 + 1 + 4
-    EXPECT_EQ(11, test_observer.icon_updates());
-  } else {
-    // 2 + 2 + 1 + 3
-    EXPECT_EQ(8, test_observer.icon_updates());
-  }
+  EXPECT_TRUE(app_service_test().AreIconImageEqual(
+      custome_icon, test_observer.last_app_icon()));
+  EXPECT_EQ(update_number, test_observer.icon_updates());
 
   // Exit.
   EXPECT_TRUE(ready_listener.WaitUntilSatisfied());

chrome/browser/ui/test/test_app_window_icon_observer.cc

@@ -11,6 +11,7 @@
 #include "extensions/browser/app_window/app_window.h"
 #include "ui/aura/client/aura_constants.h"
 #include "ui/aura/window.h"
+#include "ui/gfx/image/image_unittest_util.h"
 
 TestAppWindowIconObserver::TestAppWindowIconObserver(
     content::BrowserContext* context)
@@ -33,10 +34,17 @@ void TestAppWindowIconObserver::WaitForIconUpdates(int updates) {
   expected_icon_updates_ = updates + icon_updates_;
   icon_updated_callback_ = run_loop.QuitClosure();
   run_loop.Run();
-  base::RunLoop().RunUntilIdle();
   DCHECK_EQ(expected_icon_updates_, icon_updates_);
 }
 
+void TestAppWindowIconObserver::WaitForIconUpdates(
+    const gfx::ImageSkia& image_skia) {
+  base::RunLoop run_loop;
+  expected_image_skia_ = image_skia;
+  icon_image_updated_callback_ = run_loop.QuitClosure();
+  run_loop.Run();
+}
+
 void TestAppWindowIconObserver::OnAppWindowAdded(
     extensions::AppWindow* app_window) {
   aura::Window* window = app_window->GetNativeWindow();
@@ -95,4 +103,11 @@ void TestAppWindowIconObserver::OnWindowPropertyChanged(aura::Window* window,
       !icon_updated_callback_.is_null()) {
     std::move(icon_updated_callback_).Run();
   }
+
+  if (!icon_image_updated_callback_.is_null() &&
+      gfx::test::AreBitmapsEqual(
+          expected_image_skia_.GetRepresentation(1.0f).GetBitmap(),
+          image->GetRepresentation(1.0f).GetBitmap())) {
+    std::move(icon_image_updated_callback_).Run();
+  }
 }

chrome/browser/ui/test/test_app_window_icon_observer.h

@@ -31,6 +31,8 @@ class TestAppWindowIconObserver
   void WaitForIconUpdate();
   // Waits for |updates| number of icon updates.
   void WaitForIconUpdates(int updates);
+  // Waits for icon updates to get |image_skia|.
+  void WaitForIconUpdates(const gfx::ImageSkia& image_skia);
 
   int icon_updates() const { return icon_updates_; }
 
@@ -53,6 +55,8 @@ class TestAppWindowIconObserver
   std::map<aura::Window*, std::string> last_app_icon_hash_map_;
   base::OnceClosure icon_updated_callback_;
   gfx::ImageSkia last_app_icon_;
+  gfx::ImageSkia expected_image_skia_;
+  base::OnceClosure icon_image_updated_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(TestAppWindowIconObserver);
 };