Skip to content

Commit

Permalink
[Desktop Android CRX] Enable the chrome.test API
Browse files Browse the repository at this point in the history
We are experimenting with desktop-android configurations for Chrome.

This CL enables extension runtimes in Chrome in the experimental
desktop-android build to call extension APIs and enables the
chrome.test API (which is an API that implements a JS testing
framework, used extensively in extension tests).

This involves adding a few missing pieces needed for the API system
(such as an ExtensionWebContentsObserver), properly instantiating the
APIs, and further adjusting the content browser client behavior to
properly handle extension URLs on desktop-android (replacing
ENABLE_EXTENSIONS with ENABLE_EXTENSIONS_CORE).

After this, the general extensions API system is functional, including
renderer side bindings, custom binding JS injection, serialization of
arguments, and handling on the browser side.

Update a browsertest to exercise the same.

This CL should have no production behavior change, since the
desktop-android configuration is still highly experimental.

Cq-Include-Trybots: luci.chromium.try:android-desktop-arm64-compile-rel,android-desktop-x64-compile-rel

Bug: 356905053
Change-Id: I5f83b3b72a351461555e458b705fb6d90b227874
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5827799
Commit-Queue: Devlin Cronin <[email protected]>
Reviewed-by: David Bertoni <[email protected]>
Reviewed-by: Lei Zhang <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1351015}
  • Loading branch information
rdcronin authored and Chromium LUCI CQ committed Sep 4, 2024
1 parent 92cf594 commit e2df8cf
Show file tree
Hide file tree
Showing 9 changed files with 179 additions and 28 deletions.
2 changes: 2 additions & 0 deletions chrome/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -7752,6 +7752,8 @@ static_library("browser") {
# They should be removed.
"extensions/desktop_android/desktop_android_extension_system.cc",
"extensions/desktop_android/desktop_android_extension_system.h",
"extensions/desktop_android/desktop_android_extension_web_contents_observer.cc",
"extensions/desktop_android/desktop_android_extension_web_contents_observer.h",
"extensions/desktop_android/desktop_android_extensions_browser_client.cc",
"extensions/desktop_android/desktop_android_extensions_browser_client.h",
]
Expand Down
32 changes: 16 additions & 16 deletions chrome/browser/chrome_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2049,7 +2049,7 @@ GURL ChromeContentBrowserClient::GetEffectiveURL(
return search::GetEffectiveURLForInstant(url, profile);
#endif

#if BUILDFLAG(ENABLE_EXTENSIONS)
#if BUILDFLAG(ENABLE_EXTENSIONS_CORE)
if (ChromeContentBrowserClientExtensionsPart::AreExtensionsDisabledForProfile(
profile))
return url;
Expand All @@ -2070,7 +2070,7 @@ bool ChromeContentBrowserClient::
const GURL& destination_url) {
DCHECK(browser_context);
DCHECK(candidate_site_instance);
#if BUILDFLAG(ENABLE_EXTENSIONS)
#if BUILDFLAG(ENABLE_EXTENSIONS_CORE)
if (ChromeContentBrowserClientExtensionsPart::AreExtensionsDisabledForProfile(
browser_context)) {
return true;
Expand Down Expand Up @@ -2104,7 +2104,7 @@ bool ChromeContentBrowserClient::ShouldUseProcessPerSite(
return true;
#endif

#if BUILDFLAG(ENABLE_EXTENSIONS)
#if BUILDFLAG(ENABLE_EXTENSIONS_CORE)
if (ChromeContentBrowserClientExtensionsPart::ShouldUseProcessPerSite(
profile, site_url))
return true;
Expand Down Expand Up @@ -2177,7 +2177,7 @@ ChromeContentBrowserClient::ShouldUseSpareRenderProcessHost(
}
#endif

#if BUILDFLAG(ENABLE_EXTENSIONS)
#if BUILDFLAG(ENABLE_EXTENSIONS_CORE)
if (!ChromeContentBrowserClientExtensionsPart::
ShouldUseSpareRenderProcessHost(profile, site_url)) {
return SpareProcessRefusedByEmbedderReason::ExtensionProcess;
Expand All @@ -2190,7 +2190,7 @@ bool ChromeContentBrowserClient::DoesSiteRequireDedicatedProcess(
content::BrowserContext* browser_context,
const GURL& effective_site_url) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
#if BUILDFLAG(ENABLE_EXTENSIONS)
#if BUILDFLAG(ENABLE_EXTENSIONS_CORE)
if (ChromeContentBrowserClientExtensionsPart::DoesSiteRequireDedicatedProcess(
browser_context, effective_site_url)) {
return true;
Expand All @@ -2205,7 +2205,7 @@ bool ChromeContentBrowserClient::
const GURL& precursor,
const GURL& url) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
#if BUILDFLAG(ENABLE_EXTENSIONS)
#if BUILDFLAG(ENABLE_EXTENSIONS_CORE)
if (!ChromeContentBrowserClientExtensionsPart::
ShouldAllowCrossProcessSandboxedFrameForPrecursor(browser_context,
precursor, url)) {
Expand Down Expand Up @@ -2291,7 +2291,7 @@ void ChromeContentBrowserClient::OverrideURLLoaderFactoryParams(
factory_params->provide_loading_state_updates = false;
#endif

#if BUILDFLAG(ENABLE_EXTENSIONS)
#if BUILDFLAG(ENABLE_EXTENSIONS_CORE)
if (ChromeContentBrowserClientExtensionsPart::AreExtensionsDisabledForProfile(
browser_context)) {
return;
Expand Down Expand Up @@ -2363,7 +2363,7 @@ bool ChromeContentBrowserClient::HasCustomSchemeHandler(
bool ChromeContentBrowserClient::CanCommitURL(
content::RenderProcessHost* process_host,
const GURL& url) {
#if BUILDFLAG(ENABLE_EXTENSIONS)
#if BUILDFLAG(ENABLE_EXTENSIONS_CORE)
return ChromeContentBrowserClientExtensionsPart::CanCommitURL(process_host,
url);
#else
Expand Down Expand Up @@ -2441,7 +2441,7 @@ bool ChromeContentBrowserClient::IsSuitableHost(
}
#endif

#if BUILDFLAG(ENABLE_EXTENSIONS)
#if BUILDFLAG(ENABLE_EXTENSIONS_CORE)
return ChromeContentBrowserClientExtensionsPart::IsSuitableHost(
profile, process_host, site_url);
#else
Expand All @@ -2466,7 +2466,7 @@ bool ChromeContentBrowserClient::MayReuseHost(
}

size_t ChromeContentBrowserClient::GetProcessCountToIgnoreForLimit() {
#if BUILDFLAG(ENABLE_EXTENSIONS)
#if BUILDFLAG(ENABLE_EXTENSIONS_CORE)
return ChromeContentBrowserClientExtensionsPart::
GetProcessCountToIgnoreForLimit();
#else
Expand Down Expand Up @@ -2517,7 +2517,7 @@ bool ChromeContentBrowserClient::ShouldTryToUseExistingProcessHost(

bool ChromeContentBrowserClient::ShouldEmbeddedFramesTryToReuseExistingProcess(
content::RenderFrameHost* outermost_main_frame) {
#if BUILDFLAG(ENABLE_EXTENSIONS)
#if BUILDFLAG(ENABLE_EXTENSIONS_CORE)
return ChromeContentBrowserClientExtensionsPart::
ShouldEmbeddedFramesTryToReuseExistingProcess(outermost_main_frame);
#else
Expand Down Expand Up @@ -2555,7 +2555,7 @@ bool ChromeContentBrowserClient::ShouldSwapBrowsingInstancesForNavigation(
SiteInstance* site_instance,
const GURL& current_effective_url,
const GURL& destination_effective_url) {
#if BUILDFLAG(ENABLE_EXTENSIONS)
#if BUILDFLAG(ENABLE_EXTENSIONS_CORE)
return ChromeContentBrowserClientExtensionsPart::
ShouldSwapBrowsingInstancesForNavigation(
site_instance, current_effective_url, destination_effective_url);
Expand All @@ -2578,7 +2578,7 @@ ChromeContentBrowserClient::GetOriginsRequiringDedicatedProcess() {
isolated_origin_list.push_back(GaiaUrls::GetInstance()->gaia_origin());
}

#if BUILDFLAG(ENABLE_EXTENSIONS)
#if BUILDFLAG(ENABLE_EXTENSIONS_CORE)
auto origins_from_extensions = ChromeContentBrowserClientExtensionsPart::
GetOriginsRequiringDedicatedProcess();
std::move(std::begin(origins_from_extensions),
Expand Down Expand Up @@ -3181,7 +3181,7 @@ ChromeContentBrowserClient::AllowServiceWorker(
DCHECK_CURRENTLY_ON(BrowserThread::UI);
GURL first_party_url = top_frame_origin ? top_frame_origin->GetURL() : GURL();

#if BUILDFLAG(ENABLE_EXTENSIONS)
#if BUILDFLAG(ENABLE_EXTENSIONS_CORE)
// Check if this is an extension-related service worker, and, if so, if it's
// allowed (this can return false if, e.g., the extension is disabled).
// If it's not allowed, return immediately. We deliberately do *not* report
Expand All @@ -3208,7 +3208,7 @@ bool ChromeContentBrowserClient::MayDeleteServiceWorkerRegistration(
DCHECK(browser_context);
DCHECK_CURRENTLY_ON(BrowserThread::UI);

#if BUILDFLAG(ENABLE_EXTENSIONS)
#if BUILDFLAG(ENABLE_EXTENSIONS_CORE)
if (!ChromeContentBrowserClientExtensionsPart::
MayDeleteServiceWorkerRegistration(scope, browser_context)) {
return false;
Expand All @@ -3224,7 +3224,7 @@ bool ChromeContentBrowserClient::ShouldTryToUpdateServiceWorkerRegistration(
DCHECK(browser_context);
DCHECK_CURRENTLY_ON(BrowserThread::UI);

#if BUILDFLAG(ENABLE_EXTENSIONS)
#if BUILDFLAG(ENABLE_EXTENSIONS_CORE)
if (!ChromeContentBrowserClientExtensionsPart::
ShouldTryToUpdateServiceWorkerRegistration(scope, browser_context)) {
return false;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright 2024 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "chrome/browser/extensions/desktop_android/desktop_android_extension_web_contents_observer.h"

namespace extensions {

DesktopAndroidExtensionWebContentsObserver::
DesktopAndroidExtensionWebContentsObserver(
content::WebContents* web_contents)
: ExtensionWebContentsObserver(web_contents),
content::WebContentsUserData<DesktopAndroidExtensionWebContentsObserver>(
*web_contents) {}

DesktopAndroidExtensionWebContentsObserver::
~DesktopAndroidExtensionWebContentsObserver() = default;

void DesktopAndroidExtensionWebContentsObserver::CreateForWebContents(
content::WebContents* web_contents) {
content::WebContentsUserData<DesktopAndroidExtensionWebContentsObserver>::
CreateForWebContents(web_contents);

// Initialize this instance if necessary.
FromWebContents(web_contents)->Initialize();
}

WEB_CONTENTS_USER_DATA_KEY_IMPL(DesktopAndroidExtensionWebContentsObserver);

} // namespace extensions
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
// Copyright 2024 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef CHROME_BROWSER_EXTENSIONS_DESKTOP_ANDROID_DESKTOP_ANDROID_EXTENSION_WEB_CONTENTS_OBSERVER_H_
#define CHROME_BROWSER_EXTENSIONS_DESKTOP_ANDROID_DESKTOP_ANDROID_EXTENSION_WEB_CONTENTS_OBSERVER_H_

#include "content/public/browser/web_contents_user_data.h"
#include "extensions/browser/extension_web_contents_observer.h"

namespace extensions {

////////////////////////////////////////////////////////////////////////////////
// S T O P
// ALL THIS CODE WILL BE DELETED.
// THINK TWICE (OR THRICE) BEFORE ADDING MORE.
//
// The details:
// This is part of an experimental desktop-android build and allows us to
// bootstrap the extension system by incorporating a lightweight extensions
// runtime into the chrome binary. This allows us to do things like load
// extensions in tests and exercise code in these builds without needing to have
// the entirety of the //chrome/browser/extensions system either compiled and
// implemented (which is a massive undertaking) or gracefully if-def'd out
// (which is a massive amount of technical debt).
// This approach, by comparison, allows us to have a minimal interface in the
// chrome browser that mostly relies on the top-level //extensions layer, along
// with small bits of the //chrome code that compile cleanly on the
// experimental desktop-android build.
//
// This entire class should go away. Instead of adding new functionality here,
// it should be added in a location that can be shared across desktop-android
// and other desktop builds. In practice, this means:
// * Pulling the code up to //extensions. If it can be cleanly segmented from
// the //chrome layer, this is preferable. It gets cleanly included across
// all builds, encourages proper separation of concerns, and reduces the
// interdependency between features.
// * Including the functionality in the desktop-android build. This can be done
// for //chrome sources that do not have any dependencies on areas that
// cannot be included in desktop-android (such as dependencies on `Browser`
// or native UI code).
//
// TODO(https://crbug.com/356905053): Delete this class once desktop-android
// properly leverages the extension system.
////////////////////////////////////////////////////////////////////////////////
class DesktopAndroidExtensionWebContentsObserver
: public ExtensionWebContentsObserver,
public content::WebContentsUserData<
DesktopAndroidExtensionWebContentsObserver> {
public:
DesktopAndroidExtensionWebContentsObserver(
const DesktopAndroidExtensionWebContentsObserver&) = delete;
DesktopAndroidExtensionWebContentsObserver& operator=(
const DesktopAndroidExtensionWebContentsObserver&) = delete;

~DesktopAndroidExtensionWebContentsObserver() override;

// Creates and initializes an instance of this class for the given
// `web_contents`, if it doesn't already exist.
static void CreateForWebContents(content::WebContents* web_contents);

private:
friend class content::WebContentsUserData<
DesktopAndroidExtensionWebContentsObserver>;

explicit DesktopAndroidExtensionWebContentsObserver(
content::WebContents* web_contents);

WEB_CONTENTS_USER_DATA_KEY_DECL();
};

} // namespace extensions

#endif // CHROME_BROWSER_EXTENSIONS_DESKTOP_ANDROID_DESKTOP_ANDROID_EXTENSION_WEB_CONTENTS_OBSERVER_H_
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include "chrome/browser/browser_process.h"
#include "chrome/browser/extensions/desktop_android/desktop_android_extension_system.h"
#include "chrome/browser/extensions/desktop_android/desktop_android_extension_web_contents_observer.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "components/version_info/version_info.h"
Expand All @@ -17,10 +18,12 @@
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/common/user_agent.h"
#include "extensions/browser/api/core_extensions_browser_api_provider.h"
#include "extensions/browser/api/extensions_api_client.h"
#include "extensions/browser/event_router.h"
#include "extensions/browser/extension_web_contents_observer.h"
#include "extensions/browser/extensions_browser_interface_binders.h"
#include "extensions/browser/kiosk/kiosk_delegate.h"
#include "extensions/browser/null_app_sorting.h"
#include "extensions/browser/updater/null_extension_cache.h"
#include "extensions/browser/url_request_util.h"
Expand All @@ -32,8 +35,26 @@ using content::BrowserThread;

namespace extensions {

namespace {

class DesktopAndroidKioskDelegate : public KioskDelegate {
public:
DesktopAndroidKioskDelegate() = default;
~DesktopAndroidKioskDelegate() override = default;

bool IsAutoLaunchedKioskApp(const ExtensionId& id) const override {
// Desktop-android does not support kiosk apps.
return false;
}
};

} // namespace

DesktopAndroidExtensionsBrowserClient::DesktopAndroidExtensionsBrowserClient()
: extension_cache_(std::make_unique<NullExtensionCache>()) {}
: extension_cache_(std::make_unique<NullExtensionCache>()),
kiosk_delegate_(std::make_unique<DesktopAndroidKioskDelegate>()) {
AddAPIProvider(std::make_unique<CoreExtensionsBrowserAPIProvider>());
}

DesktopAndroidExtensionsBrowserClient::
~DesktopAndroidExtensionsBrowserClient() {}
Expand Down Expand Up @@ -263,16 +284,20 @@ bool DesktopAndroidExtensionsBrowserClient::IsMinBrowserVersionSupported(
}

void DesktopAndroidExtensionsBrowserClient::CreateExtensionWebContentsObserver(
content::WebContents* web_contents) {}
content::WebContents* web_contents) {
DesktopAndroidExtensionWebContentsObserver::CreateForWebContents(
web_contents);
}

ExtensionWebContentsObserver*
DesktopAndroidExtensionsBrowserClient::GetExtensionWebContentsObserver(
content::WebContents* web_contents) {
return ExtensionWebContentsObserver::GetForWebContents(web_contents);
return DesktopAndroidExtensionWebContentsObserver::FromWebContents(
web_contents);
}

KioskDelegate* DesktopAndroidExtensionsBrowserClient::GetKioskDelegate() {
return nullptr;
return kiosk_delegate_.get();
}

bool DesktopAndroidExtensionsBrowserClient::IsLockScreenContext(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ class DesktopAndroidExtensionsBrowserClient : public ExtensionsBrowserClient {

private:
std::unique_ptr<ExtensionCache> extension_cache_;
std::unique_ptr<KioskDelegate> kiosk_delegate_;
};

} // namespace extensions
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,10 @@ IN_PROC_BROWSER_TEST_F(DesktopAndroidExtensionsBrowserTest,
}

// Tests the adding an extension to the registry and navigating to a
// corresponding page in the extension, verifying the expected content.
// corresponding page in the extension, verifying the expected content, and
// leveraging the chrome.test API to pass a result. The latter verifies the
// core extension bindings system and API handling works, including
// exercising custom bindings.
IN_PROC_BROWSER_TEST_F(DesktopAndroidExtensionsBrowserTest,
NavigateToExtensionPage) {
static constexpr char kManifest[] =
Expand All @@ -106,11 +109,20 @@ IN_PROC_BROWSER_TEST_F(DesktopAndroidExtensionsBrowserTest,
static constexpr char kPageHtml[] =
R"(<html>
Hello, world
<script src="page.js"></script>
</html>)";
static constexpr char kPageJs[] =
R"(chrome.test.runTests([
function sanityCheck() {
chrome.test.assertEq(2, 1 + 1);
chrome.test.succeed();
}
]);)";

TestExtensionDir test_dir;
test_dir.WriteManifest(kManifest);
test_dir.WriteFile(FILE_PATH_LITERAL("page.html"), kPageHtml);
test_dir.WriteFile(FILE_PATH_LITERAL("page.js"), kPageJs);

scoped_refptr<const Extension> extension =
LoadExtensionFromDirectory(test_dir.UnpackedPath());
Expand All @@ -126,10 +138,12 @@ IN_PROC_BROWSER_TEST_F(DesktopAndroidExtensionsBrowserTest,

GURL extension_page = extension->GetResourceURL("page.html");

ResultCatcher result_catcher;
EXPECT_TRUE(content::NavigateToURL(GetActiveWebContents(), extension_page));
EXPECT_EQ(extension_page, GetActiveWebContents()->GetLastCommittedURL());
EXPECT_EQ("Hello, world",
content::EvalJs(GetActiveWebContents(), "document.body.innerText"));
EXPECT_TRUE(result_catcher.GetNextResult()) << result_catcher.message();
}

} // namespace extensions
Loading

0 comments on commit e2df8cf

Please sign in to comment.