Skip to content

Commit

Permalink
Enable WebAppProvider in Kiosk session
Browse files Browse the repository at this point in the history
WebAppProvider was disabled in Kiosk due to conflict with Kiosk
accessibility menu (https://crbug.com/1180380). This CL disables the
creation of system web app handlers in Kiosk so that we can enable
WebAppProvider and use it to install web apps in Kiosk. Changes are
guarded by feature flag KioskEnableAppService.

Bug: b:240493670
Change-Id: I93822f399667fe5b18a78347fb5c908fba8293ae
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3833515
Reviewed-by: Jiewei Qian <qjw@chromium.org>
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Commit-Queue: Yi Xie <yixie@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1035974}
  • Loading branch information
imxieyi authored and Chromium LUCI CQ committed Aug 17, 2022
1 parent 0db0116 commit 73b5b5b
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 3 deletions.
11 changes: 10 additions & 1 deletion chrome/browser/ash/system_web_apps/system_web_app_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
#include "chrome/browser/ash/web_applications/terminal_system_web_app_info.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profiles_state.h"
#include "chrome/browser/web_applications/external_install_options.h"
#include "chrome/browser/web_applications/manifest_update_manager.h"
#include "chrome/browser/web_applications/policy/web_app_policy_manager.h"
Expand All @@ -76,6 +77,7 @@
#include "chrome/browser/web_applications/web_app_system_web_app_delegate_map_utils.h"
#include "chrome/browser/web_applications/web_app_ui_manager.h"
#include "chrome/browser/web_applications/web_app_utils.h"
#include "chrome/common/chrome_features.h"
#include "chrome/common/webui_url_constants.h"
#include "chrome/grit/generated_resources.h"
#include "chromeos/strings/grit/chromeos_strings.h" // nogncheck
Expand Down Expand Up @@ -217,7 +219,14 @@ SystemWebAppManager::SystemWebAppManager(Profile* profile)
// Always create delegates because many System Web App WebUIs are disabled
// when the delegate is not present and we need them in tests. Tests can
// override the list of delegates with SetSystemAppsForTesting().
system_app_delegates_ = CreateSystemWebApps(profile_);
//
// TODO(https://crbug.com/1353262): SWAM is not supported in Kiosk mode. Many
// components assume that SWAM always exists alongside WebAppProvider. We want
// to use WebAppProvider to install web apps in Kiosk without enabling SWAM.
if (!base::FeatureList::IsEnabled(::features::kKioskEnableAppService) ||
!profiles::IsKioskSession()) {
system_app_delegates_ = CreateSystemWebApps(profile_);
}

#if defined(OFFICIAL_BUILD)
const bool is_official = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
#include "chrome/browser/web_applications/web_app_utils.h"
#include "chrome/common/chrome_features.h"
#include "chrome/test/base/chrome_render_view_host_test_harness.h"
#include "chromeos/login/login_state/login_state.h"
#include "components/webapps/browser/install_result_code.h"
#include "content/public/browser/navigation_handle.h"
#include "content/public/test/test_utils.h"
Expand Down Expand Up @@ -1587,4 +1588,54 @@ TEST_F(SystemWebAppManagerTest, DestroyUiManager) {
run_loop.Run();
}

class SystemWebAppManagerInKioskTest : public ChromeRenderViewHostTestHarness {
public:
template <typename... TaskEnvironmentTraits>
explicit SystemWebAppManagerInKioskTest(TaskEnvironmentTraits&&... traits)
: ChromeRenderViewHostTestHarness(
std::forward<TaskEnvironmentTraits>(traits)...) {}
SystemWebAppManagerInKioskTest(const SystemWebAppManagerInKioskTest&) =
delete;
SystemWebAppManagerInKioskTest& operator=(
const SystemWebAppManagerInKioskTest&) = delete;

~SystemWebAppManagerInKioskTest() override = default;

void SetUp() override {
ChromeRenderViewHostTestHarness::SetUp();
scoped_feature_list_ = std::make_unique<base::test::ScopedFeatureList>();
scoped_feature_list_->InitAndEnableFeature(
::features::kKioskEnableAppService);

chromeos::LoginState::Initialize();
chromeos::LoginState::Get()->SetLoggedInState(
chromeos::LoginState::LOGGED_IN_ACTIVE,
chromeos::LoginState::LOGGED_IN_USER_KIOSK);

system_web_app_manager_ = std::make_unique<SystemWebAppManager>(profile());
}

void TearDown() override {
system_web_app_manager_.reset();
scoped_feature_list_.reset();
chromeos::LoginState::Shutdown();
ChromeRenderViewHostTestHarness::TearDown();
}

protected:
SystemWebAppManager& system_web_app_manager() {
return *system_web_app_manager_;
}

private:
std::unique_ptr<base::test::ScopedFeatureList> scoped_feature_list_;
std::unique_ptr<SystemWebAppManager> system_web_app_manager_;
};

// Checks that SWA delegates are not created in Kiosk sessions.
TEST_F(SystemWebAppManagerInKioskTest, ShoudNotCreateDelegate) {
EXPECT_EQ(system_web_app_manager().GetSystemApp(SystemWebAppType::SETTINGS),
nullptr);
}

} // namespace ash
6 changes: 4 additions & 2 deletions chrome/browser/web_applications/web_app_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -186,9 +186,11 @@ bool AreWebAppsEnabled(const Profile* profile) {
if (!ash::ProfileHelper::IsRegularProfile(original_profile)) {
return false;
}
// Disable Web Apps if running any kiosk app.
// Disable Web Apps if running any kiosk app and kKioskEnableAppService is not
// enabled.
auto* user_manager = user_manager::UserManager::Get();
if (user_manager && user_manager->IsLoggedInAsAnyKioskApp()) {
if (user_manager && user_manager->IsLoggedInAsAnyKioskApp() &&
!base::FeatureList::IsEnabled(features::kKioskEnableAppService)) {
return false;
}
#elif BUILDFLAG(IS_CHROMEOS_LACROS)
Expand Down
11 changes: 11 additions & 0 deletions chrome/browser/web_applications/web_app_utils_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@

#include "base/containers/adapters.h"
#include "base/files/file_path.h"
#include "base/test/scoped_feature_list.h"
#include "build/chromeos_buildflags.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/web_applications/test/web_app_test.h"
#include "chrome/browser/web_applications/user_display_mode.h"
#include "chrome/browser/web_applications/web_app_install_info.h"
#include "chrome/common/chrome_constants.h"
#include "chrome/common/chrome_features.h"
#include "chrome/test/base/testing_browser_process.h"
#include "chrome/test/base/testing_profile_manager.h"
#include "testing/gmock/include/gmock/gmock.h"
Expand Down Expand Up @@ -251,6 +253,15 @@ TEST_F(WebAppUtilsTest, AreWebAppsEnabled) {
user_manager::ScopedUserManager enabler(std::move(user_manager));
EXPECT_FALSE(AreWebAppsEnabled(regular_profile));
}
{
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(features::kKioskEnableAppService);
auto user_manager = std::make_unique<MockUserManager>();
EXPECT_CALL(*user_manager, IsLoggedInAsKioskApp())
.WillOnce(testing::Return(true));
user_manager::ScopedUserManager enabler(std::move(user_manager));
EXPECT_TRUE(AreWebAppsEnabled(regular_profile));
}
#endif
}

Expand Down

0 comments on commit 73b5b5b

Please sign in to comment.