Test suite covering both KURL and GURL.

This CL adds an AbstractUrlTest test suite that covers both KURL and
GURL.  Having tests that are shared across Blink and //url will help 1)
avoid code duplication and 2) ensure that Blink and //url behave in the
same way (at least wrt things covered by the shared tests).

Bug: 1164416
Change-Id: I6812c0230cc1ef91416aa9a8f967a5db15d4bf77
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2622736Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Auto-Submit: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842560}

third_party/blink/renderer/platform/weborigin/kurl_test.cc

@@ -41,6 +41,7 @@
 #include "third_party/blink/renderer/platform/weborigin/scheme_registry.h"
 #include "third_party/blink/renderer/platform/wtf/std_lib_extras.h"
 #include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
+#include "url/gurl_abstract_tests.h"
 #include "url/url_util.h"
 
 namespace blink {
@@ -778,20 +779,6 @@ TEST(KURLTest, LastPathComponent) {
   EXPECT_EQ(String(), invalid_utf8.LastPathComponent());
 }
 
-TEST(KURLTest, IsAboutBlankURL) {
-  EXPECT_TRUE(BlankURL().IsAboutBlankURL());
-
-  EXPECT_TRUE(KURL("about:blank").IsAboutBlankURL());
-  EXPECT_TRUE(KURL("about:blank#ref").IsAboutBlankURL());
-  EXPECT_TRUE(KURL("about:blank?query=123").IsAboutBlankURL());
-}
-
-TEST(KURLTest, IsAboutSrcdocURL) {
-  EXPECT_TRUE(KURL("about:srcdoc").IsAboutSrcdocURL());
-  EXPECT_TRUE(KURL("about:srcdoc#ref").IsAboutSrcdocURL());
-  EXPECT_TRUE(KURL("about:srcdoc?query=123").IsAboutSrcdocURL());
-}
-
 TEST(KURLTest, IsHierarchical) {
   // Note that it's debatable whether "filesystem" URLs are or hierarchical.
   // They're currently registered in the url lib as standard; but the parsed
@@ -1081,3 +1068,26 @@ INSTANTIATE_TEST_SUITE_P(All,
                          ::testing::ValuesIn(port_test_cases));
 
 }  // namespace blink
+
+// Apparently INSTANTIATE_TYPED_TEST_SUITE_P needs to be used in the same
+// namespace as where the typed test suite was defined.
+namespace url {
+
+class KURLTestTraits final : public UrlTraitsBase<blink::KURL> {
+ public:
+  UrlType CreateUrlFromString(base::StringPiece s) override {
+    return blink::KURL(String::FromUTF8(s));
+  }
+
+  bool IsAboutBlank(const UrlType& url) override {
+    return url.IsAboutBlankURL();
+  }
+
+  bool IsAboutSrcdoc(const UrlType& url) override {
+    return url.IsAboutSrcdocURL();
+  }
+};
+
+INSTANTIATE_TYPED_TEST_SUITE_P(KURL, AbstractUrlTest, KURLTestTraits);
+
+}  // namespace url

url/BUILD.gn

@@ -174,7 +174,10 @@ if (is_android) {
 source_set("url_test_support") {
   testonly = true
 
-  sources = [ "origin_abstract_tests.h" ]
+  sources = [
+    "gurl_abstract_tests.h",
+    "origin_abstract_tests.h",
+  ]
 
   public_deps = [
     ":url",

url/gurl_abstract_tests.h

+// Copyright 2021 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.
+
+#ifndef URL_GURL_ABSTRACT_TESTS_H_
+#define URL_GURL_ABSTRACT_TESTS_H_
+
+// AbstractUrlTest below abstracts away differences between GURL and blink::KURL
+// by parametrizing the tests with a class that has to be derived from
+// UrlTraitsBase below.
+template <typename TConcreteUrlType>
+class UrlTraitsBase {
+ public:
+  using UrlType = TConcreteUrlType;
+  UrlTraitsBase() = default;
+
+  // Constructing an origin.
+  virtual UrlType CreateUrlFromString(base::StringPiece s) = 0;
+
+  // Accessors for origin properties.
+  virtual bool IsAboutBlank(const UrlType& url) = 0;
+  virtual bool IsAboutSrcdoc(const UrlType& url) = 0;
+
+  // This type is non-copyable and non-moveable.
+  UrlTraitsBase(const UrlTraitsBase&) = delete;
+  UrlTraitsBase& operator=(const UrlTraitsBase&) = delete;
+};
+
+// Test suite for tests that cover both url::Url and blink::SecurityUrl.
+template <typename TUrlTraits>
+class AbstractUrlTest : public testing::Test {
+  static_assert(std::is_base_of<UrlTraitsBase<typename TUrlTraits::UrlType>,
+                                TUrlTraits>::value,
+                "TUrlTraits needs to expose the right members.");
+
+ protected:
+  // Wrappers that allow tests to ignore presence of `url_traits_`.
+  //
+  // Note that calling the wrappers needs to be prefixed with `this->...` to
+  // avoid hitting: explicit qualification required to use member 'IsAboutBlank'
+  // from dependent base class.
+  using UrlType = typename TUrlTraits::UrlType;
+  UrlType CreateUrlFromString(base::StringPiece s) {
+    return url_traits_.CreateUrlFromString(s);
+  }
+  bool IsAboutBlank(const UrlType& url) {
+    return url_traits_.IsAboutBlank(url);
+  }
+  bool IsAboutSrcdoc(const UrlType& url) {
+    return url_traits_.IsAboutSrcdoc(url);
+  }
+
+ private:
+  TUrlTraits url_traits_;
+};
+
+TYPED_TEST_SUITE_P(AbstractUrlTest);
+
+TYPED_TEST_P(AbstractUrlTest, IsAboutBlankTest) {
+  // See https://tools.ietf.org/html/rfc6694 which explicitly allows
+  // `about-query` and `about-fragment` parts in about: URLs.
+  const std::string kAboutBlankUrls[] = {"about:blank", "about:blank?foo",
+                                         "about:blank/#foo",
+                                         "about:blank?foo#foo"};
+  for (const auto& input : kAboutBlankUrls) {
+    SCOPED_TRACE(testing::Message() << "Test input: " << input);
+    auto url = this->CreateUrlFromString(input);
+    EXPECT_TRUE(this->IsAboutBlank(url));
+  }
+
+  const std::string kNotAboutBlankUrls[] = {"",
+                                            "about",
+                                            "about:",
+                                            "about:blanky",
+                                            "about:blan",
+                                            "about:about:blank:",
+                                            "data:blank",
+                                            "http:blank",
+                                            "about://blank",
+                                            "about:blank/foo",
+                                            "about://:8000/blank",
+                                            "about://foo:foo@/blank",
+                                            "foo@about:blank",
+                                            "foo:bar@about:blank",
+                                            "about:blank:8000",
+                                            "about:blANk"};
+  for (const auto& input : kNotAboutBlankUrls) {
+    SCOPED_TRACE(testing::Message() << "Test input: " << input);
+    auto url = this->CreateUrlFromString(input);
+    EXPECT_FALSE(this->IsAboutBlank(url));
+  }
+}
+
+TYPED_TEST_P(AbstractUrlTest, IsAboutSrcdocTest) {
+  // See https://tools.ietf.org/html/rfc6694 which explicitly allows
+  // `about-query` and `about-fragment` parts in about: URLs.
+  //
+  // `about:srcdoc` is defined in
+  // https://html.spec.whatwg.org/multipage/urls-and-fetching.html#about:srcdoc
+  // which refers to rfc6694 for details.
+  const std::string kAboutSrcdocUrls[] = {
+      "about:srcdoc", "about:srcdoc/", "about:srcdoc?foo", "about:srcdoc/#foo",
+      "about:srcdoc?foo#foo"};
+  for (const auto& input : kAboutSrcdocUrls) {
+    SCOPED_TRACE(testing::Message() << "Test input: " << input);
+    auto url = this->CreateUrlFromString(input);
+    EXPECT_TRUE(this->IsAboutSrcdoc(url));
+  }
+
+  const std::string kNotAboutSrcdocUrls[] = {"",
+                                             "about",
+                                             "about:",
+                                             "about:srcdocx",
+                                             "about:srcdo",
+                                             "about:about:srcdoc:",
+                                             "data:srcdoc",
+                                             "http:srcdoc",
+                                             "about:srcdo",
+                                             "about://srcdoc",
+                                             "about://srcdoc\\",
+                                             "about:srcdoc/foo",
+                                             "about://:8000/srcdoc",
+                                             "about://foo:foo@/srcdoc",
+                                             "foo@about:srcdoc",
+                                             "foo:bar@about:srcdoc",
+                                             "about:srcdoc:8000",
+                                             "about:srCDOc"};
+  for (const auto& input : kNotAboutSrcdocUrls) {
+    SCOPED_TRACE(testing::Message() << "Test input: " << input);
+    auto url = this->CreateUrlFromString(input);
+    EXPECT_FALSE(this->IsAboutSrcdoc(url));
+  }
+}
+
+REGISTER_TYPED_TEST_SUITE_P(AbstractUrlTest,
+                            IsAboutBlankTest,
+                            IsAboutSrcdocTest);
+
+#endif  // URL_GURL_ABSTRACT_TESTS_H_

url/gurl_unittest.cc

@@ -9,6 +9,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
+#include "url/gurl_abstract_tests.h"
 #include "url/origin.h"
 #include "url/url_canon.h"
 #include "url/url_test_utils.h"
@@ -883,70 +884,6 @@ TEST(GURLTest, PathForNonStandardURLs) {
   }
 }
 
-TEST(GURLTest, IsAboutBlank) {
-  // See https://tools.ietf.org/html/rfc6694 which explicitly allows
-  // `about-query` and `about-fragment` parts in about: URLs.
-  const std::string kAboutBlankUrls[] = {"about:blank", "about:blank?foo",
-                                         "about:blank/#foo",
-                                         "about:blank?foo#foo"};
-  for (const auto& url : kAboutBlankUrls)
-    EXPECT_TRUE(GURL(url).IsAboutBlank()) << url;
-
-  const std::string kNotAboutBlankUrls[] = {"",
-                                            "about",
-                                            "about:",
-                                            "about:blanky",
-                                            "about:blan",
-                                            "about:about:blank:",
-                                            "data:blank",
-                                            "http:blank",
-                                            "about://blank",
-                                            "about:blank/foo",
-                                            "about://:8000/blank",
-                                            "about://foo:foo@/blank",
-                                            "foo@about:blank",
-                                            "foo:bar@about:blank",
-                                            "about:blank:8000",
-                                            "about:blANk"};
-  for (const auto& url : kNotAboutBlankUrls)
-    EXPECT_FALSE(GURL(url).IsAboutBlank()) << url;
-}
-
-TEST(GURLTest, IsAboutSrcdoc) {
-  // See https://tools.ietf.org/html/rfc6694 which explicitly allows
-  // `about-query` and `about-fragment` parts in about: URLs.
-  //
-  // `about:srcdoc` is defined in
-  // https://html.spec.whatwg.org/multipage/urls-and-fetching.html#about:srcdoc
-  // which refers to rfc6694 for details.
-  const std::string kAboutSrcdocUrls[] = {
-      "about:srcdoc", "about:srcdoc/", "about:srcdoc?foo", "about:srcdoc/#foo",
-      "about:srcdoc?foo#foo"};
-  for (const auto& url : kAboutSrcdocUrls)
-    EXPECT_TRUE(GURL(url).IsAboutSrcdoc()) << url;
-
-  const std::string kNotAboutSrcdocUrls[] = {"",
-                                             "about",
-                                             "about:",
-                                             "about:srcdocx",
-                                             "about:srcdo",
-                                             "about:about:srcdoc:",
-                                             "data:srcdoc",
-                                             "http:srcdoc",
-                                             "about:srcdo",
-                                             "about://srcdoc",
-                                             "about://srcdoc\\",
-                                             "about:srcdoc/foo",
-                                             "about://:8000/srcdoc",
-                                             "about://foo:foo@/srcdoc",
-                                             "foo@about:srcdoc",
-                                             "foo:bar@about:srcdoc",
-                                             "about:srcdoc:8000",
-                                             "about:srCDOc"};
-  for (const auto& url : kNotAboutSrcdocUrls)
-    EXPECT_FALSE(GURL(url).IsAboutSrcdoc()) << url;
-}
-
 TEST(GURLTest, EqualsIgnoringRef) {
   const struct {
     const char* url_a;
@@ -1055,4 +992,17 @@ TEST(GURLTest, PortZero) {
   EXPECT_FALSE(default_port_origin.IsSameOriginWith(resolved_origin));
 }
 
+class GURLTestTraits final : public UrlTraitsBase<GURL> {
+ public:
+  UrlType CreateUrlFromString(base::StringPiece s) override { return GURL(s); }
+
+  bool IsAboutBlank(const UrlType& url) override { return url.IsAboutBlank(); }
+
+  bool IsAboutSrcdoc(const UrlType& url) override {
+    return url.IsAboutSrcdoc();
+  }
+};
+
+INSTANTIATE_TYPED_TEST_SUITE_P(GURL, AbstractUrlTest, GURLTestTraits);
+
 }  // namespace url