Revert "[dPWA/AppIdentity]: Add integration test for icon updates."

This reverts commit 49b1515. Reason for revert: Possible culprit https://ci.chromium.org/p/chromium/builders/ci/Mac10.14%20Tests?limit=200 Original change's description: > [dPWA/AppIdentity]: Add integration test for icon updates. > > Bug: 1292563 > Change-Id: I153645c732b2131956f02f252e7f10416cb9f1df > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3507181 > Reviewed-by: Daniel Murphy <dmurph@chromium.org> > Commit-Queue: Finnur Thorarinsson <finnur@chromium.org> > Cr-Commit-Position: refs/heads/main@{#979318} Bug: 1292563 Change-Id: Iabacf52ebfb5a9b203dd25d1fa9c94251649101f No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3514636 Reviewed-by: Muyao Xu <muyaoxu@google.com> Owners-Override: Muyao Xu <muyaoxu@google.com> Auto-Submit: Muyao Xu <muyaoxu@google.com> Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Cr-Commit-Position: refs/heads/main@{#979430}
Denger-Network · Mar 9, 2022 · a53232a · a53232a
1 parent b10146f
commit a53232a
Show file tree

Hide file tree

Showing 14 changed files with 7 additions and 137 deletions.
diff --git a/chrome/browser/ui/views/web_apps/web_app_integration_browsertest.cc b/chrome/browser/ui/views/web_apps/web_app_integration_browsertest.cc
@@ -25,16 +25,6 @@ IN_PROC_BROWSER_TEST_F(WebAppIntegrationBrowserTest, ManifestUpdateScope) {
   helper_.CheckLaunchIconShown();
 }
 
-IN_PROC_BROWSER_TEST_F(WebAppIntegrationBrowserTest, ManifestUpdateIcon) {
-  helper_.InstallMenuOption("SiteA");
-  helper_.CheckAppIconSiteA("green");
-  helper_.ManifestUpdateIcon("SiteA");
-  helper_.AcceptAppIdUpdateDialog();
-  helper_.ClosePwa();
-  helper_.LaunchFromLaunchIcon("SiteA");
-  helper_.CheckAppIconSiteA("red");
-}
-
 IN_PROC_BROWSER_TEST_F(WebAppIntegrationBrowserTest, ManifestUpdateTitle) {
   helper_.InstallMenuOption("SiteA");
   helper_.CheckAppTitleSiteA("Site A");

diff --git a/chrome/browser/ui/views/web_apps/web_app_integration_test_driver.cc b/chrome/browser/ui/views/web_apps/web_app_integration_test_driver.cc
@@ -58,7 +58,6 @@
 #include "chrome/browser/web_applications/policy/web_app_policy_manager.h"
 #include "chrome/browser/web_applications/test/web_app_install_test_utils.h"
 #include "chrome/browser/web_applications/test/web_app_test_observers.h"
-#include "chrome/browser/web_applications/web_app_icon_generator.h"
 #include "chrome/browser/web_applications/web_app_id.h"
 #include "chrome/browser/web_applications/web_app_install_finalizer.h"
 #include "chrome/browser/web_applications/web_app_provider.h"
@@ -165,12 +164,12 @@ const base::flat_map<std::string, std::string> g_site_mode_to_app_name = {
 
 #if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN)
 const base::flat_map<std::string, SkColor> g_app_name_icon_color = {
-    {"Site A", SkColorSetARGB(0xFF, 0x00, 0xFF, 0x00)},
+    {"Site A", SkColorSetARGB(0xFF, 0x00, 0x00, 0x00)},
     {"Site B", SkColorSetARGB(0xFF, 0x00, 0x00, 0x00)},
     {"Site C", SkColorSetARGB(0x00, 0x00, 0x00, 0x00)},
     {"Site A Foo", SkColorSetARGB(0xFF, 0x00, 0x00, 0x00)},
     {"Site A Bar", SkColorSetARGB(0xFF, 0x00, 0x00, 0x00)},
-    {"Site A - Updated name", SkColorSetARGB(0xFF, 0x00, 0xFF, 0x00)}};
+    {"Site A - Updated name", SkColorSetARGB(0xFF, 0x00, 0x00, 0x00)}};
 #endif
 
 #if !BUILDFLAG(IS_CHROMEOS)
@@ -347,7 +346,6 @@ AppState::AppState(web_app::AppId app_id,
                    const apps::RunOnOsLoginMode& run_on_os_login_mode,
                    const blink::mojom::DisplayMode& effective_display_mode,
                    const blink::mojom::DisplayMode& user_display_mode,
-                   const std::string& manifest_install_icon,
                    bool installed_locally,
                    bool shortcut_created)
     : id(app_id),
@@ -357,7 +355,6 @@ AppState::AppState(web_app::AppId app_id,
       run_on_os_login_mode(run_on_os_login_mode),
       effective_display_mode(effective_display_mode),
       user_display_mode(user_display_mode),
-      manifest_install_icon(manifest_install_icon),
       is_installed_locally(installed_locally),
       is_shortcut_created(shortcut_created) {}
 AppState::~AppState() = default;
@@ -368,7 +365,6 @@ bool AppState::operator==(const AppState& other) const {
          run_on_os_login_mode == other.run_on_os_login_mode &&
          effective_display_mode == other.effective_display_mode &&
          user_display_mode == other.user_display_mode &&
-         manifest_install_icon == other.manifest_install_icon &&
          is_installed_locally == other.is_installed_locally &&
          is_shortcut_created == other.is_shortcut_created;
 }
@@ -438,8 +434,6 @@ std::ostream& operator<<(std::ostream& os, const StateSnapshot& state) {
                           static_cast<int>(app.effective_display_mode));
       app_value.SetIntKey("user_display_mode",
                           static_cast<int>(app.effective_display_mode));
-      app_value.SetStringKey("manifest_install_icon",
-                             app.manifest_install_icon);
       app_value.SetBoolKey("is_installed_locally", app.is_installed_locally);
       app_value.SetBoolKey("is_shortcut_created", app.is_shortcut_created);
 
@@ -981,27 +975,6 @@ void WebAppIntegrationTestDriver::NavigateTabbedBrowserToSite(const GURL& url) {
   AfterStateChangeAction();
 }
 
-void WebAppIntegrationTestDriver::ManifestUpdateIcon(
-    const std::string& site_mode) {
-  BeforeStateChangeAction(__FUNCTION__);
-  ASSERT_EQ("SiteA", site_mode) << "Only site mode of 'SiteA' is supported";
-  ASSERT_TRUE(base::Contains(g_site_mode_to_relative_scope_url, site_mode));
-
-  app_id_update_dialog_waiter_ =
-      std::make_unique<views::NamedWidgetShownWaiter>(
-          views::test::AnyWidgetTestPasskey{},
-          "WebAppIdentityUpdateConfirmationView");
-
-  auto scope_url_path =
-      g_site_mode_to_relative_scope_url.find(site_mode)->second;
-  std::string str_template =
-      "/web_apps/%s/basic.html?manifest=manifest_icon_%u.json";
-  GURL url = embedded_test_server()->GetURL(base::StringPrintf(
-      str_template.c_str(), scope_url_path.c_str(), kInstallIconSize));
-  ForceUpdateManifestContents(site_mode, url);
-  AfterStateChangeAction();
-}
-
 void WebAppIntegrationTestDriver::ManifestUpdateTitle(
     const std::string& site_mode) {
   BeforeStateChangeAction(__FUNCTION__);
@@ -1457,18 +1430,6 @@ void WebAppIntegrationTestDriver::CheckPlatformShortcutNotExists(
   AfterStateCheckAction();
 }
 
-void WebAppIntegrationTestDriver::CheckAppIconSiteA(const std::string& color) {
-  BeforeStateCheckAction(__FUNCTION__);
-  absl::optional<AppState> app_state = GetAppBySiteMode(
-      after_state_change_action_state_.get(), profile(), "SiteA");
-  ASSERT_TRUE(app_state);
-  // TODO(finnur): Implement checking the actual icon data.
-  EXPECT_EQ(app_state->manifest_install_icon,
-            base::StringPrintf("%ux%u-%s.png", kInstallIconSize,
-                               kInstallIconSize, color.c_str()));
-  AfterStateCheckAction();
-}
-
 void WebAppIntegrationTestDriver::CheckAppTitleSiteA(const std::string& title) {
   BeforeStateCheckAction(__FUNCTION__);
   absl::optional<AppState> app_state = GetAppBySiteMode(
@@ -1843,16 +1804,6 @@ WebAppIntegrationTestDriver::ConstructStateSnapshot() {
                                                    apps::AppType::kWeb,
                                                    /*delegate=*/nullptr, true);
     for (const auto& app_id : app_ids) {
-      std::string manifest_install_icon;
-      std::vector<apps::IconInfo> icon_infos =
-          provider()->registrar().GetAppIconInfos(app_id);
-      for (const auto& info : icon_infos) {
-        int icon_size = info.square_size_px.value_or(-1);
-        if (icon_size == kInstallIconSize) {
-          manifest_install_icon = info.url.ExtractFileName();
-        }
-      }
-
       app_state.emplace(
           app_id,
           AppState(app_id, registrar.GetAppShortName(app_id),
@@ -1863,7 +1814,7 @@ WebAppIntegrationTestDriver::ConstructStateSnapshot() {
                        registrar.GetAppRunOnOsLoginMode(app_id).value),
                    registrar.GetAppEffectiveDisplayMode(app_id),
                    registrar.GetAppUserDisplayMode(app_id),
-                   manifest_install_icon, registrar.IsLocallyInstalled(app_id),
+                   registrar.IsLocallyInstalled(app_id),
                    IsShortcutAndIconCreated(
                        profile, registrar.GetAppShortName(app_id), app_id)));
     }
@@ -2175,7 +2126,6 @@ PageActionIconView* WebAppIntegrationTestDriver::intent_picker_view() {
 WebAppIntegrationBrowserTest::WebAppIntegrationBrowserTest() : helper_(this) {
   std::vector<base::Feature> enabled_features;
   std::vector<base::Feature> disabled_features;
-  enabled_features.push_back(features::kPwaUpdateDialogForIcon);
   enabled_features.push_back(features::kPwaUpdateDialogForName);
 #if BUILDFLAG(IS_CHROMEOS_ASH)
   disabled_features.push_back(features::kWebAppsCrosapi);

diff --git a/chrome/browser/ui/views/web_apps/web_app_integration_test_driver.h b/chrome/browser/ui/views/web_apps/web_app_integration_test_driver.h
@@ -77,7 +77,6 @@ struct AppState {
            const apps::RunOnOsLoginMode& run_on_os_login_mode,
            const blink::mojom::DisplayMode& effective_display_mode,
            const blink::mojom::DisplayMode& user_display_mode,
-           const std::string& manifest_install_icon,
            bool is_installed_locally,
            bool is_shortcut_created);
   ~AppState();
@@ -91,7 +90,6 @@ struct AppState {
   apps::RunOnOsLoginMode run_on_os_login_mode;
   blink::mojom::DisplayMode effective_display_mode;
   blink::mojom::DisplayMode user_display_mode;
-  std::string manifest_install_icon;
   bool is_installed_locally;
   bool is_shortcut_created;
 };
@@ -178,7 +176,6 @@ class WebAppIntegrationTestDriver : WebAppInstallManagerObserver {
   void NavigatePwaSiteATo(const std::string& site_mode);
   void NavigateNotfoundUrl();
   void NavigateTabbedBrowserToSite(const GURL& url);
-  void ManifestUpdateIcon(const std::string& site_mode);
   void ManifestUpdateTitle(const std::string& site_mode);
   void ManifestUpdateDisplayBrowser(const std::string& site_mode);
   void ManifestUpdateDisplayMinimal(const std::string& site_mode);
@@ -204,7 +201,6 @@ class WebAppIntegrationTestDriver : WebAppInstallManagerObserver {
   void CheckBrowserNavigationIsAppSettings(const std::string& site_mode);
   void CheckAppSettingsAppState(const std::string& site_mode);
   void CheckAppNotInList(const std::string& site_mode);
-  void CheckAppIconSiteA(const std::string& color);
   void CheckAppTitleSiteA(const std::string& title);
   void CheckAppWindowMode(const std::string& site_mode,
                           apps::WindowMode window_mode);

diff --git a/chrome/test/data/web_apps/site_a/48x48-green.png b/chrome/test/data/web_apps/site_a/48x48-green.png
diff --git a/chrome/test/data/web_apps/site_a/48x48-red.png b/chrome/test/data/web_apps/site_a/48x48-red.png
diff --git a/chrome/test/data/web_apps/site_a/96x96-green.png b/chrome/test/data/web_apps/site_a/96x96-green.png
diff --git a/chrome/test/data/web_apps/site_a/96x96-red.png b/chrome/test/data/web_apps/site_a/96x96-red.png
diff --git a/chrome/test/data/web_apps/site_a/basic-48.png b/chrome/test/data/web_apps/site_a/basic-48.png
diff --git a/chrome/test/data/web_apps/site_a/basic.json b/chrome/test/data/web_apps/site_a/basic.json
@@ -2,15 +2,10 @@
   "name": "Site A",
   "icons": [
     {
-      "src": "48x48-green.png",
+      "src": "basic-48.png",
       "sizes": "48x48",
       "type": "image/png"
     },
-    {
-      "src": "96x96-green.png",
-      "sizes": "96x96",
-      "type": "image/png"
-    },
     {
       "src": "basic-192.png",
       "sizes": "192x192",

diff --git a/chrome/test/data/web_apps/site_a/manifest_browser.json b/chrome/test/data/web_apps/site_a/manifest_browser.json
@@ -2,15 +2,10 @@
   "name": "Site A",
   "icons": [
     {
-      "src": "48x48-green.png",
+      "src": "basic-48.png",
       "sizes": "48x48",
       "type": "image/png"
     },
-    {
-      "src": "96x96-green.png",
-      "sizes": "96x96",
-      "type": "image/png"
-    },
     {
       "src": "basic-192.png",
       "sizes": "192x192",

diff --git a/chrome/test/data/web_apps/site_a/manifest_icon_48.json b/chrome/test/data/web_apps/site_a/manifest_icon_48.json
diff --git a/chrome/test/data/web_apps/site_a/manifest_icon_96.json b/chrome/test/data/web_apps/site_a/manifest_icon_96.json
diff --git a/chrome/test/data/web_apps/site_a/manifest_minimal_ui.json b/chrome/test/data/web_apps/site_a/manifest_minimal_ui.json
@@ -2,15 +2,10 @@
   "name": "Site A",
   "icons": [
     {
-      "src": "48x48-green.png",
+      "src": "basic-48.png",
       "sizes": "48x48",
       "type": "image/png"
     },
-    {
-      "src": "96x96-green.png",
-      "sizes": "96x96",
-      "type": "image/png"
-    },
     {
       "src": "basic-192.png",
       "sizes": "192x192",

diff --git a/chrome/test/data/web_apps/site_a/manifest_title.json b/chrome/test/data/web_apps/site_a/manifest_title.json
@@ -2,15 +2,10 @@
   "name": "Site A - Updated name",
   "icons": [
     {
-      "src": "48x48-green.png",
+      "src": "basic-48.png",
       "sizes": "48x48",
       "type": "image/png"
     },
-    {
-      "src": "96x96-green.png",
-      "sizes": "96x96",
-      "type": "image/png"
-    },
     {
       "src": "basic-192.png",
       "sizes": "192x192",