Commit 6bd3d56b authored by Glen Robertson's avatar Glen Robertson Committed by Commit Bot

Add IsShortcutApp and use when checking whether a URL is in scope.

Fixes issue where CanBookmarkAppHandleUrl could correctly be false for a PWA but
  then its launch URL would be checked anyway as if it was a Shortcut App.

Bug: 1006054
Change-Id: I5d5e58bd01f214f8cd42641f06afecb7fef2ab69
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1815803
Commit-Queue: Glen Robertson <glenrob@chromium.org>
Reviewed-by: default avatarAlan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#698812}
parent 33096a65
...@@ -1211,6 +1211,25 @@ IN_PROC_BROWSER_TEST_P(SharedPWATest, ...@@ -1211,6 +1211,25 @@ IN_PROC_BROWSER_TEST_P(SharedPWATest,
EXPECT_EQ(GetAppMenuCommandState(IDC_INSTALL_PWA, browser()), kNotPresent); EXPECT_EQ(GetAppMenuCommandState(IDC_INSTALL_PWA, browser()), kNotPresent);
} }
// Tests that an installed PWA is not used when out of scope by one path level.
IN_PROC_BROWSER_TEST_P(SharedPWATest, MenuOptionsOutsideInstalledPwaScope) {
ASSERT_TRUE(https_server()->Start());
NavigateToURLAndWait(
browser(),
https_server()->GetURL("/banners/scope_is_start_url/index.html"));
InstallPwaForCurrentUrl();
// Open a page that is one directory up from the installed PWA.
Browser* new_browser = NavigateInNewWindowAndAwaitInstallabilityCheck(
https_server()->GetURL("/banners/no_manifest_test_page.html"));
EXPECT_EQ(GetAppMenuCommandState(IDC_CREATE_SHORTCUT, new_browser), kEnabled);
EXPECT_EQ(GetAppMenuCommandState(IDC_INSTALL_PWA, new_browser), kNotPresent);
EXPECT_EQ(GetAppMenuCommandState(IDC_OPEN_IN_PWA_WINDOW, new_browser),
kNotPresent);
}
IN_PROC_BROWSER_TEST_P(SharedPWATest, InstallInstallableSite) { IN_PROC_BROWSER_TEST_P(SharedPWATest, InstallInstallableSite) {
base::UserActionTester user_action_tester; base::UserActionTester user_action_tester;
ASSERT_TRUE(https_server()->Start()); ASSERT_TRUE(https_server()->Start());
......
...@@ -96,4 +96,10 @@ std::vector<web_app::AppId> AppRegistrar::FindAppsInScope( ...@@ -96,4 +96,10 @@ std::vector<web_app::AppId> AppRegistrar::FindAppsInScope(
return in_scope; return in_scope;
} }
bool AppRegistrar::IsShortcutApp(const AppId& app_id) const {
// TODO (crbug/910016): Make app scope always return a value and record this
// distinction in some other way.
return !GetAppScope(app_id).has_value();
}
} // namespace web_app } // namespace web_app
...@@ -84,6 +84,9 @@ class AppRegistrar { ...@@ -84,6 +84,9 @@ class AppRegistrar {
// Finds all apps that are installed under |scope|. // Finds all apps that are installed under |scope|.
std::vector<AppId> FindAppsInScope(const GURL& scope) const; std::vector<AppId> FindAppsInScope(const GURL& scope) const;
// Returns whether the app is a shortcut app (as opposed to a PWA).
bool IsShortcutApp(const AppId& app_id) const;
// Returns true if the app with the specified |start_url| is currently fully // Returns true if the app with the specified |start_url| is currently fully
// locally installed. The provided |start_url| must exactly match the launch // locally installed. The provided |start_url| must exactly match the launch
// URL for the app; this method does not consult the app scope or match URLs // URL for the app; this method does not consult the app scope or match URLs
......
...@@ -7,6 +7,8 @@ ...@@ -7,6 +7,8 @@
#include "base/strings/string_piece.h" #include "base/strings/string_piece.h"
#include "base/values.h" #include "base/values.h"
#include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/web_applications/components/app_registrar.h"
#include "chrome/browser/web_applications/components/web_app_provider_base.h"
#include "chrome/common/chrome_features.h" #include "chrome/common/chrome_features.h"
#include "chrome/common/extensions/api/url_handlers/url_handlers_parser.h" #include "chrome/common/extensions/api/url_handlers/url_handlers_parser.h"
#include "chrome/common/extensions/manifest_handlers/app_launch_info.h" #include "chrome/common/extensions/manifest_handlers/app_launch_info.h"
...@@ -61,18 +63,17 @@ bool IsInNavigationScopeForLaunchUrl(const GURL& launch_url, const GURL& url) { ...@@ -61,18 +63,17 @@ bool IsInNavigationScopeForLaunchUrl(const GURL& launch_url, const GURL& url) {
base::StringPiece(url.spec()).substr(0, scope_str_length); base::StringPiece(url.spec()).substr(0, scope_str_length);
} }
const Extension* GetInstalledShortcutForUrl( const Extension* GetInstalledShortcutForUrl(Profile* profile, const GURL& url) {
content::BrowserContext* browser_context, const ExtensionPrefs* prefs = ExtensionPrefs::Get(profile);
const GURL& url) { web_app::AppRegistrar& registrar =
const ExtensionPrefs* prefs = ExtensionPrefs::Get(browser_context); web_app::WebAppProviderBase::GetProviderBase(profile)->registrar();
for (scoped_refptr<const Extension> app : for (scoped_refptr<const Extension> app :
ExtensionRegistry::Get(browser_context)->enabled_extensions()) { ExtensionRegistry::Get(profile)->enabled_extensions()) {
if (!app->from_bookmark()) if (!app->from_bookmark())
continue; continue;
if (!BookmarkAppIsLocallyInstalled(prefs, app.get())) if (!BookmarkAppIsLocallyInstalled(prefs, app.get()))
continue; continue;
// Skip PWAs. if (!registrar.IsShortcutApp(app->id()))
if (UrlHandlers::CanBookmarkAppHandleUrl(app.get(), url))
continue; continue;
const GURL launch_url = AppLaunchInfo::GetLaunchWebURL(app.get()); const GURL launch_url = AppLaunchInfo::GetLaunchWebURL(app.get());
......
...@@ -10,6 +10,7 @@ class BrowserContext; ...@@ -10,6 +10,7 @@ class BrowserContext;
} }
class GURL; class GURL;
class Profile;
namespace extensions { namespace extensions {
...@@ -40,9 +41,7 @@ bool IsInNavigationScopeForLaunchUrl(const GURL& launch_url, const GURL& url); ...@@ -40,9 +41,7 @@ bool IsInNavigationScopeForLaunchUrl(const GURL& launch_url, const GURL& url);
// Finds the first Shortcut App (a non-PWA Bookmark App) with |url| in its // Finds the first Shortcut App (a non-PWA Bookmark App) with |url| in its
// scope, returns nullptr if there are none. // scope, returns nullptr if there are none.
const Extension* GetInstalledShortcutForUrl( const Extension* GetInstalledShortcutForUrl(Profile* profile, const GURL& url);
content::BrowserContext* browser_context,
const GURL& url);
// Count a number of all bookmark apps which are installed by user // Count a number of all bookmark apps which are installed by user
// (non default-installed apps). // (non default-installed apps).
......
<head>
<title>Web app /banners/scope_is_start_url</title>
<link rel="manifest" href="manifest.webmanifest">
<script>navigator.serviceWorker.register('/banners/service_worker.js');</script>
</head>
{
"name": "Manifest test app in /banners/scope_is_start_url",
"icons": [
{
"src": "/banners/image-512px.png",
"sizes": "512x512",
"type": "image/png"
}
],
"scope": "/banners/scope_is_start_url",
"start_url": "/banners/scope_is_start_url",
"display": "standalone"
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment