[KURL] Avoid re-hashing input if is already canonicalized

third_party/WebKit/Source/platform/weborigin/KURL.cpp

Index: third_party/WebKit/Source/platform/weborigin/KURL.cpp
diff --git a/third_party/WebKit/Source/platform/weborigin/KURL.cpp b/third_party/WebKit/Source/platform/weborigin/KURL.cpp
index 666ec1043795a491dc0698c7ab5ff94c498730f4..995f6ba0facc5973ed1be52f9eab74528119e8a6 100644
--- a/third_party/WebKit/Source/platform/weborigin/KURL.cpp
+++ b/third_party/WebKit/Source/platform/weborigin/KURL.cpp
@@ -717,11 +717,29 @@ bool protocolIs(const String& url, const char* protocol) {
 void KURL::init(const KURL& base,
                 const String& relative,
                 const WTF::TextEncoding* queryEncoding) {
+  url::RawCanonOutputT<char> canonical;
   if (!relative.isNull() && relative.is8Bit()) {
     StringUTF8Adaptor relativeUTF8(relative);
-    init(base, relativeUTF8.data(), relativeUTF8.length(), queryEncoding);
-  } else
-    init(base, relative.characters16(), relative.length(), queryEncoding);
+    int relativeLength = static_cast<int>(relativeUTF8.length());

[esprehn, 2016/10/25 22:23:50]

This is scary, our strings can be unsigned in length, but net URL is using int?
I think this means you can overflow this thing. I think you'd want to clamp?

[Charlie Harrison, 2016/10/27 02:12:36]

Done.

+    init(base, relativeUTF8.data(), relativeLength, queryEncoding, &canonical);
+
+    // If the relative input is exactly equal to the canonical output, do not
+    // re-allocate the string. Note that this dangerously makes the input String
+    // an AtomicString, which is not safe for Strings that are meant to be
+    // passed between threads.
+    if (!relative.isEmpty() && relative.containsOnlyASCII() &&

[esprehn, 2016/10/25 22:23:50]

Why does containsOnlyASCII() matter here? I don't think there's any situation
where the length is the same and the latin1 representation and the utf8
representation mean different things.

[Charlie Harrison, 2016/10/27 02:12:36]

You're right. removed.

+        relativeLength == canonical.length() &&
+        !memcmp(canonical.data(), relativeUTF8.data(), canonical.length())) {
+      m_string = AtomicString(relative);
+    }
+  } else {
+    init(base, relative.characters16(), relative.length(), queryEncoding,
+         &canonical);
+  }
+
+  if (!m_string)
+    m_string = AtomicString::fromUTF8(canonical.data(), canonical.length());
+
   initProtocolIsInHTTPFamily();
   initInnerURL();
 }
@@ -730,7 +748,8 @@ template <typename CHAR>
 void KURL::init(const KURL& base,
                 const CHAR* relative,
                 int relativeLength,
-                const WTF::TextEncoding* queryEncoding) {
+                const WTF::TextEncoding* queryEncoding,
+                url::RawCanonOutputT<char>* output) {
   // As a performance optimization, we do not use the charset converter
   // if encoding is UTF-8 or other Unicode encodings. Note that this is
   // per HTML5 2.5.3 (resolving URL). The URL canonicalizer will be more
@@ -747,14 +766,9 @@ void KURL::init(const KURL& base,
 
   StringUTF8Adaptor baseUTF8(base.getString());
 
-  url::RawCanonOutputT<char> output;
   m_isValid = url::ResolveRelative(baseUTF8.data(), baseUTF8.length(),
                                    base.m_parsed, relative, relativeLength,
-                                   charsetConverter, &output, &m_parsed);
-
-  // See FIXME in KURLPrivate in the header. If canonicalization has not
-  // changed the string, we can avoid an extra allocation by using assignment.
-  m_string = AtomicString::fromUTF8(output.data(), output.length());
+                                   charsetConverter, output, &m_parsed);
 }
 
 void KURL::initInnerURL() {