Skip to content

Commit

Permalink
Create KioskLauncherFactory
Browse files Browse the repository at this point in the history
This change is in preparation for a change in the business logic that will allow us to create the KioskLaunchController on the fly.

WANT_LGTM=all

Bug: b:256596278
Test: KioskLaunchControllerTest
Change-Id: I7a57e0f2849366d6c6104bf10b997bc0e6397bc5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4189443
Reviewed-by: Jeroen Dhollander <jeroendh@google.com>
Reviewed-by: Leonid Baraz <lbaraz@chromium.org>
Reviewed-by: Michael Ershov <miersh@google.com>
Commit-Queue: Ben Franz <bfranz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1099749}
  • Loading branch information
bfranz authored and Chromium LUCI CQ committed Feb 1, 2023
1 parent be65034 commit 0dc0893
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 71 deletions.
119 changes: 67 additions & 52 deletions chrome/browser/ash/login/app_mode/kiosk_launch_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,19 @@
#include "ash/public/cpp/login_accelerators.h"
#include "base/check_is_test.h"
#include "base/feature_list.h"
#include "base/functional/bind.h"
#include "base/functional/callback.h"
#include "base/functional/callback_helpers.h"
#include "base/memory/ptr_util.h"
#include "base/metrics/histogram_functions.h"
#include "base/metrics/histogram_macros.h"
#include "base/notreached.h"
#include "base/syslog_logging.h"
#include "chrome/browser/ash/app_mode/arc/arc_kiosk_app_manager.h"
#include "chrome/browser/ash/app_mode/arc/arc_kiosk_app_service.h"
#include "chrome/browser/ash/app_mode/kiosk_app_launcher.h"
#include "chrome/browser/ash/app_mode/kiosk_app_manager.h"
#include "chrome/browser/ash/app_mode/kiosk_app_types.h"
#include "chrome/browser/ash/app_mode/startup_app_launcher.h"
#include "chrome/browser/ash/app_mode/web_app/web_kiosk_app_launcher.h"
#include "chrome/browser/ash/app_mode/web_app/web_kiosk_app_manager.h"
Expand Down Expand Up @@ -149,6 +154,36 @@ class ArcKioskAppServiceWrapper : public KioskAppLauncher {
ArcKioskAppService* const service_;
};

std::unique_ptr<KioskAppLauncher> BuildKioskAppLauncher(
Profile* profile,
const KioskAppId& kiosk_app_id,
KioskAppLauncher::NetworkDelegate* network_delegate) {
switch (kiosk_app_id.type) {
case KioskAppType::kArcApp:
// ArcKioskAppService lifetime is bound to the profile, therefore
// wrap it into a separate object.
return std::make_unique<ArcKioskAppServiceWrapper>(
ArcKioskAppService::Get(profile), network_delegate);
case KioskAppType::kChromeApp:
return std::make_unique<StartupAppLauncher>(
profile, kiosk_app_id.app_id.value(), /*should_skip_install=*/false,
network_delegate);
case KioskAppType::kWebApp:
// TODO(b/242023891): |WebKioskAppServiceLauncher| does not support
// Lacros until App Service installation API is available.
if (base::FeatureList::IsEnabled(features::kKioskEnableAppService) &&
!crosapi::browser_util::IsLacrosEnabled()) {
return std::make_unique<WebKioskAppServiceLauncher>(
profile, kiosk_app_id.account_id.value(), network_delegate);
} else {
return std::make_unique<WebKioskAppLauncher>(
profile, kiosk_app_id.account_id.value(),
/*should_skip_install=*/false, network_delegate);
}
}
NOTREACHED();
}

} // namespace

const char kKioskLaunchStateCrashKey[] = "kiosk-launch-state";
Expand All @@ -175,10 +210,17 @@ void SetKioskLaunchStateCrashKey(KioskLaunchState state) {
}

KioskLaunchController::KioskLaunchController(OobeUI* oobe_ui)
: host_(LoginDisplayHost::default_host()),
splash_screen_view_(oobe_ui->GetView<AppLaunchSplashScreenHandler>()) {}

KioskLaunchController::KioskLaunchController() : host_(nullptr) {}
: KioskLaunchController(LoginDisplayHost::default_host(),
oobe_ui->GetView<AppLaunchSplashScreenHandler>(),
base::BindRepeating(&BuildKioskAppLauncher)) {}

KioskLaunchController::KioskLaunchController(
LoginDisplayHost* host,
AppLaunchSplashScreenView* splash_screen,
KioskAppLauncherFactory app_launcher_factory)
: host_(host),
splash_screen_view_(splash_screen),
app_launcher_factory_(std::move(app_launcher_factory)) {}

KioskLaunchController::~KioskLaunchController() {
if (splash_screen_view_) {
Expand Down Expand Up @@ -253,6 +295,8 @@ bool KioskLaunchController::HandleAccelerator(LoginAcceleratorAction action) {

void KioskLaunchController::OnProfileLoaded(Profile* profile) {
SYSLOG(INFO) << "Profile loaded... Starting app launch.";
DCHECK(!profile_) << "OnProfileLoaded called twice";
profile_ = profile;

// Call `ClearMigrationStep()` once per signin so that the check for migration
// is run exactly once per signin. Check the comment for `kMigrationStep` in
Expand All @@ -272,51 +316,33 @@ void KioskLaunchController::OnProfileLoaded(Profile* profile) {
return;
}

profile_ = profile;

// This is needed to trigger input method extensions being loaded.
profile->InitChromeOSPreferences();

InitializeKeyboard();
InitializeLauncher();

if (network_ui_state_ == NetworkUIState::kNeedToShow) {
ShowNetworkConfigureUI();
}
}

void KioskLaunchController::InitializeKeyboard() {
// Reset virtual keyboard to use IME engines in app profile early.
ChromeKeyboardControllerClient::Get()->RebuildKeyboardIfEnabled();

// Do not set update `app_launcher_` if has been set.
if (!app_launcher_) {
switch (kiosk_app_id_.type) {
case KioskAppType::kArcApp:
// ArcKioskAppService lifetime is bound to the profile, therefore
// wrap it into a separate object.
app_launcher_ = std::make_unique<ArcKioskAppServiceWrapper>(
ArcKioskAppService::Get(profile_), /*delegate=*/this);
break;
case KioskAppType::kChromeApp:
app_launcher_ = std::make_unique<StartupAppLauncher>(
profile_, *kiosk_app_id_.app_id, /*should_skip_install=*/false,
/*delegate=*/this);
break;
case KioskAppType::kWebApp:
// Make keyboard config sync with the `VirtualKeyboardFeatures` policy.
ChromeKeyboardControllerClient::Get()->SetKeyboardConfigFromPref(true);
// TODO(b/242023891): |WebKioskAppServiceLauncher| does not support
// Lacros until App Service installation API is available.
if (base::FeatureList::IsEnabled(features::kKioskEnableAppService) &&
!crosapi::browser_util::IsLacrosEnabled()) {
app_launcher_ = std::make_unique<WebKioskAppServiceLauncher>(
profile, *kiosk_app_id_.account_id, /*delegate=*/this);
} else {
app_launcher_ = std::make_unique<WebKioskAppLauncher>(
profile, *kiosk_app_id_.account_id,
/*should_skip_install=*/false, /*delegate=*/this);
}
break;
}
if (kiosk_app_id_.type == KioskAppType::kWebApp) {
// Make keyboard config sync with the `VirtualKeyboardFeatures`
// policy.
ChromeKeyboardControllerClient::Get()->SetKeyboardConfigFromPref(true);
}
app_launcher_observation_.Observe(app_launcher_.get());
}

void KioskLaunchController::InitializeLauncher() {
DCHECK(!app_launcher_);

app_launcher_ = app_launcher_factory_.Run(profile_, kiosk_app_id_, this);
app_launcher_observation_.Observe(app_launcher_.get());
app_launcher_->Initialize();
if (network_ui_state_ == NetworkUIState::kNeedToShow) {
ShowNetworkConfigureUI();
}
}

void KioskLaunchController::OnConfigureNetwork() {
Expand Down Expand Up @@ -879,15 +905,4 @@ void KioskLaunchController::
need_owner_auth_to_configure_network_callback = callback;
}

// static
std::unique_ptr<KioskLaunchController> KioskLaunchController::CreateForTesting(
AppLaunchSplashScreenView* view,
std::unique_ptr<KioskAppLauncher> app_launcher) {
std::unique_ptr<KioskLaunchController> controller(
new KioskLaunchController());
controller->splash_screen_view_ = view;
controller->app_launcher_ = std::move(app_launcher);
return controller;
}

} // namespace ash
34 changes: 24 additions & 10 deletions chrome/browser/ash/login/app_mode/kiosk_launch_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <memory>

#include "ash/public/cpp/login_accelerators.h"
#include "base/functional/callback_forward.h"
#include "base/observer_list.h"
#include "base/observer_list_types.h"
#include "base/scoped_observation.h"
Expand Down Expand Up @@ -92,7 +93,19 @@ class KioskLaunchController : public KioskProfileLoader::Delegate,

using ReturnBoolCallback = base::RepeatingCallback<bool()>;

// Factory class that constructs a `KioskAppLauncher`.
// The default implementation constructs the correct implementation of
// `KioskAppLauncher` based on the kiosk type associated with `KioskAppId`.
using KioskAppLauncherFactory =
base::RepeatingCallback<std::unique_ptr<KioskAppLauncher>(
Profile*,
const KioskAppId&,
KioskAppLauncher::NetworkDelegate*)>;

explicit KioskLaunchController(OobeUI* oobe_ui);
KioskLaunchController(LoginDisplayHost* host,
AppLaunchSplashScreenView* splash_screen,
KioskAppLauncherFactory app_launcher_factory);
KioskLaunchController(const KioskLaunchController&) = delete;
KioskLaunchController& operator=(const KioskLaunchController&) = delete;
~KioskLaunchController() override;
Expand All @@ -112,10 +125,6 @@ class KioskLaunchController : public KioskProfileLoader::Delegate,
static void SetNeedOwnerAuthToConfigureNetworkCallbackForTesting(
ReturnBoolCallback* callback);

static std::unique_ptr<KioskLaunchController> CreateForTesting(
AppLaunchSplashScreenView* view,
std::unique_ptr<KioskAppLauncher> app_launcher);

bool waiting_for_network() const {
return app_state_ == AppState::kInitNetwork;
}
Expand Down Expand Up @@ -153,10 +162,10 @@ class KioskLaunchController : public KioskProfileLoader::Delegate,
kShowing // Network configure UI is being shown.
};

KioskLaunchController();

void OnCancelAppLaunch();
void OnNetworkConfigRequested();
void InitializeKeyboard();
void InitializeLauncher();

// `AppLaunchSplashScreenView::Delegate`
void OnConfigureNetwork() override;
Expand Down Expand Up @@ -218,10 +227,15 @@ class KioskLaunchController : public KioskProfileLoader::Delegate,
// Current state of network configure dialog.
NetworkUIState network_ui_state_ = NetworkUIState::kNotShowing;

LoginDisplayHost* const host_; // Not owned, destructed upon shutdown.
AppLaunchSplashScreenView* splash_screen_view_ = nullptr; // Owned by OobeUI.
KioskAppId kiosk_app_id_; // Current app.
Profile* profile_ = nullptr; // Not owned.
// Not owned, destructed upon shutdown.
raw_ptr<LoginDisplayHost> const host_;
// Owned by OobeUI.
raw_ptr<AppLaunchSplashScreenView> splash_screen_view_ = nullptr;
// Current app.
KioskAppId kiosk_app_id_;
// Not owned.
raw_ptr<Profile> profile_ = nullptr;
const KioskAppLauncherFactory app_launcher_factory_;

// Whether app should be launched as soon as it is ready.
bool launch_on_install_ = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include <memory>

#include "base/functional/bind.h"
#include "base/memory/scoped_refptr.h"
#include "base/test/metrics/histogram_tester.h"
#include "base/test/task_environment.h"
Expand Down Expand Up @@ -101,12 +102,12 @@ class KioskLaunchControllerTest : public extensions::ExtensionServiceTestBase {
disable_wait_timer_and_login_operations_for_testing_ =
KioskLaunchController::DisableWaitTimerAndLoginOperationsForTesting();

auto app_launcher = std::make_unique<FakeKioskAppLauncher>();
app_launcher_ = app_launcher.get();

view_ = std::make_unique<FakeAppLaunchSplashScreenHandler>();
controller_ = KioskLaunchController::CreateForTesting(
view_.get(), std::move(app_launcher));
controller_ = std::make_unique<KioskLaunchController>(
/*host=*/nullptr, view_.get(),
base::BindRepeating(
&KioskLaunchControllerTest::BuildFakeKioskAppLauncher,
base::Unretained(this)));

// We can't call `crash_reporter::ResetCrashKeysForTesting()` to reset crash
// keys since it destroys the storage for static crash keys. Instead we set
Expand Down Expand Up @@ -134,6 +135,8 @@ class KioskLaunchControllerTest : public extensions::ExtensionServiceTestBase {

FakeKioskAppLauncher& launcher() { return *app_launcher_; }

int num_launchers_created() { return app_launchers_created_; }

auto HasState(AppState app_state, NetworkUIState network_state) {
return testing::AllOf(
testing::Field("app_state", &KioskLaunchController::app_state_,
Expand Down Expand Up @@ -183,6 +186,16 @@ class KioskLaunchControllerTest : public extensions::ExtensionServiceTestBase {
}

private:
std::unique_ptr<KioskAppLauncher> BuildFakeKioskAppLauncher(
Profile*,
const KioskAppId& kiosk_app_id,
KioskAppLauncher::NetworkDelegate*) {
app_launchers_created_++;
auto app_launcher = std::make_unique<FakeKioskAppLauncher>();
app_launcher_ = app_launcher.get();
return std::move(app_launcher);
}

TestingProfile profile_;
session_manager::SessionManager session_manager_;
std::unique_ptr<ChromeKeyboardControllerClientTestHelper>
Expand All @@ -192,7 +205,8 @@ class KioskLaunchControllerTest : public extensions::ExtensionServiceTestBase {
std::unique_ptr<base::AutoReset<bool>>
disable_wait_timer_and_login_operations_for_testing_;
std::unique_ptr<FakeAppLaunchSplashScreenHandler> view_;
FakeKioskAppLauncher* app_launcher_; // owned by `controller_`.
FakeKioskAppLauncher* app_launcher_ = nullptr; // owned by `controller_`.
int app_launchers_created_ = 0;
std::unique_ptr<KioskLaunchController> controller_;
KioskAppId kiosk_app_id_;
};
Expand Down Expand Up @@ -369,6 +383,7 @@ TEST_F(KioskLaunchControllerTest, KioskProfileLoadFailedObserverShouldBeFired) {

controller().RemoveKioskProfileLoadFailedObserver(
&profile_load_failed_observer);
EXPECT_EQ(num_launchers_created(), 0);
}

TEST_F(KioskLaunchControllerTest, KioskProfileLoadErrorShouldBeStored) {
Expand Down Expand Up @@ -629,11 +644,12 @@ TEST_F(KioskLaunchControllerWithExtensionTest,
TEST_F(KioskLaunchControllerTest, TestFullFlow) {
SetOnline(true);

EXPECT_FALSE(launcher().IsInitialized());
EXPECT_FALSE(launcher().HasAppLaunched());
EXPECT_FALSE(launcher().HasContinueWithNetworkReadyBeenCalled());
EXPECT_EQ(num_launchers_created(), 0);

controller().Start(kiosk_app_id(), /*auto_launch=*/false);

EXPECT_EQ(num_launchers_created(), 0);

profile_controls().OnProfileLoaded(profile());

EXPECT_EQ(launcher().initialize_called(), 1);
Expand All @@ -655,5 +671,6 @@ TEST_F(KioskLaunchControllerTest, TestFullFlow) {
EXPECT_EQ(launcher().initialize_called(), 1);
EXPECT_EQ(launcher().continue_with_network_ready_called(), 1);
EXPECT_EQ(launcher().launch_app_called(), 1);
EXPECT_EQ(num_launchers_created(), 1);
}
} // namespace ash

0 comments on commit 0dc0893

Please sign in to comment.