Skip to content

Commit

Permalink
Move waiting for app window into launcher
Browse files Browse the repository at this point in the history 
When Lacros is active, Lacros browser needs to be the one waiting for
app window, so we need to move this part into the launcher.

Bug: b/226560767
Change-Id: If833a47a0070443c58b2ccff9311202c5708bfe7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3641709
Reviewed-by: Jeroen Dhollander <jeroendh@google.com>
Commit-Queue: Ben Franz <bfranz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1003665}
  • Loading branch information
Ben Franz authored and Chromium LUCI CQ committed May 16, 2022
1 parent b3aa1d5 commit f831099
Show file tree
Hide file tree
Showing 6 changed files with 154 additions and 54 deletions.
28 changes: 3 additions & 25 deletions chrome/browser/ash/app_mode/startup_app_launcher.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@
#include "chrome/browser/profiles/profile.h"
#include "chrome/common/chrome_switches.h"
#include "components/crx_file/id_util.h"
#include "extensions/browser/app_window/app_window.h"
#include "extensions/browser/app_window/app_window_registry.h"
#include "net/base/load_flags.h"

namespace ash {
Expand All @@ -44,10 +42,7 @@ StartupAppLauncher::StartupAppLauncher(Profile* profile,
DCHECK(crx_file::id_util::IdIsValid(app_id_));
}

StartupAppLauncher::~StartupAppLauncher() {
if (state_ == LaunchState::kWaitingForWindow)
window_registry_->RemoveObserver(this);
}
StartupAppLauncher::~StartupAppLauncher() = default;

void StartupAppLauncher::Initialize() {
DCHECK(state_ != LaunchState::kReadyToLaunch &&
Expand Down Expand Up @@ -241,26 +236,9 @@ void StartupAppLauncher::OnLaunchComplete(
}

void StartupAppLauncher::OnLaunchSuccess() {
state_ = LaunchState::kLaunchSucceeded;
delegate_->OnAppLaunched();
state_ = LaunchState::kWaitingForWindow;

window_registry_ = extensions::AppWindowRegistry::Get(profile_);
// Start waiting for app window.
if (!window_registry_->GetAppWindowsForApp(app_id_).empty()) {
delegate_->OnAppWindowCreated();
state_ = LaunchState::kLaunchSucceeded;
return;
} else {
window_registry_->AddObserver(this);
}
}

void StartupAppLauncher::OnAppWindowAdded(extensions::AppWindow* app_window) {
if (app_window->extension_id() == app_id_) {
state_ = LaunchState::kLaunchSucceeded;
window_registry_->RemoveObserver(this);
delegate_->OnAppWindowCreated();
}
delegate_->OnAppWindowCreated();
}

void StartupAppLauncher::OnLaunchFailure(KioskAppLaunchError::Error error) {
Expand Down
13 changes: 1 addition & 12 deletions chrome/browser/ash/app_mode/startup_app_launcher.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,14 @@
#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 "extensions/browser/app_window/app_window_registry.h"

class Profile;

namespace extensions {
class AppWindowRegistry;
}

namespace ash {

// Responsible for the startup of the app for Chrome App kiosk.
class StartupAppLauncher : public KioskAppLauncher,
public KioskAppManagerObserver,
public extensions::AppWindowRegistry::Observer {
public KioskAppManagerObserver {
public:
StartupAppLauncher(Profile* profile,
const std::string& app_id,
Expand Down Expand Up @@ -71,9 +65,6 @@ class StartupAppLauncher : public KioskAppLauncher,
bool RetryWhenNetworkIsAvailable();
void OnKioskAppDataLoadStatusChanged(const std::string& app_id);

// AppWindowRegistry::Observer:
void OnAppWindowAdded(extensions::AppWindow* app_window) override;

// KioskAppManagerObserver overrides.
void OnKioskExtensionLoadedInCache(const std::string& app_id) override;
void OnKioskExtensionDownloadFailed(const std::string& app_id) override;
Expand All @@ -86,8 +77,6 @@ class StartupAppLauncher : public KioskAppLauncher,
std::unique_ptr<ChromeKioskAppInstaller> installer_;
std::unique_ptr<ChromeKioskAppLauncher> launcher_;

extensions::AppWindowRegistry* window_registry_;

base::ScopedObservation<KioskAppManagerBase, KioskAppManagerObserver>
kiosk_app_manager_observation_{this};

Expand Down
71 changes: 58 additions & 13 deletions chrome/browser/ash/app_mode/startup_app_launcher_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <vector>

#include "ash/components/settings/cros_settings_names.h"
#include "ash/test/ash_test_helper.h"
#include "base/callback.h"
#include "base/command_line.h"
#include "base/files/file_path.h"
Expand All @@ -33,10 +34,13 @@
#include "chrome/browser/extensions/external_provider_impl.h"
#include "chrome/browser/extensions/install_tracker.h"
#include "chrome/browser/extensions/pending_extension_manager.h"
#include "chrome/browser/ui/apps/chrome_app_delegate.h"
#include "chrome/common/chrome_switches.h"
#include "components/account_id/account_id.h"
#include "components/user_manager/scoped_user_manager.h"
#include "components/version_info/channel.h"
#include "content/public/browser/browser_context.h"
#include "extensions/browser/app_window/test_app_window_contents.h"
#include "extensions/browser/extension_prefs.h"
#include "extensions/browser/extension_registry.h"
#include "extensions/browser/external_install_info.h"
Expand Down Expand Up @@ -333,18 +337,56 @@ class TestKioskLoaderVisitor
std::set<std::string> pending_update_urls_;
};

void InitAppWindow(extensions::AppWindow* app_window, const gfx::Rect& bounds) {
// Create a TestAppWindowContents for the ShellAppDelegate to initialize the
// ShellExtensionWebContentsObserver with.
std::unique_ptr<content::WebContents> web_contents(
content::WebContents::Create(
content::WebContents::CreateParams(app_window->browser_context())));
auto app_window_contents =
std::make_unique<extensions::TestAppWindowContents>(
std::move(web_contents));

// Initialize the web contents and AppWindow.
app_window->app_delegate()->InitWebContents(
app_window_contents->GetWebContents());

content::RenderFrameHost* main_frame =
app_window_contents->GetWebContents()->GetMainFrame();
DCHECK(main_frame);

extensions::AppWindow::CreateParams params;
params.content_spec.bounds = bounds;
app_window->Init(GURL(), app_window_contents.release(), main_frame, params);
}

extensions::AppWindow* CreateAppWindow(Profile* profile,
const TestKioskExtensionBuilder& builder,
gfx::Rect bounds = {}) {
extensions::AppWindow* app_window = new extensions::AppWindow(
profile, new ChromeAppDelegate(profile, true), builder.Build().get());
InitAppWindow(app_window, bounds);
return app_window;
}

} // namespace

class StartupAppLauncherTest : public extensions::ExtensionServiceTestBase,
public KioskAppManager::Overrides {
public:
StartupAppLauncherTest() = default;
StartupAppLauncherTest()
: extensions::ExtensionServiceTestBase(
std::make_unique<content::BrowserTaskEnvironment>(
content::BrowserTaskEnvironment::REAL_IO_THREAD)) {}

StartupAppLauncherTest(const StartupAppLauncherTest&) = delete;
StartupAppLauncherTest& operator=(const StartupAppLauncherTest&) = delete;
~StartupAppLauncherTest() override = default;

// testing::Test:
void SetUp() override {
ash_test_helper_.SetUp();

command_line_.GetProcessCommandLine()->AppendSwitch(
switches::kForceAppMode);
command_line_.GetProcessCommandLine()->AppendSwitch(switches::kAppId);
Expand All @@ -355,7 +397,6 @@ class StartupAppLauncherTest : public extensions::ExtensionServiceTestBase,

extensions::ExtensionServiceTestBase::SetUp();

InitializeKioskAppUser();
InitializeEmptyExtensionService();
external_apps_loader_handler_ = std::make_unique<TestKioskLoaderVisitor>(
browser_context(), registry(), service());
Expand Down Expand Up @@ -385,6 +426,8 @@ class StartupAppLauncherTest : public extensions::ExtensionServiceTestBase,
accounts_settings_helper_.reset();

extensions::ExtensionServiceTestBase::TearDown();

ash_test_helper_.TearDown();
}

// KioskAppManager::Overrides:
Expand Down Expand Up @@ -568,17 +611,6 @@ class StartupAppLauncherTest : public extensions::ExtensionServiceTestBase,
}

protected:
void InitializeKioskAppUser() {
const AccountId kiosk_account_id(
AccountId::FromUserEmail(kTestUserAccount));
auto fake_user_manager_ = std::make_unique<FakeChromeUserManager>();
fake_user_manager_->AddKioskAppUser(kiosk_account_id);
fake_user_manager_->LoginUser(kiosk_account_id);

user_manager_enabler_ = std::make_unique<user_manager::ScopedUserManager>(
std::move(fake_user_manager_));
}

TestAppLaunchDelegate startup_launch_delegate_;

std::unique_ptr<KioskAppLauncher> startup_app_launcher_;
Expand All @@ -590,6 +622,7 @@ class StartupAppLauncherTest : public extensions::ExtensionServiceTestBase,
bool kiosk_app_session_initialized_ = false;

private:
ash::AshTestHelper ash_test_helper_;
base::test::ScopedCommandLine command_line_;

std::unique_ptr<ScopedCrosSettingsTestHelper> accounts_settings_helper_;
Expand Down Expand Up @@ -630,6 +663,7 @@ TEST_F(StartupAppLauncherTest, PrimaryAppLaunchFlow) {

EXPECT_FALSE(kiosk_app_session_initialized_);
startup_app_launcher_->LaunchApp();
CreateAppWindow(profile(), primary_app_builder);

EXPECT_EQ(std::vector<LaunchState>({LaunchState::kLaunchSucceeded}),
startup_launch_delegate_.launch_state_changes());
Expand Down Expand Up @@ -672,6 +706,7 @@ TEST_F(StartupAppLauncherTest, OfflineLaunchWithPrimaryAppPreInstalled) {
EXPECT_TRUE(startup_launch_delegate_.launch_state_changes().empty());

startup_app_launcher_->LaunchApp();
CreateAppWindow(profile(), primary_app_builder);

EXPECT_EQ(std::vector<LaunchState>({LaunchState::kLaunchSucceeded}),
startup_launch_delegate_.launch_state_changes());
Expand Down Expand Up @@ -704,6 +739,7 @@ TEST_F(StartupAppLauncherTest,
EXPECT_FALSE(kiosk_app_session_initialized_);

startup_app_launcher_->LaunchApp();
CreateAppWindow(profile(), primary_app_builder);

EXPECT_EQ(std::vector<LaunchState>({LaunchState::kLaunchSucceeded}),
startup_launch_delegate_.launch_state_changes());
Expand Down Expand Up @@ -844,6 +880,7 @@ TEST_F(StartupAppLauncherTest, LaunchWithSecondaryApps) {
EXPECT_FALSE(kiosk_app_session_initialized_);

startup_app_launcher_->LaunchApp();
CreateAppWindow(profile(), primary_app_builder);

EXPECT_TRUE(registry()->enabled_extensions().Contains(kTestPrimaryAppId));
EXPECT_TRUE(registry()->enabled_extensions().Contains(kSecondaryAppId));
Expand Down Expand Up @@ -893,6 +930,7 @@ TEST_F(StartupAppLauncherTest, LaunchWithSecondaryExtension) {

EXPECT_FALSE(kiosk_app_session_initialized_);
startup_app_launcher_->LaunchApp();
CreateAppWindow(profile(), primary_app_builder);

EXPECT_EQ(std::vector<LaunchState>({LaunchState::kLaunchSucceeded}),
startup_launch_delegate_.launch_state_changes());
Expand Down Expand Up @@ -944,6 +982,7 @@ TEST_F(StartupAppLauncherTest, OfflineWithPrimaryAndSecondaryAppInstalled) {
EXPECT_TRUE(startup_launch_delegate_.launch_state_changes().empty());

startup_app_launcher_->LaunchApp();
CreateAppWindow(profile(), primary_app_builder);

EXPECT_EQ(std::vector<LaunchState>({LaunchState::kLaunchSucceeded}),
startup_launch_delegate_.launch_state_changes());
Expand Down Expand Up @@ -977,6 +1016,7 @@ TEST_F(StartupAppLauncherTest, OfflineInstallPreCachedExtension) {
startup_launch_delegate_.ClearLaunchStateChanges();

startup_app_launcher_->LaunchApp();
CreateAppWindow(profile(), primary_app_builder);

EXPECT_EQ(std::vector<LaunchState>({LaunchState::kLaunchSucceeded}),
startup_launch_delegate_.launch_state_changes());
Expand Down Expand Up @@ -1006,6 +1046,7 @@ TEST_F(StartupAppLauncherTest,
startup_launch_delegate_.ClearLaunchStateChanges();

startup_app_launcher_->LaunchApp();
CreateAppWindow(profile(), primary_app_builder);

// When trying to launch app we should realize that the app is not offline
// enabled and request a network connection.
Expand All @@ -1026,6 +1067,7 @@ TEST_F(StartupAppLauncherTest,
startup_launch_delegate_.ClearLaunchStateChanges();

startup_app_launcher_->LaunchApp();
CreateAppWindow(profile(), primary_app_builder);

EXPECT_EQ(std::vector<LaunchState>({LaunchState::kLaunchSucceeded}),
startup_launch_delegate_.launch_state_changes());
Expand Down Expand Up @@ -1079,6 +1121,7 @@ TEST_F(StartupAppLauncherTest,
startup_launch_delegate_.ClearLaunchStateChanges();

startup_app_launcher_->LaunchApp();
CreateAppWindow(profile(), primary_app_builder);

EXPECT_EQ(std::vector<LaunchState>({LaunchState::kLaunchSucceeded}),
startup_launch_delegate_.launch_state_changes());
Expand Down Expand Up @@ -1115,6 +1158,7 @@ TEST_F(StartupAppLauncherTest,
startup_launch_delegate_.ClearLaunchStateChanges();

startup_app_launcher_->LaunchApp();
CreateAppWindow(profile(), primary_app_builder);

EXPECT_THAT(startup_launch_delegate_.launch_state_changes(),
ElementsAre(LaunchState::kLaunchSucceeded));
Expand Down Expand Up @@ -1148,6 +1192,7 @@ TEST_F(StartupAppLauncherTest, IgnoreSecondaryAppsSecondaryApps) {

EXPECT_FALSE(kiosk_app_session_initialized_);
startup_app_launcher_->LaunchApp();
CreateAppWindow(profile(), primary_app_builder);

EXPECT_EQ(std::vector<LaunchState>({LaunchState::kLaunchSucceeded}),
startup_launch_delegate_.launch_state_changes());
Expand Down
23 changes: 22 additions & 1 deletion chrome/browser/chromeos/app_mode/chrome_kiosk_app_launcher.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/extensions/application_launch.h"
#include "extensions/browser/app_window/app_window.h"
#include "extensions/browser/app_window/app_window_registry.h"
#include "extensions/browser/disable_reason.h"
#include "extensions/browser/extension_system.h"
#include "extensions/common/extension_id.h"
Expand Down Expand Up @@ -92,7 +94,25 @@ void ChromeKioskAppLauncher::LaunchApp(LaunchCallback callback) {
WindowOpenDisposition::NEW_WINDOW,
apps::mojom::LaunchSource::kFromKiosk));

ReportLaunchSuccess();
WaitForAppWindow();
}

void ChromeKioskAppLauncher::WaitForAppWindow() {
auto* window_registry_ = extensions::AppWindowRegistry::Get(profile_);
if (!window_registry_->GetAppWindowsForApp(app_id_).empty()) {
ReportLaunchSuccess();
} else {
// Start waiting for app window.
app_window_observation_.Observe(window_registry_);
}
}

void ChromeKioskAppLauncher::OnAppWindowAdded(
extensions::AppWindow* app_window) {
if (app_window->extension_id() == app_id_) {
app_window_observation_.Reset();
ReportLaunchSuccess();
}
}

void ChromeKioskAppLauncher::MaybeUpdateAppData() {
Expand All @@ -108,6 +128,7 @@ void ChromeKioskAppLauncher::MaybeUpdateAppData() {
}

void ChromeKioskAppLauncher::ReportLaunchSuccess() {
SYSLOG(INFO) << "App launch completed";
std::move(on_ready_callback_)
.Run(ChromeKioskAppLauncher::LaunchResult::kSuccess);
}
Expand Down
Loading

0 comments on commit f831099

Please sign in to comment.