Test suite covering both blink::SecurityOrigin and url::Origin.

This CL adds an AbstractOriginTest test suite that covers both
blink::SecurityOrigin and url::Origin.  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: I1a4ecb32eba4d31ca07328a43f4392586588aca5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2606715
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#841679}

third_party/blink/renderer/platform/BUILD.gn

@@ -2194,6 +2194,7 @@ source_set("blink_platform_unittests_sources") {
     "//ui/gfx/geometry/mojom:test_interfaces_blink",
     "//ui/gfx/mojom:test_interfaces_blink",
     "//url",
+    "//url:url_test_support",
     "//url/mojom:test_url_mojom_gurl_blink",
   ]
 

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

@@ -47,6 +47,7 @@
 #include "third_party/blink/renderer/platform/wtf/text/string_operators.h"
 #include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
 #include "url/gurl.h"
+#include "url/origin_abstract_tests.h"
 #include "url/url_util.h"
 
 namespace blink {
@@ -909,24 +910,6 @@ TEST_F(SecurityOriginTest, NonStandardScheme) {
   EXPECT_TRUE(origin->IsOpaque());
 }
 
-TEST_F(SecurityOriginTest, NonStandardSchemeWithAndroidWebViewHack) {
-  url::ScopedSchemeRegistryForTests scoped_registry;
-  url::EnableNonStandardSchemesForAndroidWebView();
-
-  // Regression test for https://crbug.com/896059.
-  scoped_refptr<const SecurityOrigin> origin =
-      SecurityOrigin::CreateFromString("cow://");
-  EXPECT_FALSE(origin->IsOpaque());
-  EXPECT_EQ("cow", origin->Protocol());
-  EXPECT_EQ("", origin->Host());
-  EXPECT_EQ(0, origin->Port());
-
-  // about:blank translates into an opaque origin, even in presence of
-  // EnableNonStandardSchemesForAndroidWebView.
-  origin = SecurityOrigin::CreateFromString("about:blank");
-  EXPECT_TRUE(origin->IsOpaque());
-}
-
 TEST_F(SecurityOriginTest, OpaqueIsolatedCopy) {
   scoped_refptr<const SecurityOrigin> origin =
       SecurityOrigin::CreateUniqueOpaque();
@@ -1214,3 +1197,38 @@ TEST_F(SecurityOriginTest, PercentEncodesHost) {
 }
 
 }  // 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 BlinkSecurityOriginTestTraits final
+    : public url::OriginTraitsBase<scoped_refptr<blink::SecurityOrigin>> {
+ public:
+  OriginType CreateOriginFromString(base::StringPiece s) const override {
+    return blink::SecurityOrigin::CreateFromString(
+        String(s.data(), s.length()));
+  }
+
+  bool IsOpaque(const OriginType& origin) const override {
+    return origin->IsOpaque();
+  }
+
+  std::string GetScheme(const OriginType& origin) const override {
+    return origin->Protocol().Utf8();
+  }
+
+  std::string GetHost(const OriginType& origin) const override {
+    return origin->Host().Utf8();
+  }
+
+  uint16_t GetPort(const OriginType& origin) const override {
+    return origin->Port();
+  }
+};
+
+INSTANTIATE_TYPED_TEST_SUITE_P(BlinkSecurityOrigin,
+                               AbstractOriginTest,
+                               BlinkSecurityOriginTestTraits);
+
+}  // namespace url

url/BUILD.gn

@@ -171,6 +171,20 @@ if (is_android) {
   }
 }
 
+source_set("url_test_support") {
+  testonly = true
+
+  sources = [ "origin_abstract_tests.h" ]
+
+  public_deps = [
+    ":url",
+    "//base",
+    "//base/test:test_support",
+    "//testing/gmock",
+    "//testing/gtest",
+  ]
+}
+
 test("url_unittests") {
   sources = [
     "gurl_unittest.cc",
@@ -186,6 +200,7 @@ test("url_unittests") {
 
   deps = [
     ":url",
+    ":url_test_support",
     "//base",
     "//base/test:test_support",
     "//testing/gmock",

url/origin_abstract_tests.h

@@ -0,0 +1,99 @@
+// Copyright 2020 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_ORIGIN_ABSTRACT_TESTS_H_
+#define URL_ORIGIN_ABSTRACT_TESTS_H_
+
+#include <string>
+#include <type_traits>
+
+#include "base/strings/string_piece.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "url/url_util.h"
+
+namespace url {
+
+// AbstractOriginTest below abstracts away differences between url::Origin and
+// blink::SecurityOrigin by parametrizing the tests with a class that has to be
+// derived from OriginTraitsBase below.
+template <typename TConcreteOriginType>
+class OriginTraitsBase {
+ public:
+  using OriginType = TConcreteOriginType;
+  OriginTraitsBase() = default;
+
+  // Constructing an origin.
+  virtual OriginType CreateOriginFromString(base::StringPiece s) const = 0;
+
+  // Accessors for origin properties.
+  virtual bool IsOpaque(const OriginType& origin) const = 0;
+  virtual std::string GetScheme(const OriginType& origin) const = 0;
+  virtual std::string GetHost(const OriginType& origin) const = 0;
+  virtual uint16_t GetPort(const OriginType& origin) const = 0;
+
+  // This type is non-copyable and non-moveable.
+  OriginTraitsBase(const OriginTraitsBase&) = delete;
+  OriginTraitsBase& operator=(const OriginTraitsBase&) = delete;
+};
+
+// Test suite for tests that cover both url::Origin and blink::SecurityOrigin.
+template <typename TOriginTraits>
+class AbstractOriginTest : public testing::Test {
+  static_assert(
+      std::is_base_of<OriginTraitsBase<typename TOriginTraits::OriginType>,
+                      TOriginTraits>::value,
+      "TOriginTraits needs to expose the right members.");
+
+ protected:
+  // Wrappers that allow tests to ignore presence of `origin_traits_`.
+  //
+  // Note that calling the wrappers needs to be prefixed with `this->...` to
+  // avoid hitting: explicit qualification required to use member 'IsOpaque'
+  // from dependent base class.
+  using OriginType = typename TOriginTraits::OriginType;
+  OriginType CreateOriginFromString(base::StringPiece s) const {
+    return origin_traits_.CreateOriginFromString(s);
+  }
+  bool IsOpaque(const OriginType& origin) const {
+    return origin_traits_.IsOpaque(origin);
+  }
+  std::string GetScheme(const OriginType& origin) const {
+    return origin_traits_.GetScheme(origin);
+  }
+  std::string GetHost(const OriginType& origin) const {
+    return origin_traits_.GetHost(origin);
+  }
+  uint16_t GetPort(const OriginType& origin) const {
+    return origin_traits_.GetPort(origin);
+  }
+
+ private:
+  TOriginTraits origin_traits_;
+};
+
+TYPED_TEST_SUITE_P(AbstractOriginTest);
+
+TYPED_TEST_P(AbstractOriginTest, NonStandardSchemeWithAndroidWebViewHack) {
+  ScopedSchemeRegistryForTests scoped_registry;
+  EnableNonStandardSchemesForAndroidWebView();
+
+  // Regression test for https://crbug.com/896059.
+  auto origin = this->CreateOriginFromString("cow://");
+  EXPECT_FALSE(this->IsOpaque(origin));
+  EXPECT_EQ("cow", this->GetScheme(origin));
+  EXPECT_EQ("", this->GetHost(origin));
+  EXPECT_EQ(0, this->GetPort(origin));
+
+  // about:blank translates into an opaque origin, even in presence of
+  // EnableNonStandardSchemesForAndroidWebView.
+  origin = this->CreateOriginFromString("about:blank");
+  EXPECT_TRUE(this->IsOpaque(origin));
+}
+
+REGISTER_TYPED_TEST_SUITE_P(AbstractOriginTest,
+                            NonStandardSchemeWithAndroidWebViewHack);
+
+}  // namespace url
+
+#endif  // URL_ORIGIN_ABSTRACT_TESTS_H_

url/origin_unittest.cc

@@ -10,10 +10,13 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 #include "url/origin.h"
+#include "url/origin_abstract_tests.h"
 #include "url/url_util.h"
 
 namespace url {
 
+namespace {
+
 void ExpectParsedUrlsEqual(const GURL& a, const GURL& b) {
   EXPECT_EQ(a, b);
   const Parsed& a_parsed = a.parsed_for_possibly_invalid_spec();
@@ -36,6 +39,8 @@ void ExpectParsedUrlsEqual(const GURL& a, const GURL& b) {
   EXPECT_EQ(a_parsed.ref.len, b_parsed.ref.len);
 }
 
+}  // namespace
+
 class OriginTest : public ::testing::Test {
  public:
   void SetUp() override {
@@ -671,22 +676,6 @@ TEST_F(OriginTest, NonStandardScheme) {
   EXPECT_TRUE(origin.opaque());
 }
 
-TEST_F(OriginTest, NonStandardSchemeWithAndroidWebViewHack) {
-  EnableNonStandardSchemesForAndroidWebView();
-
-  // Regression test for https://crbug.com/896059.
-  Origin origin = Origin::Create(GURL("cow://"));
-  EXPECT_FALSE(origin.opaque());
-  EXPECT_EQ("cow", origin.scheme());
-  EXPECT_EQ("", origin.host());
-  EXPECT_EQ(0, origin.port());
-
-  // about:blank translates into an opaque origin, even in presence of
-  // EnableNonStandardSchemesForAndroidWebView.
-  origin = Origin::Create(GURL("about:blank"));
-  EXPECT_TRUE(origin.opaque());
-}
-
 TEST_F(OriginTest, CanBeDerivedFrom) {
   AddStandardScheme("new-standard", SchemeType::SCHEME_WITH_HOST);
   Origin opaque_unique_origin = Origin();
@@ -974,4 +963,31 @@ TEST_F(OriginTest, DeserializeValidNonce) {
   EXPECT_EQ(opaque.GetDebugString(), deserialized.value().GetDebugString());
 }
 
+class UrlOriginTestTraits final : public OriginTraitsBase<Origin> {
+ public:
+  OriginType CreateOriginFromString(base::StringPiece s) const override {
+    return Origin::Create(GURL(s));
+  }
+
+  bool IsOpaque(const OriginType& origin) const override {
+    return origin.opaque();
+  }
+
+  std::string GetScheme(const OriginType& origin) const override {
+    return origin.scheme();
+  }
+
+  std::string GetHost(const OriginType& origin) const override {
+    return origin.host();
+  }
+
+  uint16_t GetPort(const OriginType& origin) const override {
+    return origin.port();
+  }
+};
+
+INSTANTIATE_TYPED_TEST_SUITE_P(UrlOrigin,
+                               AbstractOriginTest,
+                               UrlOriginTestTraits);
+
 }  // namespace url