[Kiosk] Redownload the icon when the icon url changes

This cl makes WebKioskAppData keep track of |icon_url_| provided by
policy. Now, if the app has not been yet installed, we will schedule
an icon download whenever its url changes.

Bug: 1015383
Change-Id: I0e1d342cd11bd58d8e234becde5a81824302a63d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2353262Reviewed-by: Anqing Zhao <anqing@chromium.org>
Commit-Queue: Anatoliy Potapchuk <apotapchuk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799579}

chrome/browser/chromeos/app_mode/web_app/web_kiosk_app_data.cc

@@ -28,6 +28,7 @@ constexpr int kIconSize = 128;  // size of the icon in px.
 constexpr int kMaxIconFileSize = (2 * kIconSize) * (2 * kIconSize) * 4 + 1000;
 
 const char kKeyLaunchUrl[] = "launch_url";
+const char kKeyLastIconUrl[] = "last_icon_url";
 
 // Resizes image into other size on blocking I/O thread.
 SkBitmap ResizeImageBlocking(const SkBitmap& image, int target_size) {
@@ -169,6 +170,11 @@ bool WebKioskAppData::LoadFromCache() {
                           /* lazy_icon_load= */ true))
     return false;
 
+  // If the icon was previously downloaded using a different url, do not use
+  // that icon.
+  if (GetLastIconUrl(*dict) != icon_url_)
+    return false;
+
   if (LoadLaunchUrlFromDictionary(*dict)) {
     SetStatus(STATUS_INSTALLED);
     return true;
@@ -184,7 +190,7 @@ void WebKioskAppData::LoadIcon() {
   if (!icon_.isNull())
     return;
 
-  // We already had some icon cached, it is time to decode it.
+  // Decode the icon if one is already cached.
   if (status_ != STATUS_INIT) {
     DecodeIcon();
     return;
@@ -252,6 +258,17 @@ bool WebKioskAppData::LoadLaunchUrlFromDictionary(const base::Value& dict) {
   return true;
 }
 
+GURL WebKioskAppData::GetLastIconUrl(const base::Value& dict) const {
+  // All the previous keys should be present since this function is executed
+  // after LoadFromDictionary().
+  const std::string* icon_url_string =
+      dict.FindDictKey(KioskAppDataBase::kKeyApps)
+          ->FindDictKey(app_id())
+          ->FindStringKey(kKeyLastIconUrl);
+
+  return icon_url_string ? GURL(*icon_url_string) : GURL();
+}
+
 void WebKioskAppData::OnDidDownloadIcon(const SkBitmap& icon) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
@@ -270,6 +287,10 @@ void WebKioskAppData::OnDidDownloadIcon(const SkBitmap& icon) {
   DictionaryPrefUpdate dict_update(local_state, dictionary_name());
   SaveIconToDictionary(dict_update);
 
+  dict_update->FindDictKey(KioskAppDataBase::kKeyApps)
+      ->FindDictKey(app_id())
+      ->SetStringKey(kKeyLastIconUrl, launch_url_.spec());
+
   SetStatus(STATUS_LOADED);
 }
 

chrome/browser/chromeos/app_mode/web_app/web_kiosk_app_data.h

@@ -62,12 +62,16 @@ class WebKioskAppData : public KioskAppDataBase {
 
   bool LoadLaunchUrlFromDictionary(const base::Value& dict);
 
+  // Returns the icon url of the icon that was being provided during previous
+  // session.
+  GURL GetLastIconUrl(const base::Value& dict) const;
+
   KioskAppDataDelegate* delegate_;  // not owned.
   Status status_;
   const GURL install_url_;  // installation url.
   GURL launch_url_;         // app launch url.
 
-  const GURL icon_url_;  // Url of the icon in case nothing is cached.
+  GURL icon_url_;  // Url of the icon in case nothing is cached.
   // Used to download icon from |icon_url_|.
   std::unique_ptr<IconFetcher> icon_fetcher_;
 

chrome/browser/chromeos/app_mode/web_app/web_kiosk_app_data_browsertest.cc

@@ -32,6 +32,8 @@ const char kIconKey[] = "icon";
 const char kLaunchUrlKey[] = "launch_url";
 const char kIconPath[] = "chrome/test/data/load_image/image.png";
 const char kIconUrl[] = "/load_image/image.png";
+const char kIconUrl2[] = "/load_image/fail_image.png";
+const char kLastIconUrlKey[] = "last_icon_url";
 const char kLaunchUrl[] = "https://example.com/launch";
 
 base::FilePath GetFullPathToImage() {
@@ -123,7 +125,6 @@ IN_PROC_BROWSER_TEST_F(WebKioskAppDataTest, PRE_DownloadedIconPersists) {
   test_server.AddDefaultHandlers(GetChromeTestDataDir());
   ASSERT_TRUE(test_server.Start());
 
-  // SetCachedNameAndIcon();
   WebKioskAppData app_data(this, kAppId, EmptyAccountId(), GURL(kAppUrl),
                            kAppTitle,
                            /*icon_url*/ test_server.GetURL(kIconUrl));
@@ -138,9 +139,23 @@ IN_PROC_BROWSER_TEST_F(WebKioskAppDataTest, PRE_DownloadedIconPersists) {
 IN_PROC_BROWSER_TEST_F(WebKioskAppDataTest, DownloadedIconPersists) {
   // No test server is launched intentionaly to verify that we are using the
   // cached icon.
+  // We should still find the correct icon url in order to not initiate a
+  // redownload.
+  const std::string* icon_url_string =
+      g_browser_process->local_state()
+          ->GetDictionary(WebKioskAppManager::kWebKioskDictionaryName)
+          ->FindDictKey(KioskAppDataBase::kKeyApps)
+          ->FindDictKey(kAppId)
+          ->FindStringKey(kLastIconUrlKey);
+  ASSERT_TRUE(icon_url_string);
+  const GURL icon_url = GURL(*icon_url_string);
+
   WebKioskAppData app_data(this, kAppId, EmptyAccountId(), GURL(kAppUrl),
-                           kAppTitle2, /*icon_url=*/GURL());
+                           kAppTitle2, /*icon_url=*/icon_url);
   app_data.LoadFromCache();
+  // Icon is stored in cache.
+  EXPECT_EQ(app_data.status(), WebKioskAppData::STATUS_LOADING);
+
   app_data.LoadIcon();
   WaitForAppDataChange(2);
 
@@ -149,6 +164,45 @@ IN_PROC_BROWSER_TEST_F(WebKioskAppDataTest, DownloadedIconPersists) {
   EXPECT_EQ(app_data.name(), kAppTitle2);
 }
 
+IN_PROC_BROWSER_TEST_F(WebKioskAppDataTest,
+                       PRE_RedownloadIconWhenDifferentUrl) {
+  // Start test server.
+  net::EmbeddedTestServer test_server;
+  test_server.AddDefaultHandlers(GetChromeTestDataDir());
+  ASSERT_TRUE(test_server.Start());
+
+  WebKioskAppData app_data(this, kAppId, EmptyAccountId(), GURL(kAppUrl),
+                           kAppTitle,
+                           /*icon_url*/ test_server.GetURL(kIconUrl));
+  app_data.LoadFromCache();
+  app_data.LoadIcon();
+  WaitForAppDataChange(1);
+
+  EXPECT_EQ(app_data.status(), WebKioskAppData::STATUS_LOADED);
+  EXPECT_EQ(app_data.name(), kAppTitle);
+}
+
+IN_PROC_BROWSER_TEST_F(WebKioskAppDataTest, RedownloadIconWhenDifferentUrl) {
+  // Start test server.
+  net::EmbeddedTestServer test_server;
+  test_server.AddDefaultHandlers(GetChromeTestDataDir());
+  ASSERT_TRUE(test_server.Start());
+
+  WebKioskAppData app_data(this, kAppId, EmptyAccountId(), GURL(kAppUrl),
+                           kAppTitle2,
+                           /*icon_url*/ test_server.GetURL(kIconUrl2));
+
+  app_data.LoadFromCache();
+  // No icon was loaded from cache because urls are different.
+  EXPECT_EQ(app_data.status(), WebKioskAppData::STATUS_INIT);
+
+  app_data.LoadIcon();
+  WaitForAppDataChange(1);
+
+  EXPECT_EQ(app_data.status(), WebKioskAppData::STATUS_LOADED);
+  EXPECT_EQ(app_data.name(), kAppTitle2);
+}
+
 IN_PROC_BROWSER_TEST_F(WebKioskAppDataTest, AlreadyInstalled) {
   SetCached(/*installed = */ true);
   WebKioskAppData app_data(this, kAppId, EmptyAccountId(), GURL(kAppUrl),