Commit d48c643c authored by Hoch Hochkeppel's avatar Hoch Hochkeppel Committed by Commit Bot

WebApps: Disable "Create shortcut" for error pages

Updates the check used for gating the availability of various commands
(CanCreateWebApp) to include a check for the current page being in an
error state, correctly reflecting the functionality of the code
associated with this check (CreateWebAppFromCurrentWebContents).
Currently the only user scenario this changes is the enabled state of
the "Creates shortcut" command, but this ensures that any other
commands leveraging the same functionality will be appropriately gated
as well. Other commands already using this check (such as the command
for installing a PWA) are already disabled in the error page state due
to additional restrictions.

Test: Navigate to an invalid page (e.g. "foo.bar") and invoke
More tools > Create shortcut.  Without this change, nothing will appear
to happen until navigating to a valid page, at which point the "Create
shortcut?" dialog will finally appear.  With this change the "Create
shortcut" menu option will be disabled until you navigate to a valid
page.

Bug: 1056709
Change-Id: I9cd15876ca9bc1443b776fe8ea326608cd8ea0ca
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2063031
Commit-Queue: Hoch Hochkeppel <mhochk@microsoft.com>
Reviewed-by: default avatarAlan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#745285}
parent 1ac37d68
...@@ -471,6 +471,18 @@ IN_PROC_BROWSER_TEST_P(WebAppBrowserTest, ShortcutMenuOptionsInIncognito) { ...@@ -471,6 +471,18 @@ IN_PROC_BROWSER_TEST_P(WebAppBrowserTest, ShortcutMenuOptionsInIncognito) {
kNotPresent); kNotPresent);
} }
// Tests that both installing a PWA and creating a shortcut app are disabled for
// an error page.
IN_PROC_BROWSER_TEST_P(WebAppBrowserTest, ShortcutMenuOptionsForErrorPage) {
ASSERT_TRUE(https_server()->Start());
EXPECT_FALSE(NavigateAndAwaitInstallabilityCheck(
browser(), https_server()->GetURL("/invalid_path.html")));
EXPECT_EQ(GetAppMenuCommandState(IDC_CREATE_SHORTCUT, browser()), kDisabled);
EXPECT_EQ(GetAppMenuCommandState(IDC_INSTALL_PWA, browser()), kNotPresent);
}
// Tests that both installing a PWA and creating a shortcut app are available // Tests that both installing a PWA and creating a shortcut app are available
// for an installable PWA. // for an installable PWA.
IN_PROC_BROWSER_TEST_P(WebAppBrowserTest, IN_PROC_BROWSER_TEST_P(WebAppBrowserTest,
......
...@@ -25,6 +25,7 @@ ...@@ -25,6 +25,7 @@
#include "chrome/browser/web_applications/web_app_provider.h" #include "chrome/browser/web_applications/web_app_provider.h"
#include "chrome/common/chrome_features.h" #include "chrome/common/chrome_features.h"
#include "chrome/common/web_application_info.h" #include "chrome/common/web_application_info.h"
#include "content/public/browser/navigation_entry.h"
namespace web_app { namespace web_app {
...@@ -71,6 +72,10 @@ bool CanCreateWebApp(const Browser* browser) { ...@@ -71,6 +72,10 @@ bool CanCreateWebApp(const Browser* browser) {
browser->tab_strip_model()->GetActiveWebContents(); browser->tab_strip_model()->GetActiveWebContents();
if (!WebAppProvider::GetForWebContents(web_contents)) if (!WebAppProvider::GetForWebContents(web_contents))
return false; return false;
content::NavigationEntry* entry =
web_contents->GetController().GetLastCommittedEntry();
bool is_error_page =
entry && entry->GetPageType() == content::PAGE_TYPE_ERROR;
Profile* web_contents_profile = Profile* web_contents_profile =
Profile::FromBrowserContext(web_contents->GetBrowserContext()); Profile::FromBrowserContext(web_contents->GetBrowserContext());
banners::AppBannerManager* app_banner_manager = banners::AppBannerManager* app_banner_manager =
...@@ -80,7 +85,7 @@ bool CanCreateWebApp(const Browser* browser) { ...@@ -80,7 +85,7 @@ bool CanCreateWebApp(const Browser* browser) {
return AreWebAppsUserInstallable(web_contents_profile) && return AreWebAppsUserInstallable(web_contents_profile) &&
IsValidWebAppUrl(web_contents->GetLastCommittedURL()) && IsValidWebAppUrl(web_contents->GetLastCommittedURL()) &&
!externally_installed; !is_error_page && !externally_installed;
} }
bool CanPopOutWebApp(Profile* profile) { bool CanPopOutWebApp(Profile* profile) {
......
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