Skip to content

Commit

Permalink
Don't treat bookmark apps that aren't locally installed as PWAs.
Browse files Browse the repository at this point in the history
This means if a PWA is installed on one machine and synced to another
machine such that it isn't locally installed (i.e. it is synced to a
non-Chrome OS machine), it won't be treated as an installed PWA. This
affects features such as the navigation options available.

TBR=benwells@chromium.org

(cherry picked from commit e13d316)

Bug: 874841
Change-Id: I42667edbd7d9ac7dbe2a088d4b11bda422beee8b
Reviewed-on: https://chromium-review.googlesource.com/1195382
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Commit-Queue: Ben Wells <benwells@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#587874}
Reviewed-on: https://chromium-review.googlesource.com/1203750
Reviewed-by: Ben Wells <benwells@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#18}
Cr-Branched-From: 79f7c91-refs/heads/master@{#587811}
  • Loading branch information
Ben Wells committed Sep 4, 2018
1 parent 3a090fd commit 04d5152
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "chrome/browser/ui/extensions/application_launch.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/browser/web_applications/components/web_app_helpers.h"
#include "chrome/browser/web_applications/extensions/bookmark_app_util.h"
#include "chrome/common/chrome_features.h"
#include "chrome/common/extensions/manifest_handlers/app_launch_info.h"
#include "chrome/test/base/ui_test_utils.h"
Expand Down Expand Up @@ -590,6 +591,33 @@ IN_PROC_BROWSER_TEST_P(BookmarkAppNavigationThrottleExperimentalLinkBrowserTest,
ExpectNavigationResultHistogramEquals(global_histogram(), {});
}

// Tests that clicking a link to an app that isn't locally installed does not
// open a new app window.
IN_PROC_BROWSER_TEST_P(BookmarkAppNavigationThrottleExperimentalLinkBrowserTest,
NotLocallyInstalledApp) {
InstallTestBookmarkApp();

// Part of the installation process (setting that this is a locally installed
// app) runs asynchronously. Wait for that to complete before setting locally
// installed to false.
base::RunLoop().RunUntilIdle();
SetBookmarkAppIsLocallyInstalled(browser()->profile(), test_bookmark_app(),
false /* is_locally_installed */);

NavigateToLaunchingPage();

const GURL app_url = https_server().GetURL(GetAppUrlHost(), GetAppUrlPath());
TestTabActionDoesNotOpenAppWindow(
app_url,
base::BindOnce(&ClickLinkAndWait,
browser()->tab_strip_model()->GetActiveWebContents(),
app_url, LinkTarget::SELF, GetParam()));

// Non-locally installed apps are not considered when looking for a target
// app, so it's as if there was no app installed for the URL.
ExpectNavigationResultHistogramEquals(global_histogram(), {});
}

// Tests that clicking a link with target="_self" to the app's app_url opens the
// Bookmark App.
IN_PROC_BROWSER_TEST_P(BookmarkAppNavigationThrottleExperimentalLinkBrowserTest,
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/extensions/extension_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/webui/extensions/extension_icon_source.h"
#include "chrome/browser/web_applications/extensions/bookmark_app_util.h"
#include "chrome/common/chrome_features.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/common/extensions/api/url_handlers/url_handlers_parser.h"
Expand Down Expand Up @@ -342,6 +343,8 @@ const Extension* GetInstalledPwaForUrl(
ExtensionRegistry::Get(context)->enabled_extensions()) {
if (!app->from_bookmark())
continue;
if (!BookmarkAppIsLocallyInstalled(prefs, app.get()))
continue;
if (launch_container_filter &&
GetLaunchContainer(prefs, app.get()) != *launch_container_filter) {
continue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include "chrome/browser/ui/extensions/app_launch_params.h"
#include "chrome/browser/ui/extensions/application_launch.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/browser/web_applications/extensions/bookmark_app_util.h"
#include "chrome/common/chrome_features.h"
#include "chrome/common/chrome_render_frame.mojom.h"
#include "chrome/common/render_messages.h"
Expand Down Expand Up @@ -363,6 +364,7 @@ IN_PROC_BROWSER_TEST_F(ContextMenuBrowserTest,
ASSERT_FALSE(menu->IsItemInRangePresent(IDC_OPEN_LINK_IN_PROFILE_FIRST,
IDC_OPEN_LINK_IN_PROFILE_LAST));
}

IN_PROC_BROWSER_TEST_F(ContextMenuBrowserTest,
OpenInAppPresentForURLsInScopeOfNonWindowedBookmarkApp) {
base::test::ScopedFeatureList feature_list;
Expand Down Expand Up @@ -400,6 +402,31 @@ IN_PROC_BROWSER_TEST_F(ContextMenuBrowserTest,
IDC_OPEN_LINK_IN_PROFILE_LAST));
}

IN_PROC_BROWSER_TEST_F(ContextMenuBrowserTest,
OpenInAppAbsentForURLsInNonLocallyInstalledApp) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(features::kDesktopPWAWindowing);
const extensions::Extension* app = InstallTestBookmarkApp(GURL(kAppUrl1));

// Part of the installation process (setting that this is a locally installed
// app) runs asynchronously. Wait for that to complete before setting locally
// installed to false.
base::RunLoop().RunUntilIdle();
SetBookmarkAppIsLocallyInstalled(browser()->profile(), app,
false /* is_locally_installed */);

std::unique_ptr<TestRenderViewContextMenu> menu =
CreateContextMenuMediaTypeNone(GURL(kAppUrl1), GURL(kAppUrl1));

ASSERT_TRUE(menu->IsItemPresent(IDC_CONTENT_CONTEXT_OPENLINKNEWTAB));
ASSERT_TRUE(menu->IsItemPresent(IDC_CONTENT_CONTEXT_OPENLINKNEWWINDOW));
ASSERT_TRUE(menu->IsItemPresent(IDC_CONTENT_CONTEXT_COPYLINKLOCATION));
ASSERT_FALSE(menu->IsItemPresent(IDC_CONTENT_CONTEXT_OPENLINKBOOKMARKAPP));
ASSERT_FALSE(menu->IsItemPresent(IDC_CONTENT_CONTEXT_OPENLINKINPROFILE));
ASSERT_FALSE(menu->IsItemInRangePresent(IDC_OPEN_LINK_IN_PROFILE_FIRST,
IDC_OPEN_LINK_IN_PROFILE_LAST));
}

IN_PROC_BROWSER_TEST_F(ContextMenuBrowserTest,
InAppOpenEntryPresentForRegularURLs) {
base::test::ScopedFeatureList feature_list;
Expand Down

0 comments on commit 04d5152

Please sign in to comment.