Create a ChromeKioskAppLauncher that takes care of the launch part of…

… app kiosk setup This further splits the StartupAppLauncher into a third component for the launch part. This code will run in Lacros after the split. This change introduces some changes in the order of operations: enabling secondary apps now happens in the launch part rather than the install part. This change is reflected in changes to the unit tests. Bug: 226560767 Test: StartupAppLauncherUnittest Change-Id: Ibc214a95f711ce359b111fbfaf2fc0c8a7d80877 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3548857 Reviewed-by: Anqing Zhao <anqing@chromium.org> Reviewed-by: Jeroen Dhollander <jeroendh@google.com> Commit-Queue: Ben Franz <bfranz@chromium.org> Cr-Commit-Position: refs/heads/main@{#988419}
chromium · Apr 4, 2022 · 38c8e2a · 38c8e2a
1 parent a593268
commit 38c8e2a
Show file tree

Hide file tree

Showing 10 changed files with 563 additions and 191 deletions.
diff --git a/chrome/browser/ash/app_mode/startup_app_launcher.cc b/chrome/browser/ash/app_mode/startup_app_launcher.cc
@@ -14,17 +14,14 @@
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "base/values.h"
-#include "chrome/browser/apps/app_service/app_launch_params.h"
 #include "chrome/browser/ash/app_mode/kiosk_app_launch_error.h"
 #include "chrome/browser/ash/app_mode/kiosk_app_manager.h"
 #include "chrome/browser/ash/net/delay_network_call.h"
 #include "chrome/browser/chromeos/app_mode/chrome_kiosk_app_installer.h"
+#include "chrome/browser/chromeos/app_mode/chrome_kiosk_app_launcher.h"
 #include "chrome/browser/chromeos/app_mode/startup_app_launcher_update_checker.h"
 #include "chrome/browser/extensions/extension_service.h"
-#include "chrome/browser/extensions/install_tracker_factory.h"
-#include "chrome/browser/lifetime/application_lifetime.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/ui/extensions/application_launch.h"
 #include "chrome/common/chrome_switches.h"
 #include "components/crx_file/id_util.h"
 #include "extensions/browser/app_window/app_window.h"
@@ -55,7 +52,6 @@ StartupAppLauncher::StartupAppLauncher(Profile* profile,
     : KioskAppLauncher(delegate), profile_(profile), app_id_(app_id) {
   DCHECK(profile_);
   DCHECK(crx_file::id_util::IdIsValid(app_id_));
-  kiosk_app_manager_observation_.Observe(KioskAppManager::Get());
 }
 
 StartupAppLauncher::~StartupAppLauncher() {
@@ -77,14 +73,15 @@ void StartupAppLauncher::ContinueWithNetworkReady() {
     return;
 
   if (delegate_->ShouldSkipAppInstallation()) {
-    FinalizeAppInstall();
+    OnInstallSuccess();
     return;
   }
 
   // The network might not be ready when KioskAppManager tries to update
   // external cache initially. Update the external cache now that the network
   // is ready for sure.
   state_ = LaunchState::kWaitingForCache;
+  kiosk_app_manager_observation_.Observe(KioskAppManager::Get());
   KioskAppManager::Get()->UpdateExternalCache();
 }
 
@@ -135,7 +132,7 @@ void StartupAppLauncher::MaybeInitializeNetwork() {
   }
 
   if (delegate_->ShouldSkipAppInstallation()) {
-    FinalizeAppInstall();
+    OnInstallSuccess();
     return;
   }
 
@@ -187,33 +184,79 @@ const extensions::Extension* StartupAppLauncher::GetPrimaryAppExtension()
       app_id_);
 }
 
+void StartupAppLauncher::BeginInstall() {
+  state_ = LaunchState::kInstallingApp;
+  delegate_->OnAppInstalling();
+  installer_ = std::make_unique<ChromeKioskAppInstaller>(
+      profile_, KioskAppManager::Get()->CreatePrimaryAppInstallData(app_id_),
+      delegate_);
+  installer_->BeginInstall(base::BindOnce(
+      &StartupAppLauncher::OnInstallComplete, weak_ptr_factory_.GetWeakPtr()));
+}
+
+void StartupAppLauncher::OnInstallComplete(
+    ChromeKioskAppInstaller::InstallResult result) {
+  DCHECK(state_ == LaunchState::kInstallingApp);
+
+  installer_.reset();
+
+  switch (result) {
+    case ChromeKioskAppInstaller::InstallResult::kSuccess:
+      OnInstallSuccess();
+      return;
+    case ChromeKioskAppInstaller::InstallResult::kUnableToInstall:
+      OnLaunchFailure(KioskAppLaunchError::Error::kUnableToInstall);
+      return;
+    case ChromeKioskAppInstaller::InstallResult::kNotKioskEnabled:
+      OnLaunchFailure(KioskAppLaunchError::Error::kNotKioskEnabled);
+      return;
+    case ChromeKioskAppInstaller::InstallResult::kNetworkMissing:
+      if (!RetryWhenNetworkIsAvailable())
+        OnLaunchFailure(KioskAppLaunchError::Error::kUnableToLaunch);
+      return;
+  }
+}
+
+void StartupAppLauncher::OnInstallSuccess() {
+  state_ = LaunchState::kReadyToLaunch;
+  // Updates to cached primary app crx will be ignored after this point, so
+  // there is no need to observe the kiosk app manager any longer.
+  kiosk_app_manager_observation_.Reset();
+  delegate_->OnAppPrepared();
+}
+
 void StartupAppLauncher::LaunchApp() {
   if (state_ != LaunchState::kReadyToLaunch) {
     NOTREACHED();
     SYSLOG(ERROR) << "LaunchApp() called but launcher is not initialized.";
   }
 
-  const Extension* extension = GetPrimaryAppExtension();
-  CHECK(extension);
+  launcher_ = std::make_unique<ChromeKioskAppLauncher>(
+      profile_, app_id_, delegate_->IsNetworkReady());
 
-  if (!extensions::KioskModeInfo::IsKioskEnabled(extension)) {
-    OnLaunchFailure(KioskAppLaunchError::Error::kNotKioskEnabled);
-    return;
-  }
-
-  SYSLOG(INFO) << "Attempt to launch app.";
+  launcher_->LaunchApp(base::BindOnce(&StartupAppLauncher::OnLaunchComplete,
+                                      weak_ptr_factory_.GetWeakPtr()));
+}
 
-  // Always open the app in a window.
-  ::OpenApplication(
-      profile_,
-      apps::AppLaunchParams(
-          extension->id(), apps::mojom::LaunchContainer::kLaunchContainerWindow,
-          WindowOpenDisposition::NEW_WINDOW,
-          apps::mojom::LaunchSource::kFromKiosk));
+void StartupAppLauncher::OnLaunchComplete(
+    ChromeKioskAppLauncher::LaunchResult result) {
+  DCHECK(state_ == LaunchState::kReadyToLaunch);
 
-  KioskAppManager::Get()->InitSession(profile_, app_id_);
+  launcher_.reset();
 
-  OnLaunchSuccess();
+  switch (result) {
+    case ChromeKioskAppLauncher::LaunchResult::kSuccess:
+      KioskAppManager::Get()->InitSession(profile_, app_id_);
+      OnLaunchSuccess();
+      return;
+    case ChromeKioskAppLauncher::LaunchResult::kUnableToLaunch:
+      OnLaunchFailure(KioskAppLaunchError::Error::kUnableToLaunch);
+      return;
+    case ChromeKioskAppLauncher::LaunchResult::kNetworkMissing:
+      if (!RetryWhenNetworkIsAvailable())
+        OnLaunchFailure(KioskAppLaunchError::Error::kUnableToLaunch);
+      return;
+  }
 }
 
 void StartupAppLauncher::OnLaunchSuccess() {
@@ -246,48 +289,4 @@ void StartupAppLauncher::OnLaunchFailure(KioskAppLaunchError::Error error) {
   delegate_->OnLaunchFailed(error);
 }
 
-void StartupAppLauncher::FinalizeAppInstall() {
-  BeginInstall(/*finalize_only=*/true);
-}
-
-void StartupAppLauncher::BeginInstall(bool finalize_only) {
-  state_ = LaunchState::kInstallingApp;
-  if (!finalize_only) {
-    delegate_->OnAppInstalling();
-  }
-  installer_ = std::make_unique<ChromeKioskAppInstaller>(
-      profile_, KioskAppManager::Get()->CreatePrimaryAppInstallData(app_id_),
-      delegate_, finalize_only);
-  installer_->BeginInstall(base::BindOnce(
-      &StartupAppLauncher::OnInstallComplete, weak_ptr_factory_.GetWeakPtr()));
-}
-
-void StartupAppLauncher::OnInstallComplete(
-    ChromeKioskAppInstaller::InstallResult result) {
-  DCHECK(state_ == LaunchState::kInstallingApp);
-
-  switch (result) {
-    case ChromeKioskAppInstaller::InstallResult::kSuccess:
-      state_ = LaunchState::kReadyToLaunch;
-      // Updates to cached primary app crx will be ignored after this point,
-      // so there is no need to observe the kiosk app manager any longer.
-      kiosk_app_manager_observation_.Reset();
-      delegate_->OnAppPrepared();
-      return;
-    case ChromeKioskAppInstaller::InstallResult::kUnableToInstall:
-      OnLaunchFailure(KioskAppLaunchError::Error::kUnableToInstall);
-      return;
-    case ChromeKioskAppInstaller::InstallResult::kNotKioskEnabled:
-      OnLaunchFailure(KioskAppLaunchError::Error::kNotKioskEnabled);
-      return;
-    case ChromeKioskAppInstaller::InstallResult::kUnableToLaunch:
-      OnLaunchFailure(KioskAppLaunchError::Error::kUnableToLaunch);
-      return;
-    case ChromeKioskAppInstaller::InstallResult::kNetworkMissing:
-      if (!RetryWhenNetworkIsAvailable())
-        OnLaunchFailure(KioskAppLaunchError::Error::kUnableToLaunch);
-      return;
-  }
-}
-
 }  // namespace ash
diff --git a/chrome/browser/ash/app_mode/startup_app_launcher.h b/chrome/browser/ash/app_mode/startup_app_launcher.h
@@ -15,6 +15,7 @@
 #include "chrome/browser/ash/app_mode/kiosk_app_manager.h"
 #include "chrome/browser/ash/app_mode/kiosk_app_manager_observer.h"
 #include "chrome/browser/chromeos/app_mode/chrome_kiosk_app_installer.h"
+#include "chrome/browser/chromeos/app_mode/chrome_kiosk_app_launcher.h"
 #include "chrome/browser/extensions/install_observer.h"
 #include "chrome/browser/extensions/install_tracker.h"
 #include "extensions/browser/app_window/app_window_registry.h"
@@ -60,16 +61,18 @@ class StartupAppLauncher : public KioskAppLauncher,
   // KioskAppLauncher:
   void Initialize() override;
   void ContinueWithNetworkReady() override;
-  void LaunchApp() override;
   void RestartLauncher() override;
+  void LaunchApp() override;
+
+  void BeginInstall();
+  void OnInstallComplete(ChromeKioskAppInstaller::InstallResult result);
+  void OnInstallSuccess();
+
+  void OnLaunchComplete(ChromeKioskAppLauncher::LaunchResult result);
 
   void OnLaunchSuccess();
   void OnLaunchFailure(KioskAppLaunchError::Error error);
 
-  void FinalizeAppInstall();
-  void BeginInstall(bool finalize_only = false);
-  void OnInstallComplete(ChromeKioskAppInstaller::InstallResult result);
-
   void MaybeInitializeNetwork();
   bool RetryWhenNetworkIsAvailable();
   void OnKioskAppDataLoadStatusChanged(const std::string& app_id);
@@ -89,6 +92,7 @@ class StartupAppLauncher : public KioskAppLauncher,
   LaunchState state_ = LaunchState::kNotStarted;
 
   std::unique_ptr<ChromeKioskAppInstaller> installer_;
+  std::unique_ptr<ChromeKioskAppLauncher> launcher_;
 
   extensions::AppWindowRegistry* window_registry_;
 

diff --git a/chrome/browser/ash/app_mode/startup_app_launcher_unittest.cc b/chrome/browser/ash/app_mode/startup_app_launcher_unittest.cc
@@ -55,6 +55,7 @@ using extensions::mojom::ManifestLocation;
 using ::testing::AssertionFailure;
 using ::testing::AssertionResult;
 using ::testing::AssertionSuccess;
+using ::testing::ElementsAre;
 
 namespace ash {
 
@@ -847,15 +848,15 @@ TEST_F(StartupAppLauncherTest, LaunchWithSecondaryApps) {
 
   EXPECT_FALSE(kiosk_app_session_initialized_);
 
+  startup_app_launcher_->LaunchApp();
+
   EXPECT_TRUE(registry()->enabled_extensions().Contains(kTestPrimaryAppId));
   EXPECT_TRUE(registry()->enabled_extensions().Contains(kSecondaryAppId));
   EXPECT_TRUE(registry()->disabled_extensions().Contains(kExtraSecondaryAppId));
   EXPECT_EQ(extensions::disable_reason::DISABLE_USER_ACTION,
             extensions::ExtensionPrefs::Get(browser_context())
                 ->GetDisableReasons(kExtraSecondaryAppId));
 
-  startup_app_launcher_->LaunchApp();
-
   EXPECT_EQ(std::vector<LaunchState>({LaunchState::kLaunchSucceeded}),
             startup_launch_delegate_.launch_state_changes());
   EXPECT_EQ(1, app_launch_tracker_->kiosk_launch_count());
@@ -999,13 +1000,19 @@ TEST_F(StartupAppLauncherTest,
 
   startup_app_launcher_->Initialize();
 
-  EXPECT_EQ(std::vector<LaunchState>({LaunchState::kInstallingApp}),
-            startup_launch_delegate_.launch_state_changes());
+  EXPECT_THAT(startup_launch_delegate_.launch_state_changes(),
+              ElementsAre(LaunchState::kInstallingApp));
   startup_launch_delegate_.ClearLaunchStateChanges();
 
   ASSERT_TRUE(FinishPrimaryAppInstall(primary_app_builder));
 
-  // After install is complete we should realize that the app is not offline
+  EXPECT_THAT(startup_launch_delegate_.launch_state_changes(),
+              ElementsAre(LaunchState::kReadyToLaunch));
+  startup_launch_delegate_.ClearLaunchStateChanges();
+
+  startup_app_launcher_->LaunchApp();
+
+  // When trying to launch app we should realize that the app is not offline
   // enabled and request a network connection.
   startup_launch_delegate_.WaitForLaunchStates(
       {LaunchState::kInitializingNetwork});

diff --git a/chrome/browser/ash/app_mode/test_kiosk_extension_builder.h b/chrome/browser/ash/app_mode/test_kiosk_extension_builder.h
@@ -20,6 +20,7 @@ namespace ash {
 
 // Wrapper around extensions::ExtensionBuilder for creating extension::Extension
 // instances for usage in kiosk app tests.
+// TODO(b/227985497): Turn this into a proper builder
 class TestKioskExtensionBuilder {
  public:
   TestKioskExtensionBuilder(extensions::Manifest::Type type,

diff --git a/chrome/browser/chromeos/BUILD.gn b/chrome/browser/chromeos/BUILD.gn
@@ -3410,6 +3410,8 @@ source_set("chromeos") {
     "app_mode/app_session.h",
     "app_mode/chrome_kiosk_app_installer.cc",
     "app_mode/chrome_kiosk_app_installer.h",
+    "app_mode/chrome_kiosk_app_launcher.cc",
+    "app_mode/chrome_kiosk_app_launcher.h",
     "app_mode/chrome_kiosk_external_loader_broker.cc",
     "app_mode/chrome_kiosk_external_loader_broker.h",
     "app_mode/kiosk_app_external_loader.cc",
@@ -4777,6 +4779,7 @@ source_set("unit_tests") {
     "../policy/default_geolocation_policy_handler_unittest.cc",
     "../ui/browser_finder_chromeos_unittest.cc",
     "app_mode/app_session_unittest.cc",
+    "app_mode/chrome_kiosk_app_launcher_unittest.cc",
     "app_mode/kiosk_session_plugin_handler_unittest.cc",
     "extensions/active_tab_permission_granter_delegate_chromeos_unittest.cc",
     "extensions/default_app_order_unittest.cc",