Improve logging for URL canonicalization failure

When NDEBUG is not set GURL::InitializeFromCanonicalSpec() re-canonicalises the input URL and DCHECKs that the results are the same. Change to using DCHECK_EQ so that the mismatched values are printed. Also add a print operator for url::Component so that it can be printed by CHECK statements. BUG=1301836 Change-Id: Ic6d679c7b84e36116a24be90ac40a4dd0430e825 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3515381 Reviewed-by: Peter Kasting <pkasting@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Commit-Queue: Adam Rice <ricea@chromium.org> Cr-Commit-Position: refs/heads/main@{#982216}
marcosholgado · Mar 17, 2022 · 43a5300 · 43a5300
1 parent 7477003
commit 43a5300
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 19 deletions.
diff --git a/components/url_formatter/url_fixer_unittest.cc b/components/url_formatter/url_fixer_unittest.cc
@@ -21,14 +21,6 @@
 #include "url/gurl.h"
 #include "url/third_party/mozilla/url_parse.h"
 
-namespace url {
-
-std::ostream& operator<<(std::ostream& os, const Component& part) {
-  return os << "(begin=" << part.begin << ", len=" << part.len << ")";
-}
-
-}  // namespace url
-
 struct SegmentCase {
   const std::string input;
   const std::string result;

diff --git a/url/gurl.cc b/url/gurl.cc
@@ -110,17 +110,17 @@ void GURL::InitializeFromCanonicalSpec() {
       // removed from a "foo:hello #ref" URL (see http://crbug.com/291747).
       GURL test_url(spec_, RETAIN_TRAILING_PATH_WHITEPACE);
 
-      DCHECK(test_url.is_valid_ == is_valid_);
-      DCHECK(test_url.spec_ == spec_);
-
-      DCHECK(test_url.parsed_.scheme == parsed_.scheme);
-      DCHECK(test_url.parsed_.username == parsed_.username);
-      DCHECK(test_url.parsed_.password == parsed_.password);
-      DCHECK(test_url.parsed_.host == parsed_.host);
-      DCHECK(test_url.parsed_.port == parsed_.port);
-      DCHECK(test_url.parsed_.path == parsed_.path);
-      DCHECK(test_url.parsed_.query == parsed_.query);
-      DCHECK(test_url.parsed_.ref == parsed_.ref);
+      DCHECK_EQ(test_url.is_valid_, is_valid_);
+      DCHECK_EQ(test_url.spec_, spec_);
+
+      DCHECK_EQ(test_url.parsed_.scheme, parsed_.scheme);
+      DCHECK_EQ(test_url.parsed_.username, parsed_.username);
+      DCHECK_EQ(test_url.parsed_.password, parsed_.password);
+      DCHECK_EQ(test_url.parsed_.host, parsed_.host);
+      DCHECK_EQ(test_url.parsed_.port, parsed_.port);
+      DCHECK_EQ(test_url.parsed_.path, parsed_.path);
+      DCHECK_EQ(test_url.parsed_.query, parsed_.query);
+      DCHECK_EQ(test_url.parsed_.ref, parsed_.ref);
     }
   }
 #endif

diff --git a/url/third_party/mozilla/url_parse.cc b/url/third_party/mozilla/url_parse.cc
@@ -692,6 +692,11 @@ bool DoExtractQueryKeyValue(const CHAR* spec,
 
 }  // namespace
 
+COMPONENT_EXPORT(URL)
+std::ostream& operator<<(std::ostream& os, const Component& component) {
+  return os << '{' << component.begin << ", " << component.len << "}";
+}
+
 Parsed::Parsed() : potentially_dangling_markup(false), inner_parsed_(NULL) {}
 
 Parsed::Parsed(const Parsed& other)

diff --git a/url/third_party/mozilla/url_parse.h b/url/third_party/mozilla/url_parse.h
@@ -5,6 +5,8 @@
 #ifndef URL_THIRD_PARTY_MOZILLA_URL_PARSE_H_
 #define URL_THIRD_PARTY_MOZILLA_URL_PARSE_H_
 
+#include <iosfwd>
+
 #include "base/component_export.h"
 
 namespace url {
@@ -47,6 +49,10 @@ struct Component {
   int len;    // Will be -1 if the component is unspecified.
 };
 
+// Permit printing Components by CHECK macros.
+COMPONENT_EXPORT(URL)
+std::ostream& operator<<(std::ostream& os, const Component& component);
+
 // Helper that returns a component created with the given begin and ending
 // points. The ending point is non-inclusive.
 inline Component MakeRange(int begin, int end) {