[Kiosk] Invalidate external cache whenever it is corrupted

Sometimes, while the icon was not loaded yet, we would end up in the
state where the crx file is corrupt and we would hide the app from the
list, causing some confusion in users. Now, in all cases when crx
file is corrupted, we will still display the app.

Also, adding more logs for STATUS_ERROR failures.

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

chrome/browser/chromeos/app_mode/kiosk_app_data.cc

@@ -360,7 +360,6 @@ void KioskAppData::SetStatus(Status status) {
 
   switch (status_) {
     case STATUS_INIT:
-      break;
     case STATUS_LOADING:
     case STATUS_LOADED:
       delegate_->OnKioskAppDataChanged(app_id());
@@ -457,6 +456,7 @@ void KioskAppData::OnWebstoreParseSuccess(
 }
 
 void KioskAppData::OnWebstoreParseFailure() {
+  LOG(WARNING) << "Webstore request parse failure for app_id=" << app_id();
   SetStatus(STATUS_ERROR);
 }
 
@@ -563,16 +563,14 @@ void KioskAppData::OnCrxLoadFinished(const CrxLoader* crx_loader) {
     return;
 
   if (!crx_loader->success()) {
-    // If we did get some data before in from Local State, but the extentension
-    // file got corrupted, we should notify the ExternalCache to remove it and
-    // redownload it upon next session start(kiosk or login).
-    if (status() == STATUS_LOADED) {
-      // Our cache is corrupted, we need to notify ExternalCache about that.
-      if (delegate_)
-        delegate_->OnExternalCacheDamaged(app_id());
-    } else {
-      SetStatus(STATUS_ERROR);
-    }
+    LOG(ERROR) << "Failed to load cached extension data for app_id="
+               << app_id();
+    // If after unpacking the cached extension we received an error, schedule a
+    // redownload upon next session start(kiosk or login).
+    if (delegate_)
+      delegate_->OnExternalCacheDamaged(app_id());
+
+    SetStatus(STATUS_INIT);
     return;
   }
 

chrome/browser/chromeos/app_mode/kiosk_app_manager_browsertest.cc

@@ -649,6 +649,59 @@ IN_PROC_BROWSER_TEST_F(KioskAppManagerTest, UpdateAppDataFromCrx) {
   CheckAppData(kAppId, kAppName, "1234");
 }
 
+IN_PROC_BROWSER_TEST_F(KioskAppManagerTest, FailedToLoadFromCrx) {
+  const char kAppId[] = "iiigpodgfihagabpagjehoocpakbnclp";
+  const char kAppName[] = "Test Kiosk App";
+
+  SetExistingApp(kAppId, kAppName, "red16x16.png", "");
+  fake_cws()->SetNoUpdate(kAppId);
+  AppDataLoadWaiter waiter(manager(), 1);
+  waiter.Wait();
+  EXPECT_EQ(waiter.data_change_count(), 1);
+  EXPECT_EQ(waiter.data_load_failure_count(), 0);
+  EXPECT_TRUE(waiter.loaded());
+
+  CheckAppData(kAppId, kAppName, "");
+
+  // Fake app data load failure so that the manager will attempt to
+  // load it from crx.
+  KioskAppData* app_data = GetAppDataMutable(kAppId);
+  app_data->SetStatusForTest(KioskAppData::STATUS_ERROR);
+
+  // Copy test crx file to temp dir because the cache moves the file.
+  base::FilePath test_dir;
+  ASSERT_TRUE(base::PathService::Get(chrome::DIR_TEST_DATA, &test_dir));
+  base::FilePath data_dir =
+      test_dir.AppendASCII("chromeos/app_mode/webstore/downloads/");
+  base::FilePath crx_file = data_dir.AppendASCII(
+      "pegeblegnlhnpgghhjblhchdllfijodp-2.0.0."
+      "crx");
+  crx_file = CopyFileToTempDir(crx_file);
+
+  ExternalCachePutWaiter put_waiter;
+  manager()->PutValidatedExternalExtension(
+      kAppId, crx_file, "2.0.0",
+      base::BindOnce(&ExternalCachePutWaiter::OnPutExtension,
+                     base::Unretained(&put_waiter)));
+  put_waiter.Wait();
+  ASSERT_TRUE(put_waiter.success());
+
+  // Wait for 3 data loaded events at the most. One for crx putting into cache,
+  // one for update check and one for crx unpack to fail which resets the app
+  // data status into INIT stage.
+  const size_t kMaxDataChange = 3;
+  for (size_t i = 0;
+       i < kMaxDataChange && app_data->status() != KioskAppData::STATUS_INIT;
+       ++i) {
+    waiter.Reset();
+    waiter.Wait();
+    EXPECT_EQ(waiter.data_change_count(), 1);
+    EXPECT_EQ(waiter.data_load_failure_count(), 0);
+  }
+  ASSERT_EQ(KioskAppData::STATUS_INIT, app_data->status());
+  CheckAppData(kAppId, kAppName, "");
+}
+
 IN_PROC_BROWSER_TEST_F(KioskAppManagerTest, BadApp) {
   AppDataLoadWaiter waiter(manager(), 2);
   manager()->AddApp("unknown_app", owner_settings_service_.get());