Skip to content

Commit

Permalink
iwa: Enable isolated web apps by enterprise policy
Browse files Browse the repository at this point in the history
This CL enables IWAs by the enterprise policy. So far this policy is
available for trusted testers only.

Before this CL we just check the feature flag. After this CL we also
check:
 - code in //chrome and //contents in fact use an overridden method in
   the content browser client;
 - code in render process check a command line flag. We use command
   line flags to propagate policy in the render process.

Design doc: go/iwa-mgs

Bug: 1364132
Change-Id: Iad7ea1367d737b0a46dcf93ec6d63a4ed353e2a2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4181324
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Oleksandr Peletskyi <peletskyi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1100982}
  • Loading branch information
peletskyi authored and Chromium LUCI CQ committed Feb 3, 2023
1 parent aff9f93 commit f2e557c
Show file tree
Hide file tree
Showing 17 changed files with 130 additions and 34 deletions.
41 changes: 34 additions & 7 deletions chrome/browser/chrome_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,7 @@
#include "content/public/browser/child_process_security_policy.h"
#include "content/public/browser/client_certificate_delegate.h"
#include "content/public/browser/file_url_loader.h"
#include "content/public/browser/isolated_web_apps_policy.h"
#include "content/public/browser/navigation_handle.h"
#include "content/public/browser/navigation_throttle.h"
#include "content/public/browser/overlay_window.h"
Expand Down Expand Up @@ -2458,7 +2459,9 @@ bool ChromeContentBrowserClient::ShouldUrlUseApplicationIsolationLevel(
const GURL& url,
bool origin_matches_flag) {
#if BUILDFLAG(ENABLE_EXTENSIONS)
if (!base::FeatureList::IsEnabled(features::kIsolatedWebApps)) {

if (!content::IsolatedWebAppsPolicy::AreIsolatedWebAppsEnabled(
browser_context)) {
return false;
}

Expand Down Expand Up @@ -2820,6 +2823,11 @@ void ChromeContentBrowserClient::AppendExtraCommandLineSwitches(
command_line->AppendSwitch(commerce::switches::kEnableChromeCart);
}
#endif

if (content::IsolatedWebAppsPolicy::AreIsolatedWebAppsEnabled(
process->GetBrowserContext())) {
command_line->AppendSwitch(switches::kEnableIsolatedWebAppsInRenderer);
}
}

MaybeAppendBlinkSettingsSwitchForFieldTrial(browser_command_line,
Expand Down Expand Up @@ -5490,7 +5498,8 @@ void ChromeContentBrowserClient::RegisterNonNetworkNavigationURLLoaderFactories(
profile, content::ChildProcessHost::kInvalidUniqueID));
#endif // BUILDFLAG(IS_CHROMEOS_ASH)
#if !BUILDFLAG(IS_ANDROID)
if (base::FeatureList::IsEnabled(features::kIsolatedWebApps) &&
if (content::IsolatedWebAppsPolicy::AreIsolatedWebAppsEnabled(
browser_context) &&
!browser_context->ShutdownStarted()) {
// TODO(crbug.com/1365848): Only register the factory if we are already in
// an isolated storage partition.
Expand Down Expand Up @@ -5529,7 +5538,8 @@ void ChromeContentBrowserClient::
DCHECK(factories);

#if !BUILDFLAG(IS_ANDROID)
if (base::FeatureList::IsEnabled(features::kIsolatedWebApps) &&
if (content::IsolatedWebAppsPolicy::AreIsolatedWebAppsEnabled(
browser_context) &&
!browser_context->ShutdownStarted()) {
factories->emplace(
chrome::kIsolatedAppScheme,
Expand Down Expand Up @@ -5765,14 +5775,16 @@ void ChromeContentBrowserClient::
#endif // BUILDFLAG(IS_CHROMEOS_ASH)

#if !BUILDFLAG(IS_ANDROID)
if (base::FeatureList::IsEnabled(features::kIsolatedWebApps)) {
{
content::BrowserContext* browser_context =
content::RenderProcessHost::FromID(render_process_id)
->GetBrowserContext();
DCHECK(browser_context);
if (!browser_context->ShutdownStarted()) {
// TODO(crbug.com/1365848): Only register the factory if we are already in
// an isolated storage partition.
if (content::IsolatedWebAppsPolicy::AreIsolatedWebAppsEnabled(
browser_context) &&
!browser_context->ShutdownStarted()) {
// TODO(crbug.com/1365848): Only register the factory if we are already
// in an isolated storage partition.

if (frame_host != nullptr) {
factories->emplace(
Expand Down Expand Up @@ -7441,6 +7453,21 @@ bool ChromeContentBrowserClient::IsFileSystemURLNavigationAllowed(
return false;
}

bool ChromeContentBrowserClient::AreIsolatedWebAppsEnabled(
content::BrowserContext* browser_context) {
#if BUILDFLAG(IS_CHROMEOS)
// Check if the enterprise policy that regulates Isolated Web Apps force
// installing is present. If it is there then the IWAs should be enabled.
Profile* profile = Profile::FromBrowserContext(browser_context);
const base::Value::List& isolated_web_apps =
profile->GetPrefs()->GetList(prefs::kIsolatedWebAppInstallForceList);
if (!isolated_web_apps.empty()) {
return true;
}
#endif
return base::FeatureList::IsEnabled(features::kIsolatedWebApps);
}

#if BUILDFLAG(IS_MAC)
base::FilePath ChromeContentBrowserClient::GetChildProcessPath(
int child_flags,
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/chrome_content_browser_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -846,6 +846,9 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
content::BrowserContext* browser_context,
const GURL& url) override;

bool AreIsolatedWebAppsEnabled(
content::BrowserContext* browser_context) override;

protected:
static bool HandleWebUI(GURL* url, content::BrowserContext* browser_context);
static bool HandleWebUIReverse(GURL* url,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,14 @@
#include <memory>

#include "base/check_deref.h"
#include "base/feature_list.h"
#include "base/functional/bind.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/web_applications/isolated_web_apps/isolated_web_app_trust_checker.h"
#include "chrome/browser/web_applications/isolated_web_apps/isolated_web_app_validator.h"
#include "chrome/browser/web_applications/web_app_utils.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h"
#include "components/web_package/signed_web_bundles/signed_web_bundle_signature_verifier.h"
#include "content/public/common/content_features.h"
#include "content/public/browser/isolated_web_apps_policy.h"

namespace web_app {

Expand Down Expand Up @@ -62,7 +61,7 @@ KeyedService* IsolatedWebAppReaderRegistryFactory::BuildServiceInstanceFor(
content::BrowserContext*
IsolatedWebAppReaderRegistryFactory::GetBrowserContextToUse(
content::BrowserContext* context) const {
if (!base::FeatureList::IsEnabled(features::kIsolatedWebApps)) {
if (!content::IsolatedWebAppsPolicy::AreIsolatedWebAppsEnabled(context)) {
return nullptr;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,6 @@ IsolatedWebAppTrustChecker::~IsolatedWebAppTrustChecker() = default;
IsolatedWebAppTrustChecker::Result IsolatedWebAppTrustChecker::IsTrusted(
const web_package::SignedWebBundleId& expected_web_bundle_id,
const web_package::SignedWebBundleIntegrityBlock& integrity_block) const {
if (!base::FeatureList::IsEnabled(features::kIsolatedWebApps)) {
return {.status = Result::Status::kErrorFeatureDisabled,
.message = "Support for Isolated Web Apps is not enabled."};
}

if (expected_web_bundle_id.type() !=
web_package::SignedWebBundleId::Type::kEd25519PublicKey) {
return {.status = Result::Status::kErrorUnsupportedWebBundleIdType,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,6 @@ class IsolatedWebAppPolicyManagerAshBrowserTest

~IsolatedWebAppPolicyManagerAshBrowserTest() override = default;

void SetUp() override {
scoped_feature_list_.InitWithFeatures({features::kIsolatedWebApps}, {});
policy::DevicePolicyCrosBrowserTest::SetUp();
}

void SetUpCommandLine(base::CommandLine* command_line) override {
DevicePolicyCrosBrowserTest::SetUpCommandLine(command_line);
command_line->AppendSwitch(ash::switches::kLoginManager);
Expand Down
6 changes: 5 additions & 1 deletion chrome/renderer/chrome_content_renderer_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,11 @@ void ChromeContentRendererClient::RenderThreadStarted() {
WebString chrome_search_scheme(
WebString::FromASCII(chrome::kChromeSearchScheme));

if (base::FeatureList::IsEnabled(features::kIsolatedWebApps)) {
// IWAs can be enabled by either the feature flag or by enterprise
// policy. In either case the kEnableIsolatedWebAppsInRenderer flag is passed
// to the renderer process.
if (base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kEnableIsolatedWebAppsInRenderer)) {
// isolated-app: is the scheme used for Isolated Web Apps, which are web
// applications packaged in Signed Web Bundles.
WebString isolated_app_scheme(
Expand Down
9 changes: 8 additions & 1 deletion content/browser/renderer_host/isolated_web_app_throttle.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "content/browser/web_exposed_isolation_info.h"
#include "content/common/navigation_params_utils.h"
#include "content/public/browser/content_browser_client.h"
#include "content/public/browser/isolated_web_apps_policy.h"
#include "content/public/browser/navigation_entry.h"
#include "content/public/browser/navigation_handle.h"
#include "content/public/browser/render_frame_host.h"
Expand Down Expand Up @@ -68,7 +69,13 @@ absl::optional<url::SchemeHostPort> GetTupleFromOptionalOrigin(
// static
std::unique_ptr<IsolatedWebAppThrottle>
IsolatedWebAppThrottle::MaybeCreateThrottleFor(NavigationHandle* handle) {
if (base::FeatureList::IsEnabled(features::kIsolatedWebApps)) {
BrowserContext* browser_context = NavigationRequest::From(handle)
->frame_tree_node()
->navigator()
.controller()
.GetBrowserContext();

if (IsolatedWebAppsPolicy::AreIsolatedWebAppsEnabled(browser_context)) {
return std::make_unique<IsolatedWebAppThrottle>(handle);
}
return nullptr;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,10 @@
// found in the LICENSE file.

#include "base/memory/raw_ptr.h"
#include "base/test/scoped_feature_list.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/content_browser_client.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/common/content_client.h"
#include "content/public/common/content_features.h"
#include "content/public/test/browser_test.h"
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/content_browser_test.h"
Expand Down Expand Up @@ -48,6 +47,8 @@ class IsolatedWebAppContentBrowserClient : public ContentBrowserClient {
return url.host() == kAppHost;
}

bool AreIsolatedWebAppsEnabled(BrowserContext*) override { return true; }

private:
url::Origin app_origin_;
};
Expand All @@ -56,9 +57,7 @@ class IsolatedWebAppContentBrowserClient : public ContentBrowserClient {

class HttpsBrowserTest : public ContentBrowserTest {
public:
HttpsBrowserTest() : https_server_(net::EmbeddedTestServer::TYPE_HTTPS) {
scoped_feature_list_.InitAndEnableFeature(features::kIsolatedWebApps);
}
HttpsBrowserTest() : https_server_(net::EmbeddedTestServer::TYPE_HTTPS) {}

void SetUpCommandLine(base::CommandLine* command_line) override {
ContentBrowserTest::SetUpCommandLine(command_line);
Expand Down Expand Up @@ -88,8 +87,6 @@ class HttpsBrowserTest : public ContentBrowserTest {
private:
net::EmbeddedTestServer https_server_;
ContentMockCertVerifier mock_cert_verifier_;

base::test::ScopedFeatureList scoped_feature_list_;
};

class IsolatedWebAppThrottleBrowserTest : public HttpsBrowserTest {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@

#include "base/command_line.h"
#include "base/memory/raw_ptr.h"
#include "base/test/scoped_feature_list.h"
#include "content/browser/renderer_host/frame_tree_node.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/content_browser_client.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/content_client.h"
Expand Down Expand Up @@ -76,6 +76,8 @@ class IsolatedWebAppContentBrowserClient : public ContentBrowserClient {
last_page_transition_ = ui::PageTransition::PAGE_TRANSITION_QUALIFIER_MASK;
}

bool AreIsolatedWebAppsEnabled(BrowserContext*) override { return true; }

private:
unsigned int external_protocol_call_count_ = 0;
ui::PageTransition last_page_transition_ =
Expand All @@ -89,8 +91,6 @@ class IsolatedWebAppThrottleTest : public RenderViewHostTestHarness {
void SetUp() override {
RenderViewHostTestHarness::SetUp();

scoped_feature_list_.InitAndEnableFeature(features::kIsolatedWebApps);

old_client_ = SetBrowserClientForTesting(&test_client_);

// COOP/COEP headers must be sent to enable application isolation.
Expand Down Expand Up @@ -211,8 +211,6 @@ class IsolatedWebAppThrottleTest : public RenderViewHostTestHarness {
raw_ptr<ContentBrowserClient> old_client_;
scoped_refptr<net::HttpResponseHeaders> coop_coep_headers_;
scoped_refptr<net::HttpResponseHeaders> corp_coep_headers_;

base::test::ScopedFeatureList scoped_feature_list_;
};

TEST_F(IsolatedWebAppThrottleTest, AllowNavigationWithinNonApp) {
Expand Down
3 changes: 3 additions & 0 deletions content/child/runtime_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,10 @@ void SetRuntimeFeaturesFromCommandLine(const base::CommandLine& command_line) {
{wrf::EnableWebGPUDeveloperFeatures,
switches::kEnableWebGPUDeveloperFeatures, true},
{wrf::EnableDirectSockets, switches::kIsolatedAppOrigins, true},
{wrf::EnableDirectSockets, switches::kEnableIsolatedWebAppsInRenderer,
true},
};

for (const auto& mapping : switchToFeatureMapping) {
if (command_line.HasSwitch(mapping.switch_name)) {
mapping.feature_enabler(mapping.target_enabled_state);
Expand Down
2 changes: 2 additions & 0 deletions content/public/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,8 @@ source_set("browser_sources") {
"installed_payment_apps_finder.h",
"interest_group_manager.h",
"invalidate_type.h",
"isolated_web_apps_policy.cc",
"isolated_web_apps_policy.h",
"jank_monitor.h",
"javascript_dialog_manager.cc",
"javascript_dialog_manager.h",
Expand Down
7 changes: 7 additions & 0 deletions content/public/browser/content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1433,4 +1433,11 @@ base::FilePath ContentBrowserClient::GetChildProcessPath(
}
#endif

bool ContentBrowserClient::AreIsolatedWebAppsEnabled(
BrowserContext* browser_context) {
// The whole logic of the IWAs lives in //chrome. So IWAs should be
// enabled at that layer.
return false;
}

} // namespace content
4 changes: 4 additions & 0 deletions content/public/browser/content_browser_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -2386,6 +2386,10 @@ class CONTENT_EXPORT ContentBrowserClient {
int child_flags,
const base::FilePath& helpers_path);
#endif // BUILDFLAG(IS_MAC)

// Checks if Isolated Web Apps are enabled, e.g. by feature flag
// or in any other way.
virtual bool AreIsolatedWebAppsEnabled(BrowserContext* browser_context);
};

} // namespace content
Expand Down
16 changes: 16 additions & 0 deletions content/public/browser/isolated_web_apps_policy.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "content/public/browser/isolated_web_apps_policy.h"

#include "content/public/browser/content_browser_client.h"
#include "content/public/common/content_client.h"

namespace content {
// static
bool IsolatedWebAppsPolicy::AreIsolatedWebAppsEnabled(
BrowserContext* browser_context) {
return GetContentClient()->browser()->AreIsolatedWebAppsEnabled(
browser_context);
}
} // namespace content
29 changes: 29 additions & 0 deletions content/public/browser/isolated_web_apps_policy.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef CONTENT_PUBLIC_BROWSER_ISOLATED_WEB_APPS_POLICY_H_
#define CONTENT_PUBLIC_BROWSER_ISOLATED_WEB_APPS_POLICY_H_

#include "content/common/content_export.h"

namespace content {

class BrowserContext;

// A centralized place for making a decision about Isolated Web Apps (IWAs).
// For more information about IWAs, see:
// https://github.com/WICG/isolated-web-apps/blob/main/README.md

class CONTENT_EXPORT IsolatedWebAppsPolicy {
public:
IsolatedWebAppsPolicy(const IsolatedWebAppsPolicy&) = delete;
IsolatedWebAppsPolicy& operator=(const IsolatedWebAppsPolicy&) = delete;
IsolatedWebAppsPolicy() = delete;

// Returns true if Isolated Web Apps are enabled.
static bool AreIsolatedWebAppsEnabled(BrowserContext* browser_context);
};

} // namespace content

#endif // CONTENT_PUBLIC_BROWSER_ISOLATED_WEB_APPS_POLICY_H_
9 changes: 9 additions & 0 deletions content/public/common/content_switches.cc
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,15 @@ const char kDisableOriginTrialControlledBlinkFeatures[] =
const char kEnableGpuMemoryBufferVideoFrames[] =
"enable-gpu-memory-buffer-video-frames";

// Enables Isolated Web Apps (IWAs) in a renderer process. There are two ways
// to enable the IWAs: by feature flag and by enterprise policy. If IWAs are
// enabled by any of the mentioned above ways then this flag is passed to
// the renderer process. This flag should not be used from command line.
// To enable IWAs from command line one should use kIsolatedWebApps feature
// flag.
const char kEnableIsolatedWebAppsInRenderer[] =
"enable-isolated-web-apps-in-renderer";

// Force logging to be enabled. Logging is disabled by default in release
// builds.
const char kEnableLogging[] = "enable-logging";
Expand Down
1 change: 1 addition & 0 deletions content/public/common/content_switches.h
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ CONTENT_EXPORT extern const char kEnableExperimentalWebPlatformFeatures[];
CONTENT_EXPORT extern const char kEnableFakeNoAllocDirectCallForTesting[];
CONTENT_EXPORT extern const char kEnableBlinkTestFeatures[];
CONTENT_EXPORT extern const char kEnableGpuMemoryBufferVideoFrames[];
CONTENT_EXPORT extern const char kEnableIsolatedWebAppsInRenderer[];
CONTENT_EXPORT extern const char kEnableLCDText[];
CONTENT_EXPORT extern const char kEnableLogging[];
CONTENT_EXPORT extern const char kEnableNetworkInformationDownlinkMax[];
Expand Down

0 comments on commit f2e557c

Please sign in to comment.