Skip to content

Commit

Permalink
Implement launch handling route_to behaviour
Browse files Browse the repository at this point in the history
This CL implements the app launch behaviour of launch_handler.route_to
values "auto", "new-client" and "existing-client".
Explainer: https://github.com/WICG/sw-launch/blob/main/launch_handler.md#launch_handler-manifest-member

The old code used NEW_FOREGROUND_TAB like a sentinel value to
identify whether the launch came from
AppsNavigationThrottle::CaptureWebAppScopeNavigations() capturing
links for tabbed web app windows. It worked because it was the only
callsite that did this window capturing. Now with route_to there any
disposition can capture so this code has been updated to check
IsTabbedWindowModeEnabled() directly instead of testing for
NEW_FOREGROUND_TAB.

Bug: 1250222
Change-Id: I9001144d2ea1f68e3023e390eab286b66e426726
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3162688
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Glen Robertson <glenrob@chromium.org>
Cr-Commit-Position: refs/heads/main@{#931888}
  • Loading branch information
alancutter authored and Chromium LUCI CQ committed Oct 15, 2021
1 parent ca6e209 commit b36e0f7
Show file tree
Hide file tree
Showing 7 changed files with 172 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -196,23 +196,17 @@ AppId InstallWebAppFromManifest(Browser* browser, const GURL& app_url) {
}

Browser* LaunchWebAppBrowser(Profile* profile, const AppId& app_id) {
BrowserChangeObserver observer(nullptr,
BrowserChangeObserver::ChangeType::kAdded);
EXPECT_TRUE(
content::WebContents* web_contents =
apps::AppServiceProxyFactory::GetForProfile(profile)
->BrowserAppLauncher()
->LaunchAppWithParams(apps::AppLaunchParams(
app_id, apps::mojom::LaunchContainer::kLaunchContainerWindow,
WindowOpenDisposition::CURRENT_TAB,
apps::mojom::AppLaunchSource::kSourceTest)));

Browser* browser = observer.Wait();
EXPECT_EQ(browser, chrome::FindLastActive());
bool is_correct_app_browser =
browser && GetAppIdFromApplicationName(browser->app_name()) == app_id;
EXPECT_TRUE(is_correct_app_browser);

return is_correct_app_browser ? browser : nullptr;
apps::mojom::AppLaunchSource::kSourceTest));
EXPECT_TRUE(web_contents);
Browser* browser = chrome::FindBrowserWithWebContents(web_contents);
EXPECT_TRUE(AppBrowserController::IsForWebApp(browser, app_id));
return browser;
}

// Launches the app, waits for the app url to load.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
// Copyright 2021 The Chromium Authors. All rights reserved.
// 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/ui/browser.h"
#include "chrome/browser/ui/browser_navigator.h"
#include "chrome/browser/ui/browser_navigator_params.h"
#include "chrome/browser/ui/web_applications/test/web_app_browsertest_util.h"
#include "chrome/browser/web_applications/os_integration_manager.h"
#include "chrome/browser/web_applications/test/web_app_install_test_utils.h"
#include "chrome/browser/web_applications/web_app.h"
#include "chrome/browser/web_applications/web_app_provider.h"
#include "chrome/browser/web_applications/web_app_registrar.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "content/public/test/browser_test.h"
#include "third_party/blink/public/common/features.h"
#include "ui/base/page_transition_types.h"

namespace web_app {

using RouteTo = LaunchHandler::RouteTo;
using NavigateExistingClient = LaunchHandler::NavigateExistingClient;

class WebAppLaunchHanderBrowserTest : public InProcessBrowserTest {
public:
WebAppLaunchHanderBrowserTest() = default;
~WebAppLaunchHanderBrowserTest() override = default;

// InProcessBrowserTest:
void SetUpOnMainThread() override {
InProcessBrowserTest::SetUpOnMainThread();
ASSERT_TRUE(embedded_test_server()->Start());
web_app::test::WaitUntilReady(
web_app::WebAppProvider::GetForTest(profile()));
}

protected:
Profile* profile() { return browser()->profile(); }

absl::optional<LaunchHandler> GetLaunchHandler(const AppId& app_id) {
return WebAppProvider::GetForTest(profile())
->registrar()
.GetAppById(app_id)
->launch_handler();
}

private:
base::test::ScopedFeatureList enable_launch_handler{
blink::features::kWebAppEnableLaunchHandler};
ScopedOsHooksSuppress os_hooks_suppress_{
OsIntegrationManager::ScopedSuppressOsHooksForTesting()};
};

IN_PROC_BROWSER_TEST_F(WebAppLaunchHanderBrowserTest, RouteToEmpty) {
AppId app_id = InstallWebAppFromPage(
browser(), embedded_test_server()->GetURL("/web_apps/basic.html"));
EXPECT_EQ(GetLaunchHandler(app_id), absl::nullopt);

Browser* browser_1 = LaunchWebAppBrowser(profile(), app_id);
Browser* browser_2 = LaunchWebAppBrowser(profile(), app_id);
EXPECT_NE(browser_1, browser_2);
}

IN_PROC_BROWSER_TEST_F(WebAppLaunchHanderBrowserTest, RouteToAuto) {
AppId app_id = InstallWebAppFromPage(
browser(), embedded_test_server()->GetURL(
"/web_apps/get_manifest.html?route_to_auto.json"));
EXPECT_EQ(GetLaunchHandler(app_id),
(LaunchHandler{RouteTo::kAuto, NavigateExistingClient::kAlways}));

Browser* browser_1 = LaunchWebAppBrowser(profile(), app_id);
Browser* browser_2 = LaunchWebAppBrowser(profile(), app_id);
EXPECT_NE(browser_1, browser_2);
}

IN_PROC_BROWSER_TEST_F(WebAppLaunchHanderBrowserTest, RouteToNewClient) {
AppId app_id = InstallWebAppFromPage(
browser(), embedded_test_server()->GetURL(
"/web_apps/get_manifest.html?route_to_new_client.json"));
EXPECT_EQ(
GetLaunchHandler(app_id),
(LaunchHandler{RouteTo::kNewClient, NavigateExistingClient::kAlways}));

Browser* browser_1 = LaunchWebAppBrowser(profile(), app_id);
Browser* browser_2 = LaunchWebAppBrowser(profile(), app_id);
EXPECT_NE(browser_1, browser_2);
}

IN_PROC_BROWSER_TEST_F(WebAppLaunchHanderBrowserTest, RouteToExistingClient) {
AppId app_id = InstallWebAppFromPage(
browser(),
embedded_test_server()->GetURL(
"/web_apps/"
"get_manifest.html?route_to_existing_client_navigate_empty.json"));
EXPECT_EQ(GetLaunchHandler(app_id),
(LaunchHandler{RouteTo::kExistingClient,
NavigateExistingClient::kAlways}));

Browser* browser_1 = LaunchWebAppBrowser(profile(), app_id);
content::WebContents* web_contents =
browser_1->tab_strip_model()->GetActiveWebContents();
GURL start_url = embedded_test_server()->GetURL("/web_apps/");
EXPECT_EQ(web_contents->GetVisibleURL(), start_url);

// Navigate window away from start_url to check that the next launch navs to
// start_url again.
NavigateParams navigate_params(browser_1, GURL("about:blank"),
ui::PAGE_TRANSITION_LINK);
ASSERT_TRUE(Navigate(&navigate_params));
EXPECT_EQ(web_contents->GetVisibleURL(), GURL("about:blank"));

Browser* browser_2 = LaunchWebAppBrowser(profile(), app_id);
EXPECT_EQ(browser_1, browser_2);
EXPECT_EQ(web_contents->GetVisibleURL(), start_url);
}

} // namespace web_app
34 changes: 23 additions & 11 deletions chrome/browser/ui/web_applications/web_app_launch_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,8 @@ class LaunchProcess {
WindowOpenDisposition GetNavigationDisposition(bool is_new_browser) const;
content::WebContents* MaybeLaunchSystemWebApp(const GURL& launch_url);
std::tuple<Browser*, bool /*is_new_browser*/> EnsureBrowser();
LaunchHandler::RouteTo GetLaunchRouteTo() const;

Browser* MaybeFindBrowserForLaunch() const;
Browser* CreateBrowserForLaunch();
content::WebContents* NavigateBrowser(Browser* browser,
Expand All @@ -195,13 +197,15 @@ class LaunchProcess {
Profile& profile_;
WebAppProvider& provider_;
const apps::AppLaunchParams& params_;
const WebApp* web_app_ = nullptr;
};

LaunchProcess::LaunchProcess(Profile& profile,
const apps::AppLaunchParams& params)
: profile_(profile),
provider_(*WebAppProvider::GetForLocalAppsUnchecked(&profile)),
params_(params) {}
params_(params),
web_app_(provider_.registrar().GetAppById(params.app_id)) {}

content::WebContents* LaunchProcess::Run() {
if (Browser::GetCreationStatusForProfile(&profile_) !=
Expand Down Expand Up @@ -243,24 +247,24 @@ content::WebContents* LaunchProcess::Run() {
}

const apps::ShareTarget* LaunchProcess::MaybeGetShareTarget() const {
DCHECK(web_app_);
bool is_share_intent =
params_.intent &&
(params_.intent->action == apps_util::kIntentActionSend ||
params_.intent->action == apps_util::kIntentActionSendMultiple);
return is_share_intent
? provider_.registrar().GetAppShareTarget(params_.app_id)
return is_share_intent && web_app_->share_target().has_value()
? &web_app_->share_target().value()
: nullptr;
}

std::tuple<GURL, bool /*is_file_handling*/> LaunchProcess::GetLaunchUrl(
const apps::ShareTarget* share_target) const {
DCHECK(web_app_);
GURL launch_url;
bool is_file_handling = false;
bool is_note_taking_intent =
params_.intent &&
params_.intent->action == apps_util::kIntentActionCreateNote;
const WebApp* web_app = provider_.registrar().GetAppById(params_.app_id);
DCHECK(web_app);

if (!params_.override_url.is_empty()) {
launch_url = params_.override_url;
Expand All @@ -282,9 +286,9 @@ std::tuple<GURL, bool /*is_file_handling*/> LaunchProcess::GetLaunchUrl(
// Handle share_target launch.
launch_url = share_target->action;
} else if (is_note_taking_intent &&
web_app->note_taking_new_note_url().is_valid()) {
web_app_->note_taking_new_note_url().is_valid()) {
// Handle Create Note launch.
launch_url = web_app->note_taking_new_note_url();
launch_url = web_app_->note_taking_new_note_url();
} else {
// This is a default launch.
launch_url = provider_.registrar().GetAppLaunchUrl(params_.app_id);
Expand Down Expand Up @@ -316,6 +320,15 @@ WindowOpenDisposition LaunchProcess::GetNavigationDisposition(
: WindowOpenDisposition::NEW_FOREGROUND_TAB;
}

LaunchHandler::RouteTo LaunchProcess::GetLaunchRouteTo() const {
DCHECK(web_app_);
LaunchHandler launch_handler =
web_app_->launch_handler().value_or(LaunchHandler());
if (launch_handler.route_to == LaunchHandler::RouteTo::kAuto)
return LaunchHandler::RouteTo::kNewClient;
return launch_handler.route_to;
}

content::WebContents* LaunchProcess::MaybeLaunchSystemWebApp(
const GURL& launch_url) {
absl::optional<SystemAppType> system_app_type =
Expand Down Expand Up @@ -348,11 +361,10 @@ Browser* LaunchProcess::MaybeFindBrowserForLaunch() const {
display::Screen::GetScreen()->GetDisplayForNewWindows().id());
}

if (params_.disposition != WindowOpenDisposition::NEW_FOREGROUND_TAB)
return nullptr;

if (!provider_.registrar().IsTabbedWindowModeEnabled(params_.app_id))
if (!provider_.registrar().IsTabbedWindowModeEnabled(params_.app_id) &&
GetLaunchRouteTo() == LaunchHandler::RouteTo::kNewClient) {
return nullptr;
}

for (Browser* browser : *BrowserList::GetInstance()) {
if (browser->profile() == &profile_ &&
Expand Down
1 change: 1 addition & 0 deletions chrome/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1990,6 +1990,7 @@ if (!is_android && !is_fuchsia) {
"../browser/ui/web_applications/web_app_browsertest.cc",
"../browser/ui/web_applications/web_app_engagement_browsertest.cc",
"../browser/ui/web_applications/web_app_file_handling_browsertest.cc",
"../browser/ui/web_applications/web_app_launch_handling_browsertest.cc",
"../browser/ui/web_applications/web_app_link_capturing_browsertest.cc",
"../browser/ui/web_applications/web_app_metrics_browsertest.cc",
"../browser/ui/web_applications/web_app_navigate_browsertest.cc",
Expand Down
8 changes: 8 additions & 0 deletions chrome/test/data/web_apps/route_to_auto.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "route_to: auto",
"start_url": ".",
"display": "standalone",
"launch_handler": {
"route_to": "auto"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "route_to: existing-client",
"start_url": ".",
"display": "standalone",
"launch_handler": {
"route_to": "existing-client"
}
}
8 changes: 8 additions & 0 deletions chrome/test/data/web_apps/route_to_new_client.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "route_to: new-client",
"start_url": ".",
"display": "standalone",
"launch_handler": {
"route_to": "new-client"
}
}

0 comments on commit b36e0f7

Please sign in to comment.