Skip to content

Commit

Permalink
Replace launch_handler: {route_to: existing-client} with existing-cli…
Browse files Browse the repository at this point in the history 
…ent-navigate and existing-client-retain

This CL implements a spec/explainer change: WICG/web-app-launch#53

This adds support for "existing-client-navigate" and
"existing-client-retain" values for the web app
manifest.launch_handler.route_to field.

This retains backwards compatibility with the old syntax, expected to be
removed before shipping the API.

Old syntax: {
  "launch_handler": {
    "route_to": "existing-client",
    "navigate_existing_client": "always"
  }
}
New syntax: {
  "launch_handler": {
    "route_to": "existing-client-navigate"
  }
}

Old syntax: {
  "launch_handler": {
    "route_to": "existing-client",
    "navigate_existing_client": "never"
  }
}
New syntax: {
  "launch_handler": {
    "route_to": "existing-client-retain"
  }
}

Sites that wish to use the new syntax but remain backwards compatible
with the old syntax can use:
{
  "launch_handler": {
    "route_to": ["existing-client-retain", "existing-client"],
    "navigate_existing_client": "never"
  }
}

Bug: 1305904
Change-Id: I5bdc2ae36b0bbaf341e41aae23b218a0e3f38735
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3521294
Reviewed-by: Glen Robertson <glenrob@chromium.org>
Reviewed-by: Nate Chapin <japhet@chromium.org>
Reviewed-by: Eric Willigers <ericwilligers@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/main@{#990282}
  • Loading branch information
@alancutter
alancutter authored and Chromium LUCI CQ committed Apr 8, 2022
1 parent e5f531b commit 87b1485
Show file tree
Hide file tree
Showing 28 changed files with 470 additions and 293 deletions.
236 changes: 137 additions & 99 deletions chrome/browser/ui/web_applications/web_app_launch_handler_browsertest.cc
View file
Expand Up @@ -29,7 +29,6 @@
namespace web_app {

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

class WebAppLaunchHandlerBrowserTest : public InProcessBrowserTest {
public:
Expand Down Expand Up @@ -109,6 +108,95 @@ class WebAppLaunchHandlerBrowserTest : public InProcessBrowserTest {
.ExtractString();
}

void ExpectExistingClientNavigateBehaviour(const AppId& app_id,
const GURL& start_url) {
EXPECT_EQ(GetLaunchHandler(app_id),
(LaunchHandler{RouteTo::kExistingClientNavigate}));

Browser* browser_1 = LaunchWebAppBrowserAndWait(profile(), app_id);
content::WebContents* web_contents =
browser_1->tab_strip_model()->GetActiveWebContents();
EXPECT_EQ(web_contents->GetLastCommittedURL(), start_url);
EXPECT_EQ(AwaitNextLaunchParamsTargetUrl(browser_1), start_url.spec());

// Navigate window away from start_url to check that the next launch navs to
// start_url again.
GURL alt_url = embedded_test_server()->GetURL("/web_apps/basic.html");
NavigateToURLAndWait(browser_1, alt_url);
EXPECT_EQ(web_contents->GetLastCommittedURL(), alt_url);

Browser* browser_2 = LaunchWebAppBrowserAndWait(profile(), app_id);
EXPECT_EQ(browser_1, browser_2);
EXPECT_EQ(web_contents->GetLastCommittedURL(), start_url);
EXPECT_EQ(AwaitNextLaunchParamsTargetUrl(browser_2), start_url.spec());
}

void ExpectExistingClientRetainBehaviour(const AppId& app_id,
const GURL& start_url) {
EXPECT_EQ(GetLaunchHandler(app_id),
(LaunchHandler{RouteTo::kExistingClientRetain}));

Browser* browser_1 = LaunchWebAppBrowserAndWait(profile(), app_id);
content::WebContents* web_contents =
browser_1->tab_strip_model()->GetActiveWebContents();
EXPECT_EQ(web_contents->GetLastCommittedURL(), start_url);
EXPECT_EQ(AwaitNextLaunchParamsTargetUrl(browser_1), start_url.spec());

// Navigate window away from start_url to an in scope URL, check that the
// next launch doesn't navigate to start_url.
{
GURL in_scope_url =
embedded_test_server()->GetURL("/web_apps/basic.html");
NavigateToURLAndWait(browser_1, in_scope_url);
EXPECT_EQ(web_contents->GetLastCommittedURL(), in_scope_url);

ASSERT_TRUE(SetUpNextLaunchParamsTargetUrlPromise(browser_1));
Browser* browser_2 = LaunchWebAppBrowser(profile(), app_id);
EXPECT_EQ(browser_1, browser_2);
EXPECT_EQ(AwaitNextLaunchParamsTargetUrlPromise(browser_2),
start_url.spec());
EXPECT_EQ(web_contents->GetLastCommittedURL(), in_scope_url);
}

// Navigate window away from start_url to an out of scope URL, check that
// the next launch does navigate to start_url.
{
GURL out_of_scope_url = embedded_test_server()->GetURL("/empty.html");
NavigateToURLAndWait(browser_1, out_of_scope_url);
EXPECT_EQ(web_contents->GetLastCommittedURL(), out_of_scope_url);

Browser* browser_2 = LaunchWebAppBrowserAndWait(profile(), app_id);
EXPECT_EQ(browser_1, browser_2);
EXPECT_EQ(AwaitNextLaunchParamsTargetUrl(browser_2), start_url.spec());
EXPECT_EQ(web_contents->GetLastCommittedURL(), start_url);
}

// Trigger launch during navigation, check that the navigation gets
// cancelled.
{
ASSERT_TRUE(EvalJs(web_contents, "window.thisIsTheSamePage = true")
.ExtractBool());

GURL hanging_url = embedded_test_server()->GetURL("/hang");
NavigateParams params(browser_1, hanging_url, ui::PAGE_TRANSITION_LINK);
Navigate(&params);
EXPECT_EQ(web_contents->GetVisibleURL(), hanging_url);
EXPECT_EQ(web_contents->GetLastCommittedURL(), start_url);

ASSERT_TRUE(SetUpNextLaunchParamsTargetUrlPromise(browser_1));
Browser* browser_2 = LaunchWebAppBrowser(profile(), app_id);
EXPECT_EQ(browser_1, browser_2);
EXPECT_EQ(AwaitNextLaunchParamsTargetUrlPromise(browser_2),
start_url.spec());
EXPECT_EQ(web_contents->GetVisibleURL(), start_url);
EXPECT_EQ(web_contents->GetLastCommittedURL(), start_url);

// Check that we never left the current page.
EXPECT_TRUE(
EvalJs(web_contents, "window.thisIsTheSamePage").ExtractBool());
}
}

private:
base::test::ScopedFeatureList feature_list_{
blink::features::kWebAppEnableLaunchHandler};
Expand All @@ -128,8 +216,7 @@ IN_PROC_BROWSER_TEST_F(WebAppLaunchHandlerBrowserTest, RouteToEmpty) {
IN_PROC_BROWSER_TEST_F(WebAppLaunchHandlerBrowserTest, RouteToAuto) {
AppId app_id =
InstallTestWebApp("/web_apps/get_manifest.html?route_to_auto.json");
EXPECT_EQ(GetLaunchHandler(app_id),
(LaunchHandler{RouteTo::kAuto, NavigateExistingClient::kAlways}));
EXPECT_EQ(GetLaunchHandler(app_id), (LaunchHandler{RouteTo::kAuto}));

std::string start_url = GetWebApp(app_id)->start_url().spec();

Expand All @@ -145,9 +232,7 @@ IN_PROC_BROWSER_TEST_F(WebAppLaunchHandlerBrowserTest, RouteToAuto) {
IN_PROC_BROWSER_TEST_F(WebAppLaunchHandlerBrowserTest, RouteToNewClient) {
AppId app_id =
InstallTestWebApp("/web_apps/get_manifest.html?route_to_new_client.json");
EXPECT_EQ(
GetLaunchHandler(app_id),
(LaunchHandler{RouteTo::kNewClient, NavigateExistingClient::kAlways}));
EXPECT_EQ(GetLaunchHandler(app_id), (LaunchHandler{RouteTo::kNewClient}));

std::string start_url = GetWebApp(app_id)->start_url().spec();

Expand All @@ -166,9 +251,7 @@ IN_PROC_BROWSER_TEST_F(WebAppLaunchHandlerBrowserTest,
RouteToNewClientNavigateNever) {
AppId app_id = InstallTestWebApp(
"/web_apps/get_manifest.html?route_to_new_client_navigate_never.json");
EXPECT_EQ(
GetLaunchHandler(app_id),
(LaunchHandler{RouteTo::kNewClient, NavigateExistingClient::kNever}));
EXPECT_EQ(GetLaunchHandler(app_id), (LaunchHandler{RouteTo::kNewClient}));

std::string start_url = GetWebApp(app_id)->start_url().spec();

Expand All @@ -181,102 +264,58 @@ IN_PROC_BROWSER_TEST_F(WebAppLaunchHandlerBrowserTest,
EXPECT_NE(browser_1, browser_2);
}

IN_PROC_BROWSER_TEST_F(WebAppLaunchHandlerBrowserTest, RouteToExistingClient) {
IN_PROC_BROWSER_TEST_F(WebAppLaunchHandlerBrowserTest,
RouteToDeprecatedExistingClient) {
AppId app_id = InstallTestWebApp(
"/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 = LaunchWebAppBrowserAndWait(profile(), app_id);
content::WebContents* web_contents =
browser_1->tab_strip_model()->GetActiveWebContents();
GURL start_url = embedded_test_server()->GetURL(
"/web_apps/basic.html?route_to=existing-client&navigate=empty");
EXPECT_EQ(web_contents->GetLastCommittedURL(), start_url);
EXPECT_EQ(AwaitNextLaunchParamsTargetUrl(browser_1), start_url.spec());

// Navigate window away from start_url to check that the next launch navs to
// start_url again.
GURL alt_url = embedded_test_server()->GetURL("/web_apps/basic.html");
NavigateToURLAndWait(browser_1, alt_url);
EXPECT_EQ(web_contents->GetLastCommittedURL(), alt_url);
"/web_apps/get_manifest.html?"
"route_to_deprecated_existing_client_navigate_empty.json");
ExpectExistingClientNavigateBehaviour(
app_id,
embedded_test_server()->GetURL(
"/web_apps/basic.html?route_to=existing-client&navigate=empty"));
}

Browser* browser_2 = LaunchWebAppBrowserAndWait(profile(), app_id);
EXPECT_EQ(browser_1, browser_2);
EXPECT_EQ(web_contents->GetLastCommittedURL(), start_url);
EXPECT_EQ(AwaitNextLaunchParamsTargetUrl(browser_2), start_url.spec());
IN_PROC_BROWSER_TEST_F(WebAppLaunchHandlerBrowserTest,
RouteToExistingClientNavigate) {
// This JSON includes the deprecated "existing_client_navigate": "never" to
// verify it has no effect when using the new "route_to":
// "existing-client-navigate" syntax.
AppId app_id = InstallTestWebApp(
"/web_apps/get_manifest.html?"
"route_to_existing_client_navigate_deprecated_navigate_never.json");
ExpectExistingClientNavigateBehaviour(
app_id,
embedded_test_server()->GetURL(
"/web_apps/"
"basic.html?route_to=existing-client-navigate&navigate=never"));
}

// TODO(crbug.com/1308334): Fix flakiness.
IN_PROC_BROWSER_TEST_F(WebAppLaunchHandlerBrowserTest,
DISABLED_RouteToExistingClientNavigateNever) {
DISABLED_RouteToDeprecatedExistingClientNavigateNever) {
AppId app_id = InstallTestWebApp(
"/web_apps/"
"get_manifest.html?route_to_existing_client_navigate_never.json");
EXPECT_EQ(GetLaunchHandler(app_id),
(LaunchHandler{RouteTo::kExistingClient,
NavigateExistingClient::kNever}));

Browser* browser_1 = LaunchWebAppBrowserAndWait(profile(), app_id);
content::WebContents* web_contents =
browser_1->tab_strip_model()->GetActiveWebContents();
GURL start_url = embedded_test_server()->GetURL(
"/web_apps/basic.html?route_to=existing-client&navigate=never");
EXPECT_EQ(web_contents->GetLastCommittedURL(), start_url);
EXPECT_EQ(AwaitNextLaunchParamsTargetUrl(browser_1), start_url.spec());

// Navigate window away from start_url to an in scope URL, check that the next
// launch doesn't navigate to start_url.
{
GURL in_scope_url = embedded_test_server()->GetURL("/web_apps/basic.html");
NavigateToURLAndWait(browser_1, in_scope_url);
EXPECT_EQ(web_contents->GetLastCommittedURL(), in_scope_url);

ASSERT_TRUE(SetUpNextLaunchParamsTargetUrlPromise(browser_1));
Browser* browser_2 = LaunchWebAppBrowser(profile(), app_id);
EXPECT_EQ(browser_1, browser_2);
EXPECT_EQ(AwaitNextLaunchParamsTargetUrlPromise(browser_2),
start_url.spec());
EXPECT_EQ(web_contents->GetLastCommittedURL(), in_scope_url);
}

// Navigate window away from start_url to an out of scope URL, check that the
// next launch does navigate to start_url.
{
GURL out_of_scope_url = embedded_test_server()->GetURL("/empty.html");
NavigateToURLAndWait(browser_1, out_of_scope_url);
EXPECT_EQ(web_contents->GetLastCommittedURL(), out_of_scope_url);

Browser* browser_2 = LaunchWebAppBrowserAndWait(profile(), app_id);
EXPECT_EQ(browser_1, browser_2);
EXPECT_EQ(AwaitNextLaunchParamsTargetUrl(browser_2), start_url.spec());
EXPECT_EQ(web_contents->GetLastCommittedURL(), start_url);
}

// Trigger launch during navigation, check that the navigation gets cancelled.
{
ASSERT_TRUE(
EvalJs(web_contents, "window.thisIsTheSamePage = true").ExtractBool());

GURL hanging_url = embedded_test_server()->GetURL("/hang");
NavigateParams params(browser_1, hanging_url, ui::PAGE_TRANSITION_LINK);
Navigate(&params);
EXPECT_EQ(web_contents->GetVisibleURL(), hanging_url);
EXPECT_EQ(web_contents->GetLastCommittedURL(), start_url);

ASSERT_TRUE(SetUpNextLaunchParamsTargetUrlPromise(browser_1));
Browser* browser_2 = LaunchWebAppBrowser(profile(), app_id);
EXPECT_EQ(browser_1, browser_2);
EXPECT_EQ(AwaitNextLaunchParamsTargetUrlPromise(browser_2),
start_url.spec());
EXPECT_EQ(web_contents->GetVisibleURL(), start_url);
EXPECT_EQ(web_contents->GetLastCommittedURL(), start_url);
"/web_apps/get_manifest.html?"
"route_to_deprecated_existing_client_navigate_never.json");
ExpectExistingClientRetainBehaviour(
app_id,
embedded_test_server()->GetURL(
"/web_apps/basic.html?route_to=existing-client&navigate=never"));
}

// Check that we never left the current page.
EXPECT_TRUE(EvalJs(web_contents, "window.thisIsTheSamePage").ExtractBool());
}
// TODO(crbug.com/1308334): Fix flakiness.
IN_PROC_BROWSER_TEST_F(WebAppLaunchHandlerBrowserTest,
DISABLED_RouteToExistingClientRetain) {
// This JSON includes the deprecated "existing_client_navigate": "always" to
// verify it has no effect when using the new "route_to":
// "existing-client-retain" syntax.
AppId app_id = InstallTestWebApp(
"/web_apps/get_manifest.html?"
"route_to_existing_client_retain_deprecated_navigate_always.json");
ExpectExistingClientRetainBehaviour(
app_id,
embedded_test_server()->GetURL(
"/web_apps/"
"basic.html?route_to=existing-client-navigate&navigate=always"));
}

IN_PROC_BROWSER_TEST_F(WebAppLaunchHandlerBrowserTest, GlobalLaunchQueue) {
Expand Down Expand Up @@ -437,8 +476,7 @@ IN_PROC_BROWSER_TEST_F(WebAppLaunchHandlerOriginTrialBrowserTest, OriginTrial) {
// Origin trial should grant the app access.
WebAppProvider& provider = *WebAppProvider::GetForTest(browser()->profile());
EXPECT_EQ(provider.registrar().GetAppById(app_id)->launch_handler(),
(LaunchHandler{LaunchHandler::RouteTo::kExistingClient,
LaunchHandler::NavigateExistingClient::kNever}));
(LaunchHandler{RouteTo::kExistingClientRetain}));

// Open the page again with the token missing.
{
Expand Down
31 changes: 22 additions & 9 deletions chrome/browser/ui/web_applications/web_app_launch_process.cc
View file
Expand Up @@ -185,7 +185,7 @@ WindowOpenDisposition WebAppLaunchProcess::GetNavigationDisposition(

// If launch handler is routing to an existing client, we want to use the
// existing WebContents rather than opening a new tab.
if (GetLaunchRouteTo() == LaunchHandler::RouteTo::kExistingClient) {
if (RouteToExistingClient()) {
return WindowOpenDisposition::CURRENT_TAB;
}

Expand All @@ -205,12 +205,26 @@ LaunchHandler::RouteTo WebAppLaunchProcess::GetLaunchRouteTo() const {
return launch_handler.route_to;
}

LaunchHandler::NavigateExistingClient
WebAppLaunchProcess::GetLaunchNavigateExistingClient() const {
DCHECK(web_app_);
return web_app_->launch_handler()
.value_or(LaunchHandler())
.navigate_existing_client;
bool WebAppLaunchProcess::RouteToExistingClient() const {
switch (GetLaunchRouteTo()) {
case LaunchHandler::RouteTo::kAuto:
case LaunchHandler::RouteTo::kNewClient:
return false;
case LaunchHandler::RouteTo::kExistingClientNavigate:
case LaunchHandler::RouteTo::kExistingClientRetain:
return true;
}
}

bool WebAppLaunchProcess::NeverNavigateExistingClients() const {
switch (GetLaunchRouteTo()) {
case LaunchHandler::RouteTo::kAuto:
case LaunchHandler::RouteTo::kNewClient:
case LaunchHandler::RouteTo::kExistingClientNavigate:
return false;
case LaunchHandler::RouteTo::kExistingClientRetain:
return true;
}
}

content::WebContents* WebAppLaunchProcess::MaybeLaunchSystemWebApp(
Expand Down Expand Up @@ -301,8 +315,7 @@ WebAppLaunchProcess::NavigateResult WebAppLaunchProcess::MaybeNavigateBrowser(

content::WebContents* existing_tab = tab_strip->GetActiveWebContents();
DCHECK(existing_tab);
if (GetLaunchNavigateExistingClient() ==
LaunchHandler::NavigateExistingClient::kNever &&
if (NeverNavigateExistingClients() &&
provider_.registrar().IsUrlInAppScope(existing_tab->GetLastCommittedURL(),
params_.app_id)) {
// If the web contents is currently navigating then interrupt it. The
Expand Down
3 changes: 2 additions & 1 deletion chrome/browser/ui/web_applications/web_app_launch_process.h
View file
Expand Up @@ -49,7 +49,8 @@ class WebAppLaunchProcess {
content::WebContents* MaybeLaunchSystemWebApp(const GURL& launch_url);
std::tuple<Browser*, bool /*is_new_browser*/> EnsureBrowser();
LaunchHandler::RouteTo GetLaunchRouteTo() const;
LaunchHandler::NavigateExistingClient GetLaunchNavigateExistingClient() const;
bool RouteToExistingClient() const;
bool NeverNavigateExistingClients() const;

Browser* MaybeFindBrowserForLaunch() const;
Browser* CreateBrowserForLaunch();
Expand Down
8 changes: 3 additions & 5 deletions chrome/browser/ui/web_applications/web_app_link_capturing_browsertest.cc
View file
Expand Up @@ -47,7 +47,6 @@ using ui_test_utils::BrowserChangeObserver;
namespace web_app {

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

#if BUILDFLAG(IS_CHROMEOS_ASH)

Expand Down Expand Up @@ -189,11 +188,10 @@ class WebAppLinkCapturingBrowserTest : public WebAppNavigationBrowserTest {
IN_PROC_BROWSER_TEST_F(WebAppLinkCapturingBrowserTest,
RouteToExistingClientFromBrowser) {
InstallTestApp(
"/web_apps/"
"get_manifest.html?route_to_existing_client_navigate_empty.json");
"/web_apps/get_manifest.html?"
"route_to_deprecated_existing_client_navigate_empty.json");
EXPECT_EQ(GetLaunchHandler(app_id_),
(LaunchHandler{RouteTo::kExistingClient,
NavigateExistingClient::kAlways}));
(LaunchHandler{RouteTo::kExistingClientNavigate}));

TurnOnLinkCapturing();

Expand Down

0 comments on commit 87b1485

Please sign in to comment.