Skip to content

Commit

Permalink
Link Capturing: Enable app-to-app capturing for target="_self" on CrOS
Browse files Browse the repository at this point in the history
When App-to-App link capturing flags are enabled on ChromeOS, this
allows link clicks within an app window to link capture, even without
the link capturing setting being enabled.

This CL only works for target="_self" links, which are the simplest case
to handle. Future CLs will apply the same business logic to
target="_blank" links.

Bug: 1480903, b/303350482
Change-Id: I85162d849c9b0c634cdfeec93e61634f92e64347
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4963142
Auto-Submit: Tim Sergeant <tsergeant@chromium.org>
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Commit-Queue: Tim Sergeant <tsergeant@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1216799}
  • Loading branch information
tgsergeant authored and Chromium LUCI CQ committed Oct 30, 2023
1 parent 912dd8d commit b5dcf4a
Show file tree
Hide file tree
Showing 9 changed files with 177 additions and 29 deletions.
17 changes: 17 additions & 0 deletions chrome/browser/apps/link_capturing/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -94,3 +94,20 @@ source_set("unit_tests") {
]
}
}

# Browser tests which require an Ash process with App Service to be running.
if (is_chromeos) {
source_set("app_service_browser_tests") {
testonly = true

defines = [ "HAS_OUT_OF_PROC_TEST_RUNNER" ]

sources = [ "chromeos_link_capturing_delegate_browsertest.cc" ]

deps = [
":link_capturing",
"//chrome/browser/ui",
"//chrome/test:test_support_ui",
]
}
}
100 changes: 76 additions & 24 deletions chrome/browser/apps/link_capturing/chromeos_link_capturing_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,12 @@

#include "chrome/browser/apps/link_capturing/chromeos_link_capturing_delegate.h"

#include <string_view>

#include "ash/webui/projector_app/public/cpp/projector_app_constants.h"
#include "base/auto_reset.h"
#include "base/containers/fixed_flat_set.h"
#include "base/feature_list.h"
#include "base/functional/bind.h"
#include "base/functional/callback_helpers.h"
#include "base/memory/values_equivalent.h"
Expand All @@ -14,10 +18,15 @@
#include "chrome/browser/apps/app_service/app_service_proxy.h"
#include "chrome/browser/apps/app_service/app_service_proxy_factory.h"
#include "chrome/browser/apps/app_service/launch_utils.h"
#include "chrome/browser/apps/link_capturing/link_capturing_features.h"
#include "chrome/browser/apps/link_capturing/metrics/intent_handling_metrics.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/web_applications/web_app_id_constants.h"
#include "chrome/browser/web_applications/web_app_provider.h"
#include "chrome/browser/web_applications/web_app_tab_helper.h"
#include "chrome/browser/web_applications/web_app_ui_manager.h"
#include "chrome/browser/web_applications/web_app_utils.h"
#include "components/webapps/common/web_app_id.h"
#include "content/public/browser/navigation_handle.h"
#include "content/public/browser/web_contents.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
Expand All @@ -27,12 +36,7 @@ namespace {
// Usually we want to only capture navigations from clicking a link. For a
// subset of apps, we want to capture typing into the omnibox as well.
bool ShouldOnlyCaptureLinks(const std::vector<std::string>& app_ids) {
for (const auto& app_id : app_ids) {
if (app_id == ash::kChromeUIUntrustedProjectorSwaAppId) {
return false;
}
}
return true;
return !base::Contains(app_ids, ash::kChromeUIUntrustedProjectorSwaAppId);
}

bool IsSystemWebApp(Profile* profile, const std::string& app_id) {
Expand Down Expand Up @@ -110,6 +114,62 @@ IntentHandlingMetrics::Platform GetMetricsPlatform(AppType app_type) {
}
}

// Returns the ID of the app window where the link click originated. Returns
// nullopt if the link was not clicked in an app window.
absl::optional<webapps::AppId> GetSourceAppId(
Profile* profile,
content::WebContents* web_contents) {
const webapps::AppId* app_id = web_app::WebAppProvider::GetForWebApps(profile)
->ui_manager()
.GetAppIdForWindow(web_contents);
return app_id ? absl::optional<webapps::AppId>(*app_id) : absl::nullopt;
}

// Returns the App ID that should be launched for this link click, if any.
absl::optional<std::string> GetLaunchAppId(
const AppIdsToLaunchForUrl& app_ids_to_launch,
bool is_navigation_from_link,
absl::optional<webapps::AppId> source_app_id) {
if (app_ids_to_launch.candidates.empty()) {
return absl::nullopt;
}

if (ShouldOnlyCaptureLinks(app_ids_to_launch.candidates) &&
!is_navigation_from_link) {
return absl::nullopt;
}

if (app_ids_to_launch.preferred) {
return app_ids_to_launch.preferred;
}

// If there is one candidate app that's not preferred, but the link was
// clicked from within an app window, we may still launch the app.
if (app_ids_to_launch.candidates.size() == 1 && source_app_id.has_value()) {
// When AppToAppLinkCapturing is enabled, always capture links from within
// app windows.
if (base::FeatureList::IsEnabled(apps::features::kAppToAppLinkCapturing)) {
return app_ids_to_launch.candidates[0];
}

// When AppToAppLinkCapturingWorkspaceApps is enabled, launch the app if
// both source and destination are Workspace apps.
if (base::FeatureList::IsEnabled(
apps::features::kAppToAppLinkCapturingWorkspaceApps)) {
constexpr auto kWorkspaceApps = base::MakeFixedFlatSet<std::string_view>(
{web_app::kGoogleDriveAppId, web_app::kGoogleDocsAppId,
web_app::kGoogleSheetsAppId, web_app::kGoogleSlidesAppId});

if (kWorkspaceApps.contains(source_app_id.value()) &&
kWorkspaceApps.contains(app_ids_to_launch.candidates[0])) {
return app_ids_to_launch.candidates[0];
}
}
}

return absl::nullopt;
}

void LaunchApp(base::WeakPtr<AppServiceProxy> proxy,
const std::string& app_id,
int32_t event_flags,
Expand Down Expand Up @@ -140,7 +200,6 @@ static const base::TickClock*& GetTickClock() {

} // namespace


// static
base::AutoReset<const base::TickClock*>
ChromeOsLinkCapturingDelegate::SetClockForTesting(
Expand All @@ -167,45 +226,38 @@ ChromeOsLinkCapturingDelegate::CreateLinkCaptureLaunchClosure(
bool is_navigation_from_link) {
AppServiceProxy* proxy = apps::AppServiceProxyFactory::GetForProfile(profile);

AppIdsToLaunchForUrl app_id_to_launch = FindAppIdsToLaunchForUrl(proxy, url);

if (app_id_to_launch.candidates.empty()) {
return absl::nullopt;
}

if (ShouldOnlyCaptureLinks(app_id_to_launch.candidates) &&
!is_navigation_from_link) {
return absl::nullopt;
}
AppIdsToLaunchForUrl app_ids_to_launch = FindAppIdsToLaunchForUrl(proxy, url);

if (!app_id_to_launch.preferred) {
absl::optional<std::string> launch_app_id =
GetLaunchAppId(app_ids_to_launch, is_navigation_from_link,
GetSourceAppId(profile, web_contents));
if (!launch_app_id) {
return absl::nullopt;
}

const std::string& preferred_app_id = *app_id_to_launch.preferred;
// Only automatically launch supported app types.
AppType app_type = proxy->AppRegistryCache().GetAppType(preferred_app_id);
AppType app_type = proxy->AppRegistryCache().GetAppType(*launch_app_id);
if (app_type != AppType::kArc && app_type != AppType::kWeb &&
!IsSystemWebApp(profile, preferred_app_id)) {
!IsSystemWebApp(profile, *launch_app_id)) {
return absl::nullopt;
}

// Don't capture if already inside the target app scope.
if (app_type == AppType::kWeb &&
base::ValuesEquivalent(web_app::WebAppTabHelper::GetAppId(web_contents),
&preferred_app_id)) {
&launch_app_id.value())) {
return absl::nullopt;
}

auto launch_source = is_navigation_from_link ? LaunchSource::kFromLink
: LaunchSource::kFromOmnibox;
GURL redirected_url =
RedirectUrlIfSwa(profile, preferred_app_id, url, GetTickClock());
RedirectUrlIfSwa(profile, *launch_app_id, url, GetTickClock());

// Note: The launch can occur after this object is destroyed, so bind to a
// static function.
return base::BindOnce(
&LaunchApp, proxy->GetWeakPtr(), preferred_app_id,
&LaunchApp, proxy->GetWeakPtr(), *launch_app_id,
GetEventFlags(WindowOpenDisposition::NEW_WINDOW,
/*prefer_container=*/true),
redirected_url, launch_source,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
// 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 "base/test/scoped_feature_list.h"
#include "chrome/browser/apps/link_capturing/link_capturing_features.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/web_applications/app_browser_controller.h"
#include "chrome/browser/ui/web_applications/test/web_app_navigation_browsertest.h"
#include "chrome/test/base/ui_test_utils.h"
#include "content/public/test/browser_test.h"

namespace apps {
namespace {

using ui_test_utils::BrowserChangeObserver;

// Test for ChromeOS-specific link capturing features. Features which are shared
// between ChromeOS and Windows/Mac/Linux are tested in
// WebAppLinkCapturingBrowserTest.
class ChromeOsLinkCapturingDelegateBrowserTest
: public web_app::WebAppNavigationBrowserTest {
public:
Browser* NavigateExpectingNewBrowser(Browser* source,
const GURL& url,
LinkTarget target) {
BrowserChangeObserver observer(nullptr,
BrowserChangeObserver::ChangeType::kAdded);
ClickLinkAndWait(source->tab_strip_model()->GetActiveWebContents(), url,
target, /*rel=*/"");
return observer.Wait();
}

private:
base::test::ScopedFeatureList features_{
apps::features::kAppToAppLinkCapturing};
};

// Verifies that target="_self" links clicked in a web app window are link
// captured, when AppToAppLinkCapturing is enabled.
IN_PROC_BROWSER_TEST_F(ChromeOsLinkCapturingDelegateBrowserTest,
AppToAppLinkCaptureTargetSelf) {
InstallTestWebApp();
webapps::AppId target_app = InstallTestWebApp("www.foo.com", "/");

Browser* app_browser = OpenTestWebApp();
Browser* result_browser = NavigateExpectingNewBrowser(
app_browser, https_server().GetURL("www.foo.com", "/launch"),
LinkTarget::SELF);

EXPECT_TRUE(
web_app::AppBrowserController::IsForWebApp(result_browser, target_app));
}

} // namespace
} // namespace apps
13 changes: 11 additions & 2 deletions chrome/browser/ui/web_applications/web_app_ui_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
#include "components/webapps/browser/uninstall_result_code.h"
#include "content/public/browser/clear_site_data_utils.h"
#include "content/public/browser/navigation_handle.h"
#include "content/public/browser/web_contents.h"
#include "extensions/browser/app_sorting.h"
#include "extensions/browser/extension_system.h"
#include "third_party/blink/public/mojom/manifest/manifest.mojom-shared.h"
Expand Down Expand Up @@ -304,6 +305,15 @@ bool WebAppUiManagerImpl::IsInAppWindow(
return AppBrowserController::IsWebApp(browser);
}

const webapps::AppId* WebAppUiManagerImpl::GetAppIdForWindow(
content::WebContents* web_contents) const {
Browser* browser = chrome::FindBrowserWithTab(web_contents);
if (AppBrowserController::IsWebApp(browser)) {
return &browser->app_controller()->app_id();
}
return nullptr;
}

void WebAppUiManagerImpl::NotifyOnAssociatedAppChanged(
content::WebContents* web_contents,
const absl::optional<webapps::AppId>& previous_app_id,
Expand Down Expand Up @@ -448,8 +458,7 @@ content::WebContents* WebAppUiManagerImpl::CreateNewTab() {
bool WebAppUiManagerImpl::IsWebContentsActiveTabInBrowser(
content::WebContents* web_contents) {
Browser* browser = chrome::FindBrowserWithTab(web_contents);
return browser &&
browser->tab_strip_model() &&
return browser && browser->tab_strip_model() &&
browser->tab_strip_model()->GetActiveWebContents() == web_contents;
}

Expand Down
4 changes: 3 additions & 1 deletion chrome/browser/ui/web_applications/web_app_ui_manager_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ class WebAppUiManagerImpl : public BrowserListObserver, public WebAppUiManager {
void AddAppToQuickLaunchBar(const webapps::AppId& app_id) override;
bool IsAppInQuickLaunchBar(const webapps::AppId& app_id) const override;
bool IsInAppWindow(content::WebContents* web_contents) const override;
const webapps::AppId* GetAppIdForWindow(
content::WebContents* web_contents) const override;
void NotifyOnAssociatedAppChanged(
content::WebContents* web_contents,
const absl::optional<webapps::AppId>& previous_app_id,
Expand Down Expand Up @@ -114,7 +116,7 @@ class WebAppUiManagerImpl : public BrowserListObserver, public WebAppUiManager {
#endif
content::WebContents* CreateNewTab() override;
bool IsWebContentsActiveTabInBrowser(
content::WebContents* web_contents) override;
content::WebContents* web_contents) override;
void TriggerInstallDialog(content::WebContents* web_contents) override;

void PresentUserUninstallDialog(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,11 @@ bool FakeWebAppUiManager::IsInAppWindow(
return false;
}

const webapps::AppId* FakeWebAppUiManager::GetAppIdForWindow(
content::WebContents* web_contents) const {
return nullptr;
}

bool FakeWebAppUiManager::CanReparentAppTabToWindow(
const webapps::AppId& app_id,
bool shortcut_created) const {
Expand Down Expand Up @@ -172,7 +177,7 @@ content::WebContents* FakeWebAppUiManager::CreateNewTab() {
}

bool FakeWebAppUiManager::IsWebContentsActiveTabInBrowser(
content::WebContents* web_contents) {
content::WebContents* web_contents) {
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ class FakeWebAppUiManager : public WebAppUiManager {
void AddAppToQuickLaunchBar(const webapps::AppId& app_id) override;
bool IsAppInQuickLaunchBar(const webapps::AppId& app_id) const override;
bool IsInAppWindow(content::WebContents* web_contents) const override;
const webapps::AppId* GetAppIdForWindow(
content::WebContents* web_contents) const override;
void NotifyOnAssociatedAppChanged(
content::WebContents* web_contents,
const absl::optional<webapps::AppId>& previous_app_id,
Expand Down Expand Up @@ -92,7 +94,7 @@ class FakeWebAppUiManager : public WebAppUiManager {
#endif
content::WebContents* CreateNewTab() override;
bool IsWebContentsActiveTabInBrowser(
content::WebContents* web_contents) override;
content::WebContents* web_contents) override;
void TriggerInstallDialog(content::WebContents* web_contents) override;

void PresentUserUninstallDialog(
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/web_applications/web_app_ui_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,8 @@ class WebAppUiManager {
// Returns whether |web_contents| is in a web app window or popup window
// created from a web app window.
virtual bool IsInAppWindow(content::WebContents* web_contents) const = 0;
virtual const webapps::AppId* GetAppIdForWindow(
content::WebContents* web_contents) const = 0;
virtual void NotifyOnAssociatedAppChanged(
content::WebContents* web_contents,
const absl::optional<webapps::AppId>& previous_app_id,
Expand Down
2 changes: 2 additions & 0 deletions chrome/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -4855,6 +4855,7 @@ if (!is_android) {
"//chrome/browser/apps/app_discovery_service/recommended_arc_apps:test_support",
"//chrome/browser/apps/app_preload_service:browser_tests",
"//chrome/browser/apps/link_capturing",
"//chrome/browser/apps/link_capturing:app_service_browser_tests",
"//chrome/browser/ash",
"//chrome/browser/ash:add_remove_user_event_proto",
"//chrome/browser/ash:arc_test_support",
Expand Down Expand Up @@ -5528,6 +5529,7 @@ if (is_chromeos_lacros) {
"//chrome/browser/apps/app_service",
"//chrome/browser/apps/app_service:app_registry_cache_waiter",
"//chrome/browser/apps/link_capturing",
"//chrome/browser/apps/link_capturing:app_service_browser_tests",
"//chrome/browser/chromeos",
"//chrome/browser/chromeos:test_support",
"//chrome/browser/chromeos/extensions/telemetry/api:browser_tests",
Expand Down

0 comments on commit b5dcf4a

Please sign in to comment.