[Kiosk] Restart Web App Kiosk in case of browser crash

Previously, if the browser process crashed during Web kiosk session, we
would have just exited the session. This cl leverages the new kiosk
launch logic, where we abstract out KioskAppLauncher.

Bug: 1054382
Change-Id: Ia8bed81f839b7f7f81d703e8a03af58e768d0fb9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2279121
Commit-Queue: Anatoliy Potapchuk <apotapchuk@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Anqing Zhao <anqing@chromium.org>
Cr-Commit-Position: refs/heads/master@{#786287}

chrome/browser/chromeos/app_mode/app_launch_utils.cc

@@ -6,7 +6,9 @@
 
 #include "base/macros.h"
 #include "chrome/browser/chromeos/app_mode/kiosk_app_launch_error.h"
+#include "chrome/browser/chromeos/app_mode/kiosk_app_types.h"
 #include "chrome/browser/chromeos/app_mode/startup_app_launcher.h"
+#include "chrome/browser/chromeos/app_mode/web_app/web_kiosk_app_launcher.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
 #include "chrome/browser/profiles/profile.h"
 #include "components/prefs/pref_service.h"
@@ -31,25 +33,28 @@ std::vector<std::string>* test_prefs_to_reset = nullptr;
 // it exits the browser process.
 class AppLaunchManager : public StartupAppLauncher::Delegate {
  public:
-  AppLaunchManager(Profile* profile, const std::string& app_id)
-      : startup_app_launcher_(
-            new StartupAppLauncher(profile,
-                                   app_id,
-                                   this)) {}
+  AppLaunchManager(Profile* profile, const KioskAppId& kiosk_app_id) {
+    CHECK(kiosk_app_id.type != KioskAppType::ARC_APP);
+
+    if (kiosk_app_id.type == KioskAppType::CHROME_APP)
+      app_launcher_ = std::make_unique<StartupAppLauncher>(
+          profile, *kiosk_app_id.app_id, this);
+    else
+      app_launcher_ = std::make_unique<WebKioskAppLauncher>(
+          profile, this, *kiosk_app_id.account_id);
+  }
 
-  void Start() { startup_app_launcher_->Initialize(); }
+  void Start() { app_launcher_->Initialize(); }
 
  private:
   ~AppLaunchManager() override {}
 
   void Cleanup() { delete this; }
 
-  // StartupAppLauncher::Delegate overrides:
+  // KioskAppLauncher::Delegate:
   void InitializeNetwork() override {
     // This is on crash-restart path and assumes network is online.
-    // TODO(xiyuan): Remove the crash-restart path for kiosk or add proper
-    // network configure handling.
-    startup_app_launcher_->ContinueWithNetworkReady();
+    app_launcher_->ContinueWithNetworkReady();
   }
   bool IsNetworkReady() const override {
     // See comments above. Network is assumed to be online here.
@@ -63,7 +68,7 @@ class AppLaunchManager : public StartupAppLauncher::Delegate {
     return true;
   }
   void OnAppInstalling() override {}
-  void OnAppPrepared() override { startup_app_launcher_->LaunchApp(); }
+  void OnAppPrepared() override { app_launcher_->LaunchApp(); }
   void OnAppLaunched() override {}
   void OnAppWindowCreated() override { Cleanup(); }
   void OnLaunchFailed(KioskAppLaunchError::Error error) override {
@@ -73,14 +78,14 @@ class AppLaunchManager : public StartupAppLauncher::Delegate {
   }
   bool IsShowingNetworkConfigScreen() const override { return false; }
 
-  std::unique_ptr<KioskAppLauncher> startup_app_launcher_;
+  std::unique_ptr<KioskAppLauncher> app_launcher_;
 
   DISALLOW_COPY_AND_ASSIGN(AppLaunchManager);
 };
 
-void LaunchAppOrDie(Profile* profile, const std::string& app_id) {
+void LaunchAppOrDie(Profile* profile, const KioskAppId& kiosk_app_id) {
   // AppLaunchManager manages its own lifetime.
-  (new AppLaunchManager(profile, app_id))->Start();
+  (new AppLaunchManager(profile, kiosk_app_id))->Start();
 }
 
 void ResetEphemeralKioskPreferences(PrefService* prefs) {

chrome/browser/chromeos/app_mode/app_launch_utils.h

@@ -12,11 +12,13 @@ class Profile;
 
 namespace chromeos {
 
-// Attempts to launch the app given by |app_id| in app mode
+class KioskAppId;
+
+// Attempts to launch the app given by |kiosk_app_id| in app mode
 // or exit on failure. This function will not show any launch UI
 // during the launch. Use StartupAppLauncher for finer control
 // over the app launch processes.
-void LaunchAppOrDie(Profile* profile, const std::string& app_id);
+void LaunchAppOrDie(Profile* profile, const KioskAppId& kiosk_app_id);
 
 // Removes obsolete preferences left out by previous user session;
 void ResetEphemeralKioskPreferences(PrefService* prefs);

chrome/browser/chromeos/app_mode/kiosk_app_types.h

@@ -30,6 +30,11 @@ class KioskAppId {
   static KioskAppId ForWebApp(const AccountId& account_id);
   static KioskAppId ForArcApp(const AccountId& account_id);
 
+  // Use this method when we are unsure which type of kiosk app this AccountId
+  // belongs to.
+  static bool FromAccountId(const AccountId& account_id,
+                            KioskAppId* kiosk_app_id);
+
  private:
   KioskAppId(KioskAppType type, const std::string& app_id);
   KioskAppId(KioskAppType type, const AccountId& account_id);

chrome/browser/chromeos/app_mode/kiosk_crash_restore_browsertest.cc

@@ -13,6 +13,7 @@
 #include "chrome/browser/chromeos/app_mode/fake_cws.h"
 #include "chrome/browser/chromeos/app_mode/kiosk_app_launch_error.h"
 #include "chrome/browser/chromeos/app_mode/kiosk_app_manager.h"
+#include "chrome/browser/chromeos/login/test/kiosk_test_helpers.h"
 #include "chrome/browser/chromeos/ownership/owner_settings_service_chromeos_factory.h"
 #include "chrome/browser/chromeos/policy/device_local_account.h"
 #include "chrome/browser/chromeos/policy/device_policy_builder.h"
@@ -32,12 +33,18 @@
 #include "net/dns/mock_host_resolver.h"
 
 namespace em = enterprise_management;
+constexpr em::DeviceLocalAccountInfoProto_AccountType kWebKioskAccountType =
+    em::DeviceLocalAccountInfoProto_AccountType_ACCOUNT_TYPE_WEB_KIOSK_APP;
+constexpr em::DeviceLocalAccountInfoProto_AccountType kKioskAccountType =
+    em::DeviceLocalAccountInfoProto_AccountType_ACCOUNT_TYPE_KIOSK_APP;
 
 namespace chromeos {
 
 namespace {
 
 const char kTestKioskApp[] = "ggaeimfdpnmlhdhpcikgoblffmkckdmn";
+const char kTestWebAppId[] = "id";
+const char kTestWebAppUrl[] = "https://example.com/";
 
 }  // namespace
 
@@ -83,7 +90,8 @@ class KioskCrashRestoreTest : public InProcessBrowserTest {
         CryptohomeClient::GetStubSanitizedUsername(cryptohome_id));
 
     fake_cws_->Init(embedded_test_server());
-    fake_cws_->SetUpdateCrx(test_app_id_, test_app_id_ + ".crx", "1.0.0");
+    fake_cws_->SetUpdateCrx(kTestKioskApp, std::string(kTestKioskApp) + ".crx",
+                            "1.0.0");
   }
 
   void SetUpOnMainThread() override {
@@ -93,12 +101,7 @@ class KioskCrashRestoreTest : public InProcessBrowserTest {
     embedded_test_server()->StartAcceptingConnections();
   }
 
-  const std::string GetTestAppUserId() const {
-    return policy::GenerateDeviceLocalAccountUserId(
-        test_app_id_, policy::DeviceLocalAccount::TYPE_KIOSK_APP);
-  }
-
-  const std::string& test_app_id() const { return test_app_id_; }
+  virtual const std::string GetTestAppUserId() const = 0;
 
  private:
   void SetUpExistingKioskApp() {
@@ -106,12 +109,20 @@ class KioskCrashRestoreTest : public InProcessBrowserTest {
     em::DeviceLocalAccountsProto* const device_local_accounts =
         device_policy_.payload().mutable_device_local_accounts();
 
-    em::DeviceLocalAccountInfoProto* const account =
-        device_local_accounts->add_account();
-    account->set_account_id(test_app_id_);
-    account->set_type(
-        em::DeviceLocalAccountInfoProto_AccountType_ACCOUNT_TYPE_KIOSK_APP);
-    account->mutable_kiosk_app()->set_app_id(test_app_id_);
+    {
+      em::DeviceLocalAccountInfoProto* const account =
+          device_local_accounts->add_account();
+      account->set_account_id(kTestKioskApp);
+      account->set_type(kKioskAccountType);
+      account->mutable_kiosk_app()->set_app_id(kTestKioskApp);
+    }
+    {
+      em::DeviceLocalAccountInfoProto* const account =
+          device_local_accounts->add_account();
+      account->set_account_id(kTestWebAppId);
+      account->set_type(kWebKioskAccountType);
+      account->mutable_web_kiosk_app()->set_url(kTestWebAppUrl);
+    }
     device_policy_.Build();
 
     // Prepare the policy data to store in device policy cache.
@@ -136,8 +147,6 @@ class KioskCrashRestoreTest : public InProcessBrowserTest {
     base::WriteFile(local_state_file, local_state_json);
   }
 
-  std::string test_app_id_ = kTestKioskApp;
-
   policy::DevicePolicyBuilder device_policy_;
   scoped_refptr<ownership::MockOwnerKeyUtil> owner_key_util_;
   std::unique_ptr<FakeCWS> fake_cws_;
@@ -145,11 +154,33 @@ class KioskCrashRestoreTest : public InProcessBrowserTest {
   DISALLOW_COPY_AND_ASSIGN(KioskCrashRestoreTest);
 };
 
-IN_PROC_BROWSER_TEST_F(KioskCrashRestoreTest, AppNotInstalled) {
+class ChromeKioskCrashRestoreTest : public KioskCrashRestoreTest {
+  const std::string GetTestAppUserId() const override {
+    return policy::GenerateDeviceLocalAccountUserId(
+        kTestKioskApp, policy::DeviceLocalAccount::TYPE_KIOSK_APP);
+  }
+};
+
+IN_PROC_BROWSER_TEST_F(ChromeKioskCrashRestoreTest, ChromeAppNotInstalled) {
   // If app is not installed when restoring from crash, the kiosk launch is
   // expected to fail, as in that case the crash occured during the app
   // initialization - before the app was actually launched.
   EXPECT_EQ(KioskAppLaunchError::UNABLE_TO_LAUNCH, KioskAppLaunchError::Get());
 }
 
+class WebKioskCrashRestoreTest : public KioskCrashRestoreTest {
+  const std::string GetTestAppUserId() const override {
+    return policy::GenerateDeviceLocalAccountUserId(
+        kTestWebAppId, policy::DeviceLocalAccount::TYPE_WEB_KIOSK_APP);
+  }
+};
+
+IN_PROC_BROWSER_TEST_F(WebKioskCrashRestoreTest, WebKioskLaunches) {
+  // If app is not installed when restoring from crash, the kiosk launch is
+  // expected to fail, as in that case the crash occured during the app
+  // initialization - before the app was actually launched.
+  EXPECT_EQ(KioskAppLaunchError::NONE, KioskAppLaunchError::Get());
+  KioskSessionInitializedWaiter().Wait();
+}
+
 }  // namespace chromeos

chrome/browser/ui/startup/startup_browser_creator.cc

@@ -33,6 +33,8 @@
 #include "chrome/browser/app_mode/app_mode_utils.h"
 #include "chrome/browser/apps/platform_apps/app_load_service.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/chromeos/app_mode/kiosk_app_types.h"
+#include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/extensions/startup_helper.h"
 #include "chrome/browser/first_run/first_run.h"
 #include "chrome/browser/prefs/incognito_mode_prefs.h"
@@ -116,9 +118,7 @@ namespace {
 class ProfileLaunchObserver : public ProfileObserver,
                               public BrowserListObserver {
  public:
-  ProfileLaunchObserver() {
-    BrowserList::AddObserver(this);
-  }
+  ProfileLaunchObserver() { BrowserList::AddObserver(this); }
 
   ~ProfileLaunchObserver() override { BrowserList::RemoveObserver(this); }
 
@@ -656,25 +656,30 @@ bool StartupBrowserCreator::ProcessCmdLineImpl(
   if (command_line.HasSwitch(chromeos::switches::kLoginManager))
     silent_launch = true;
 
-  if (chrome::IsRunningInAppMode() &&
-      command_line.HasSwitch(switches::kAppId)) {
-    chromeos::LaunchAppOrDie(
-        last_used_profile,
-        command_line.GetSwitchValueASCII(switches::kAppId));
+  if (chrome::IsRunningInForcedAppMode()) {
+    user_manager::User* user =
+        chromeos::ProfileHelper::Get()->GetUserByProfile(last_used_profile);
+    if (user && user->GetType() == user_manager::USER_TYPE_KIOSK_APP) {
+      chromeos::LaunchAppOrDie(
+          last_used_profile,
+          chromeos::KioskAppId::ForChromeApp(
+              command_line.GetSwitchValueASCII(switches::kAppId)));
+    } else if (user &&
+               user->GetType() == user_manager::USER_TYPE_WEB_KIOSK_APP) {
+      chromeos::LaunchAppOrDie(
+          last_used_profile,
+          chromeos::KioskAppId::ForWebApp(user->GetAccountId()));
+    } else {
+      // If we are here, we are either in ARC kiosk session or the user is
+      // invalid. We should terminate the session in such cases.
+      chrome::AttemptUserExit();
+      return false;
+    }
 
     // Skip browser launch since app mode launches its app window.
     silent_launch = true;
   }
 
-  // If we are in the recoverable ARC/PWA app mode state (we do not have kAppId
-  // set), we should terminate the session instead of just showing black screen.
-  // TODO(crbug.com/1054382): Add a way of restarting PWA and Arc kiosks.
-  if (chrome::IsRunningInForcedAppMode() &&
-      !command_line.HasSwitch(switches::kAppId)) {
-    chrome::AttemptUserExit();
-    return false;
-  }
-
   // If we are a demo app session and we crashed, there is no safe recovery
   // possible. We should instead cleanly exit and go back to the OOBE screen,
   // where we will launch again after the timeout has expired.
@@ -770,8 +775,8 @@ bool StartupBrowserCreator::ProcessCmdLineImpl(
     base::CommandLine::StringType path =
         command_line.GetSwitchValueNative(apps::kLoadAndLaunchApp);
 
-    if (!apps::AppLoadService::Get(last_used_profile)->LoadAndLaunch(
-            base::FilePath(path), command_line, cur_dir)) {
+    if (!apps::AppLoadService::Get(last_used_profile)
+             ->LoadAndLaunch(base::FilePath(path), command_line, cur_dir)) {
       return false;
     }
 
@@ -814,11 +819,12 @@ bool StartupBrowserCreator::LaunchBrowserForLastProfiles(
   }
 #endif  // !defined(OS_CHROMEOS)
 
-  chrome::startup::IsProcessStartup is_process_startup = process_startup ?
-      chrome::startup::IS_PROCESS_STARTUP :
-      chrome::startup::IS_NOT_PROCESS_STARTUP;
-  chrome::startup::IsFirstRun is_first_run = first_run::IsChromeFirstRun() ?
-      chrome::startup::IS_FIRST_RUN : chrome::startup::IS_NOT_FIRST_RUN;
+  chrome::startup::IsProcessStartup is_process_startup =
+      process_startup ? chrome::startup::IS_PROCESS_STARTUP
+                      : chrome::startup::IS_NOT_PROCESS_STARTUP;
+  chrome::startup::IsFirstRun is_first_run =
+      first_run::IsChromeFirstRun() ? chrome::startup::IS_FIRST_RUN
+                                    : chrome::startup::IS_NOT_FIRST_RUN;
 
   // On Windows, when chrome is launched by notification activation where the
   // kNotificationLaunchId switch is used, always use |last_used_profile| which
@@ -1022,7 +1028,7 @@ bool StartupBrowserCreator::ActivatedProfile() {
 
 bool HasPendingUncleanExit(Profile* profile) {
   return profile->GetLastSessionExitType() == Profile::EXIT_CRASHED &&
-      !profile_launch_observer.Get().HasBeenLaunched(profile);
+         !profile_launch_observer.Get().HasBeenLaunched(profile);
 }
 
 base::FilePath GetStartupProfilePath(const base::FilePath& user_data_dir,
@@ -1076,9 +1082,10 @@ Profile* GetStartupProfile(const base::FilePath& user_data_dir,
   // We want to show the user manager. To indicate this, return the guest
   // profile. However, we can only do this if the system profile (where the user
   // manager lives) also exists (or is creatable).
-  return profile_manager->GetProfile(ProfileManager::GetSystemProfilePath()) ?
-         profile_manager->GetProfile(ProfileManager::GetGuestProfilePath()) :
-         nullptr;
+  return profile_manager->GetProfile(ProfileManager::GetSystemProfilePath())
+             ? profile_manager->GetProfile(
+                   ProfileManager::GetGuestProfilePath())
+             : nullptr;
 }
 
 Profile* GetFallbackStartupProfile() {
@@ -1095,8 +1102,8 @@ Profile* GetFallbackStartupProfile() {
   for (Profile* profile : ProfileManager::GetLastOpenedProfiles()) {
     // Return any profile that is not locked.
     ProfileAttributesEntry* entry;
-    bool has_entry = storage->GetProfileAttributesWithPath(profile->GetPath(),
-                                                           &entry);
+    bool has_entry =
+        storage->GetProfileAttributesWithPath(profile->GetPath(), &entry);
     if (!has_entry || !entry->IsSigninRequired())
       return profile;
   }