Use manifest for focus mode controller.

Now when "focus mode" loads it requests the page manifest, which
provides the correct scope URL. It still falls back to using the initial
URL as the scope.

Bug: 981703
Change-Id: I81a28541551ab373c948a79a5f8a62696df309ac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1972614
Commit-Queue: Dana Fried <dfried@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Jay Harris <harrisjay@chromium.org>
Cr-Commit-Position: refs/heads/master@{#727389}

chrome/browser/ui/manifest_web_app_browser_controller.cc

@@ -12,14 +12,14 @@
 #include "content/public/common/origin_util.h"
 #include "content/public/common/url_constants.h"
 #include "extensions/common/constants.h"
+#include "third_party/blink/public/common/manifest/manifest.h"
 #include "ui/gfx/favicon_size.h"
 #include "ui/gfx/image/image_skia.h"
 #include "url/gurl.h"
 
 ManifestWebAppBrowserController::ManifestWebAppBrowserController(
     Browser* browser)
-    : AppBrowserController(browser, /*app_id=*/base::nullopt),
-      app_launch_url_(GURL()) {}
+    : AppBrowserController(browser, /*app_id=*/base::nullopt) {}
 
 ManifestWebAppBrowserController::~ManifestWebAppBrowserController() = default;
 
@@ -83,14 +83,49 @@ GURL ManifestWebAppBrowserController::GetAppLaunchURL() const {
 }
 
 bool ManifestWebAppBrowserController::IsUrlInAppScope(const GURL& url) const {
-  // TODO(981703): Use the scope in the manifest instead of same origin check.
-  return url::IsSameOriginWith(GetAppLaunchURL(), url);
+  // Prefer to use manifest scope URL if available; fall back to app launch URL
+  // if not available. Manifest fallback is always launch URL minus filename,
+  // query, and fragment.
+  const GURL scope_url = !manifest_scope_.is_empty()
+                             ? manifest_scope_
+                             : GetAppLaunchURL().GetWithoutFilename();
+
+  return IsInScope(url, scope_url);
 }
 
 void ManifestWebAppBrowserController::OnTabInserted(
     content::WebContents* contents) {
-  if (app_launch_url_.is_empty())
+  // Since we are experimenting with multi-tab PWAs, we only try to load the
+  // manifest if this is the first web contents being loaded in this window.
+  DCHECK(!browser()->tab_strip_model()->empty());
+  if (browser()->tab_strip_model()->count() == 1) {
     app_launch_url_ = contents->GetURL();
+    contents->GetManifest(
+        base::BindOnce(&ManifestWebAppBrowserController::OnManifestLoaded,
+                       weak_factory_.GetWeakPtr()));
+  }
   AppBrowserController::OnTabInserted(contents);
   UpdateCustomTabBarVisibility(false);
 }
+
+void ManifestWebAppBrowserController::OnManifestLoaded(
+    const GURL& manifest_url,
+    const blink::Manifest& manifest) {
+  manifest_scope_ = manifest.scope;
+}
+
+// static
+bool ManifestWebAppBrowserController::IsInScope(const GURL& url,
+                                                const GURL& scope) {
+  if (!url::IsSameOriginWith(scope, url))
+    return false;
+
+  std::string scope_path = scope.path();
+  if (base::EndsWith(scope_path, "/", base::CompareCase::SENSITIVE))
+    scope_path = scope_path.substr(0, scope_path.length() - 1);
+
+  const std::string url_path = url.path();
+  return url_path == scope_path ||
+         base::StartsWith(url_path, scope_path + "/",
+                          base::CompareCase::SENSITIVE);
+}

chrome/browser/ui/manifest_web_app_browser_controller.h

@@ -8,6 +8,7 @@
 #include <string>
 
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "base/optional.h"
 #include "base/strings/string16.h"
 #include "chrome/browser/extensions/extension_uninstall_dialog.h"
@@ -18,6 +19,10 @@
 
 class Browser;
 
+namespace blink {
+struct Manifest;
+}
+
 namespace gfx {
 class ImageSkia;
 }
@@ -44,7 +49,15 @@ class ManifestWebAppBrowserController : public web_app::AppBrowserController {
   void OnTabInserted(content::WebContents* contents) override;
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(ManifestWebAppBrowserControllerTest, IsInScope);
+  void OnManifestLoaded(const GURL& manifest_url,
+                        const blink::Manifest& manifest);
+
+  static bool IsInScope(const GURL& url, const GURL& scope);
+
   GURL app_launch_url_;
+  GURL manifest_scope_;
+  base::WeakPtrFactory<ManifestWebAppBrowserController> weak_factory_{this};
 };
 
 #endif  // CHROME_BROWSER_UI_MANIFEST_WEB_APP_BROWSER_CONTROLLER_H_

chrome/browser/ui/manifest_web_app_browser_controller_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/manifest_web_app_browser_controller.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+TEST(ManifestWebAppBrowserControllerTest, IsInScope) {
+  struct TestCase {
+    const char* scope;
+    const char* url;
+    bool is_in_scope;
+  };
+
+  const TestCase kTestCases[]{
+      {"https://www.foo.com/", "https://www.foo.com/", true},
+      {"https://www.foo.com/", "https://www.foo.com/bar/", true},
+      {"https://www.foo.com/", "https://www.foo.com/bar/fnord.asp", true},
+      {"https://www.foo.com/", "https://www.foo.com/bar?baz=2", true},
+      {"https://www.foo.com/", "https://www.foo.com/bar#foobar", true},
+      {"https://www.foo.com/x/", "https://www.foo.com/x/", true},
+      {"https://www.foo.com/x/", "https://www.foo.com/x/bar/", true},
+      {"https://www.foo.com/x/", "https://www.foo.com/x/bar/fnord.asp", true},
+      {"https://www.foo.com/x/", "https://www.foo.com/x/bar?baz=2", true},
+      {"https://www.foo.com/x/", "https://www.foo.com/x/bar#foobar", true},
+      {"https://www.foo.com/x", "https://www.foo.com/x", true},
+      {"https://www.foo.com/x", "https://www.foo.com/x/", true},
+      {"https://www.foo.com/x", "https://www.foo.com/x/bar/", true},
+      {"https://www.foo.com/x", "https://www.foo.com/x/bar/fnord.asp", true},
+      {"https://www.foo.com/x", "https://www.foo.com/x/bar?baz=2", true},
+      {"https://www.foo.com/x", "https://www.foo.com/x/bar#foobar", true},
+      {"https://www.foo.com/x/", "https://www.foo.com/xavier/", false},
+      {"https://www.foo.com/x/", "https://www.foo.com/", false},
+      {"https://www.foo.com/x/", "https://www.foo.com/y/", false},
+      {"https://www.foo.com/x/", "https://web.foo.com/x/", false},
+      {"https://www.foo.com/x/", "http://www.foo.com/x/", false},
+      {"https://www.foo.com/x/", "http://www.foo.com:123/x/", false},
+  };
+
+  for (const TestCase test_case : kTestCases) {
+    EXPECT_EQ(test_case.is_in_scope,
+              ManifestWebAppBrowserController::IsInScope(GURL(test_case.url),
+                                                         GURL(test_case.scope)))
+        << "URL: " << test_case.url << " SCOPE: " << test_case.scope;
+  }
+}

chrome/test/BUILD.gn

@@ -3999,6 +3999,7 @@ test("unit_tests") {
       "../browser/ui/in_product_help/global_media_controls_in_product_help_unittest.cc",
       "../browser/ui/in_product_help/reopen_tab_in_product_help_trigger_unittest.cc",
       "../browser/ui/in_product_help/reopen_tab_in_product_help_unittest.cc",
+      "../browser/ui/manifest_web_app_browser_controller_unittest.cc",
       "../browser/ui/omnibox/chrome_omnibox_navigation_observer_unittest.cc",
       "../browser/ui/omnibox/clipboard_utils_unittest.cc",
       "../browser/ui/page_info/permission_menu_model_unittest.cc",