Desktop PWAs: On reparenting to app window, clear navigation history

When a desktop PWA is installed and the user requests
'Open in a window', we clear navigation history that occurred before
the user most recently entered the app's scope.

When the user chooses "Open in <app>" from the browser's
three dot menu, we similarly prune navigation history.

If the current page is the user's first page in the app, the Back
button will be initially disabled.

The user can still access history using chrome://history/

BookmarkAppRegistrar::IsInstalled now returns false (instead of
crashing) when called with a Chrome packaged app id.

Bug: 1027393
Change-Id: I4855b952801f6279607fcb52e6ed56e12725e109
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1978317
Commit-Queue: Eric Willigers <ericwilligers@chromium.org>
Reviewed-by: Alexey Baskakov <loyso@chromium.org>
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#727211}

chrome/browser/ui/extensions/hosted_app_browsertest.cc

@@ -2193,6 +2193,13 @@ IN_PROC_BROWSER_TEST_P(HostedAppProcessModelTest, MAYBE_FromOutsideHostedApp) {
   }
 }
 
+// Tests that a packaged app is not considered an installed bookmark app.
+IN_PROC_BROWSER_TEST_P(HostedAppProcessModelTest,
+                       AppRegistrarExcludesPackaged) {
+  SetupApp("https_app");
+  EXPECT_FALSE(registrar().IsInstalled(app_->id()));
+}
+
 // Helper class that sets up two isolated origins, where one is a subdomain of
 // the other: https://isolated.com and https://very.isolated.com.
 class HostedAppIsolatedOriginTest : public HostedAppProcessModelTest {

chrome/browser/ui/web_applications/web_app_launch_utils.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/ui/web_applications/web_app_launch_utils.h"
 
 #include "base/feature_list.h"
+#include "base/strings/string_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
@@ -12,13 +13,22 @@
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/web_applications/components/app_registrar.h"
+#include "chrome/browser/web_applications/components/web_app_provider_base.h"
 #include "chrome/browser/web_applications/web_app_provider.h"
 #include "chrome/common/chrome_features.h"
 #include "components/omnibox/browser/location_bar_model.h"
+#include "content/public/browser/navigation_controller.h"
+#include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/web_contents.h"
+#include "url/gurl.h"
 
 namespace {
 
+bool IsInScope(content::NavigationEntry* entry, const std::string& scope_spec) {
+  return base::StartsWith(entry->GetURL().spec(), scope_spec,
+                          base::CompareCase::SENSITIVE);
+}
+
 Browser* ReparentWebContentsWithBrowserCreateParams(
     content::WebContents* contents,
     const Browser::CreateParams& browser_params) {
@@ -67,6 +77,26 @@ base::Optional<AppId> GetPwaForSecureActiveTab(Browser* browser) {
       web_contents->GetMainFrame()->GetLastCommittedURL());
 }
 
+void PrunePreScopeNavigationHistory(const GURL& scope,
+                                    content::WebContents* contents) {
+  content::NavigationController& navigation_controller =
+      contents->GetController();
+  if (!navigation_controller.CanPruneAllButLastCommitted())
+    return;
+
+  const std::string scope_spec = scope.spec();
+  int index = navigation_controller.GetEntryCount() - 1;
+  while (index >= 0 &&
+         IsInScope(navigation_controller.GetEntryAtIndex(index), scope_spec)) {
+    --index;
+  }
+
+  while (index >= 0) {
+    navigation_controller.RemoveEntryAtIndex(index);
+    --index;
+  }
+}
+
 Browser* ReparentWebAppForSecureActiveTab(Browser* browser) {
   base::Optional<AppId> app_id = GetPwaForSecureActiveTab(browser);
   if (!app_id)
@@ -81,6 +111,21 @@ Browser* ReparentWebContentsIntoAppBrowser(content::WebContents* contents,
   // Incognito tabs reparent correctly, but remain incognito without any
   // indication to the user, so disallow it.
   DCHECK(!profile->IsOffTheRecord());
+
+  // Clear navigation history that occurred before the user most recently
+  // entered the app's scope. The minimal-ui Back button will be initially
+  // disabled if the previous page was outside scope. Packaged apps are not
+  // affected.
+  AppRegistrar& registrar =
+      WebAppProviderBase::GetProviderBase(profile)->registrar();
+  if (registrar.IsInstalled(app_id)) {
+    base::Optional<GURL> app_scope = registrar.GetAppScope(app_id);
+    if (!app_scope)
+      app_scope = registrar.GetAppLaunchURL(app_id).GetWithoutFilename();
+
+    PrunePreScopeNavigationHistory(*app_scope, contents);
+  }
+
   Browser::CreateParams browser_params(Browser::CreateParams::CreateForApp(
       GenerateApplicationNameFromAppId(app_id), true /* trusted_source */,
       gfx::Rect(), profile, true /* user_gesture */));

chrome/browser/ui/web_applications/web_app_launch_utils.h

@@ -11,6 +11,7 @@
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
 
 class Browser;
+class GURL;
 
 namespace content {
 class WebContents;
@@ -20,6 +21,10 @@ namespace web_app {
 
 base::Optional<AppId> GetPwaForSecureActiveTab(Browser* browser);
 
+// Clears navigation history prior to user entering app scope.
+void PrunePreScopeNavigationHistory(const GURL& scope,
+                                    content::WebContents* contents);
+
 // Reparents the active tab into a new app browser for the web app that has the
 // tab's URL in its scope. Does nothing if the tab is not secure or there is no
 // applicable web app.

chrome/browser/ui/web_applications/web_app_launch_utils_unittest.cc

+// Copyright 2019 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 "chrome/browser/ui/web_applications/web_app_launch_utils.h"
+#include "chrome/browser/web_applications/test/web_app_test.h"
+#include "content/public/browser/navigation_controller.h"
+#include "content/public/browser/navigation_entry.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/test/navigation_simulator.h"
+#include "url/gurl.h"
+
+using content::NavigationSimulator;
+
+using WebAppLaunchUtilsTest = WebAppTest;
+
+TEST_F(WebAppLaunchUtilsTest, PruneHistory) {
+  const GURL scope("https://example.com/a/");
+
+  const GURL first("https://example.com/a/first");
+  const GURL second("https://example.com/a/second");
+  const GURL third("https://example.com/b/third");  // Out of scope.
+  const GURL fourth("https://example.com/a/fourth");
+  const GURL fifth("https://example.com/a/fifth");
+
+  NavigationSimulator::NavigateAndCommitFromBrowser(web_contents(), first);
+  NavigationSimulator::NavigateAndCommitFromBrowser(web_contents(), second);
+  NavigationSimulator::NavigateAndCommitFromBrowser(web_contents(), third);
+  NavigationSimulator::NavigateAndCommitFromBrowser(web_contents(), fourth);
+  NavigationSimulator::NavigateAndCommitFromBrowser(web_contents(), fifth);
+
+  web_app::PrunePreScopeNavigationHistory(scope, web_contents());
+  EXPECT_EQ(web_contents()->GetController().GetEntryCount(), 2);
+  EXPECT_EQ(web_contents()->GetController().GetEntryAtIndex(0)->GetURL(),
+            fourth);
+  EXPECT_EQ(web_contents()->GetController().GetEntryAtIndex(1)->GetURL(),
+            fifth);
+}

chrome/browser/web_applications/extensions/bookmark_app_registrar.cc

@@ -35,18 +35,19 @@ BookmarkAppRegistrar::BookmarkAppRegistrar(Profile* profile)
 BookmarkAppRegistrar::~BookmarkAppRegistrar() = default;
 
 bool BookmarkAppRegistrar::IsInstalled(const web_app::AppId& app_id) const {
-  return GetExtension(app_id) != nullptr;
+  const Extension* extension = GetExtension(app_id);
+  return extension && extension->from_bookmark();
 }
 
 bool BookmarkAppRegistrar::IsLocallyInstalled(
     const web_app::AppId& app_id) const {
-  const Extension* extension = GetExtension(app_id);
+  const Extension* extension = GetBookmarkApp(app_id);
   return extension && BookmarkAppIsLocallyInstalled(profile(), extension);
 }
 
 bool BookmarkAppRegistrar::WasInstalledByUser(
     const web_app::AppId& app_id) const {
-  const Extension* extension = GetExtension(app_id);
+  const Extension* extension = GetBookmarkApp(app_id);
   return extension && !extension->was_installed_by_default();
 }
 
@@ -83,29 +84,21 @@ void BookmarkAppRegistrar::OnShutdown(ExtensionRegistry* registry) {
   extension_observer_.RemoveAll();
 }
 
-const Extension* BookmarkAppRegistrar::GetExtension(
-    const web_app::AppId& app_id) const {
-  const Extension* extension =
-      ExtensionRegistry::Get(profile())->enabled_extensions().GetByID(app_id);
-  DCHECK(!extension || extension->from_bookmark());
-  return extension;
-}
-
 std::string BookmarkAppRegistrar::GetAppShortName(
     const web_app::AppId& app_id) const {
-  const Extension* extension = GetExtension(app_id);
+  const Extension* extension = GetBookmarkApp(app_id);
   return extension ? extension->short_name() : std::string();
 }
 
 std::string BookmarkAppRegistrar::GetAppDescription(
     const web_app::AppId& app_id) const {
-  const Extension* extension = GetExtension(app_id);
+  const Extension* extension = GetBookmarkApp(app_id);
   return extension ? extension->description() : std::string();
 }
 
 base::Optional<SkColor> BookmarkAppRegistrar::GetAppThemeColor(
     const web_app::AppId& app_id) const {
-  const Extension* extension = GetExtension(app_id);
+  const Extension* extension = GetBookmarkApp(app_id);
   if (!extension)
     return base::nullopt;
 
@@ -119,18 +112,18 @@ base::Optional<SkColor> BookmarkAppRegistrar::GetAppThemeColor(
 
 const GURL& BookmarkAppRegistrar::GetAppLaunchURL(
     const web_app::AppId& app_id) const {
-  const Extension* extension = GetExtension(app_id);
+  const Extension* extension = GetBookmarkApp(app_id);
   return extension ? AppLaunchInfo::GetLaunchWebURL(extension)
                    : GURL::EmptyGURL();
 }
 
 base::Optional<GURL> BookmarkAppRegistrar::GetAppScope(
     const web_app::AppId& app_id) const {
-  const Extension* extension = GetExtension(app_id);
+  const Extension* extension = GetBookmarkApp(app_id);
   if (!extension)
     return base::nullopt;
 
-  GURL scope_url = GetScopeURLFromBookmarkApp(GetExtension(app_id));
+  GURL scope_url = GetScopeURLFromBookmarkApp(GetBookmarkApp(app_id));
   if (scope_url.is_valid())
     return scope_url;
 
@@ -139,7 +132,7 @@ base::Optional<GURL> BookmarkAppRegistrar::GetAppScope(
 
 DisplayMode BookmarkAppRegistrar::GetAppDisplayMode(
     const web_app::AppId& app_id) const {
-  const Extension* extension = GetExtension(app_id);
+  const Extension* extension = GetBookmarkApp(app_id);
   if (!extension)
     return DisplayMode::kUndefined;
 
@@ -148,7 +141,7 @@ DisplayMode BookmarkAppRegistrar::GetAppDisplayMode(
 
 DisplayMode BookmarkAppRegistrar::GetAppUserDisplayMode(
     const web_app::AppId& app_id) const {
-  const Extension* extension = GetExtension(app_id);
+  const Extension* extension = GetBookmarkApp(app_id);
   if (!extension)
     return DisplayMode::kStandalone;
 
@@ -168,7 +161,7 @@ DisplayMode BookmarkAppRegistrar::GetAppUserDisplayMode(
 std::vector<WebApplicationIconInfo> BookmarkAppRegistrar::GetAppIconInfos(
     const web_app::AppId& app_id) const {
   std::vector<WebApplicationIconInfo> result;
-  const Extension* extension = GetExtension(app_id);
+  const Extension* extension = GetBookmarkApp(app_id);
   if (!extension)
     return result;
   for (const LinkedAppIcons::IconInfo& icon_info :
@@ -192,4 +185,17 @@ std::vector<web_app::AppId> BookmarkAppRegistrar::GetAppIds() const {
   return app_ids;
 }
 
+const Extension* BookmarkAppRegistrar::GetBookmarkApp(
+    const web_app::AppId& app_id) const {
+  const Extension* extension = GetExtension(app_id);
+  DCHECK(!extension || extension->from_bookmark());
+  return extension;
+}
+
+const Extension* BookmarkAppRegistrar::GetExtension(
+    const web_app::AppId& app_id) const {
+  return ExtensionRegistry::Get(profile())->enabled_extensions().GetByID(
+      app_id);
+}
+
 }  // namespace extensions

chrome/browser/web_applications/extensions/bookmark_app_registrar.h

@@ -53,6 +53,7 @@ class BookmarkAppRegistrar : public web_app::AppRegistrar,
   void OnShutdown(ExtensionRegistry* registry) override;
 
  private:
+  const Extension* GetBookmarkApp(const web_app::AppId& app_id) const;
   const Extension* GetExtension(const web_app::AppId& app_id) const;
 
   ScopedObserver<ExtensionRegistry, ExtensionRegistryObserver>

chrome/test/BUILD.gn

@@ -4034,6 +4034,7 @@ test("unit_tests") {
       "../browser/ui/toolbar/toolbar_actions_bar_unittest.h",
       "../browser/ui/toolbar/toolbar_actions_model_unittest.cc",
       "../browser/ui/web_applications/app_browser_controller_unittest.cc",
+      "../browser/ui/web_applications/web_app_launch_utils_unittest.cc",
       "../browser/ui/webui/downloads/downloads_dom_handler_unittest.cc",
       "../browser/ui/webui/downloads/downloads_list_tracker_unittest.cc",
       "../browser/ui/webui/downloads/mock_downloads_page.cc",