Revert of Optimize KURL protocols (patchset #9 id:160001 of...

Revert of Optimize KURL protocols (patchset #9 id:160001 of https://codereview.chromium.org/2463703002/ )

Reason for revert:
Causing DCHECK flakes. crbug.com/674388

Original issue's description:
> Optimize KURL protocols
>
> This patch optimizes KURL::protocol and KURL::protocolIs by keeping
> an AtomicString m_protocol on KURL. This reduces string allocations
> throughout the code using KURL::protocol().
>
> This also fixes an inconsistency with KURL::protocolIs that will return
> true for invalid URLs.
>
> BUG=348655
>
> Committed: https://crrev.com/775abc2d7c903f191f7b24f8b299ebabbea3f624
> Cr-Commit-Position: refs/heads/master@{#438197}

TBR=esprehn@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=348655,674388

Review-Url: https://codereview.chromium.org/2586253004
Cr-Commit-Position: refs/heads/master@{#439722}

third_party/WebKit/Source/core/loader/MixedContentCheckerTest.cpp

@@ -38,6 +38,7 @@ TEST(MixedContentCheckerTest, IsMixedContent) {
       {"https://example.com/foo", "blob:null/foo", false},
       {"https://example.com/foo", "filesystem:https://example.com/foo", false},
       {"https://example.com/foo", "filesystem:http://example.com/foo", false},
+      {"https://example.com/foo", "filesystem:null/foo", false},
 
       {"https://example.com/foo", "http://example.com/foo", true},
       {"https://example.com/foo", "http://google.com/foo", true},

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

@@ -34,7 +34,6 @@
 #include "wtf/StdLibExtras.h"
 #include "wtf/text/CString.h"
 #include "wtf/text/StringHash.h"
-#include "wtf/text/StringStatics.h"
 #include "wtf/text/StringUTF8Adaptor.h"
 #include "wtf/text/TextEncoding.h"
 #include <algorithm>
@@ -49,7 +48,6 @@ static const int invalidPortNumber = 0xFFFF;
 
 static void assertProtocolIsGood(const char* protocol) {
 #if ENABLE(ASSERT)
-  DCHECK_NE(protocol, "");
   const char* p = protocol;
   while (*p) {
     ASSERT(*p > ' ' && *p < 0x7F && !(*p >= 'A' && *p <= 'Z'));
@@ -243,7 +241,7 @@ KURL::KURL(const AtomicString& canonicalString,
       m_protocolIsInHTTPFamily(false),
       m_parsed(parsed),
       m_string(canonicalString) {
-  initProtocolMetadata();
+  initProtocolIsInHTTPFamily();
   initInnerURL();
 }
 
@@ -255,7 +253,6 @@ KURL::KURL(WTF::HashTableDeletedValueType)
 KURL::KURL(const KURL& other)
     : m_isValid(other.m_isValid),
       m_protocolIsInHTTPFamily(other.m_protocolIsInHTTPFamily),
-      m_protocol(other.m_protocol),
       m_parsed(other.m_parsed),
       m_string(other.m_string) {
   if (other.m_innerURL.get())
@@ -267,7 +264,6 @@ KURL::~KURL() {}
 KURL& KURL::operator=(const KURL& other) {
   m_isValid = other.m_isValid;
   m_protocolIsInHTTPFamily = other.m_protocolIsInHTTPFamily;
-  m_protocol = other.m_protocol;
   m_parsed = other.m_parsed;
   m_string = other.m_string;
   if (other.m_innerURL)
@@ -281,7 +277,6 @@ KURL KURL::copy() const {
   KURL result;
   result.m_isValid = m_isValid;
   result.m_protocolIsInHTTPFamily = m_protocolIsInHTTPFamily;
-  result.m_protocol = m_protocol.isolatedCopy();
   result.m_parsed = m_parsed;
   result.m_string = m_string.isolatedCopy();
   if (m_innerURL)
@@ -317,7 +312,7 @@ bool KURL::hasPath() const {
 
 String KURL::lastPathComponent() const {
   if (!m_isValid)
-    return stringViewForInvalidComponent().toString();
+    return stringForInvalidComponent();
   ASSERT(!m_string.isNull());
 
   // When the output ends in a slash, WebCore has different expectations than
@@ -341,8 +336,7 @@ String KURL::lastPathComponent() const {
 }
 
 String KURL::protocol() const {
-  DCHECK_EQ(componentString(m_parsed.scheme), m_protocol);
-  return m_protocol;
+  return componentString(m_parsed.scheme);
 }
 
 String KURL::host() const {
@@ -371,9 +365,6 @@ unsigned short KURL::port() const {
   return static_cast<unsigned short>(port);
 }
 
-// TODO(csharrison): Migrate pass() and user() to return a StringView. Most
-// consumers just need to know if the string is empty.
-
 String KURL::pass() const {
   // Bug: https://bugs.webkit.org/show_bug.cgi?id=21015 this function returns
   // a null string when the password is empty, which we duplicate here.
@@ -776,8 +767,9 @@ void KURL::init(const KURL& base,
     m_string = AtomicString::fromUTF8(output.data(), output.length());
   }
 
-  initProtocolMetadata();
+  initProtocolIsInHTTPFamily();
   initInnerURL();
+  DCHECK_EQ(protocol(), protocol().lower());
 }
 
 void KURL::initInnerURL() {
@@ -794,26 +786,50 @@ void KURL::initInnerURL() {
     m_innerURL.reset();
 }
 
-void KURL::initProtocolMetadata() {
+template <typename CHAR>
+bool internalProtocolIs(const url::Component& scheme,
+                        const CHAR* spec,
+                        const char* protocol) {
+  const CHAR* begin = spec + scheme.begin;
+  const CHAR* end = begin + scheme.len;
+
+  while (begin != end && *protocol) {
+    ASSERT(toASCIILower(*protocol) == *protocol);
+    if (toASCIILower(*begin++) != *protocol++)
+      return false;
+  }
+
+  // Both strings are equal (ignoring case) if and only if all of the characters
+  // were equal, and the end of both has been reached.
+  return begin == end && !*protocol;
+}
+
+template <typename CHAR>
+bool checkIfProtocolIsInHTTPFamily(const url::Component& scheme,
+                                   const CHAR* spec) {
+  if (scheme.len == 4)
+    return internalProtocolIs(scheme, spec, "http");
+  if (scheme.len == 5)
+    return internalProtocolIs(scheme, spec, "https");
+  if (scheme.len == 7)
+    return internalProtocolIs(scheme, spec, "http-so");
+  if (scheme.len == 8)
+    return internalProtocolIs(scheme, spec, "https-so");
+  return false;
+}
+
+void KURL::initProtocolIsInHTTPFamily() {
   if (!m_isValid) {
     m_protocolIsInHTTPFamily = false;
-    m_protocol = componentString(m_parsed.scheme);
     return;
   }
 
-  DCHECK(!m_string.isNull());
-  StringView protocol = componentStringView(m_parsed.scheme);
-  m_protocolIsInHTTPFamily = true;
-  if (protocol == WTF::httpsAtom) {
-    m_protocol = WTF::httpsAtom;
-  } else if (protocol == WTF::httpAtom) {
-    m_protocol = WTF::httpAtom;
-  } else {
-    m_protocol = AtomicString(protocol.toString());
-    m_protocolIsInHTTPFamily =
-        m_protocol == "http-so" || m_protocol == "https-so";
-  }
-  DCHECK_EQ(m_protocol, m_protocol.lower());
+  ASSERT(!m_string.isNull());
+  m_protocolIsInHTTPFamily =
+      m_string.is8Bit() ? checkIfProtocolIsInHTTPFamily(m_parsed.scheme,
+                                                        m_string.characters8())
+                        : checkIfProtocolIsInHTTPFamily(
+                              m_parsed.scheme, m_string.characters16());
 }
 
 bool KURL::protocolIs(const char* protocol) const {
@@ -824,16 +840,26 @@ bool KURL::protocolIs(const char* protocol) const {
   // instead.
   // FIXME: Chromium code needs to be fixed for this assert to be enabled.
   // ASSERT(strcmp(protocol, "javascript"));
-  return m_protocol == protocol;
+
+  if (m_string.isNull() || m_parsed.scheme.len <= 0)
+    return *protocol == '\0';
+
+  return m_string.is8Bit()
+             ? internalProtocolIs(m_parsed.scheme, m_string.characters8(),
+                                  protocol)
+             : internalProtocolIs(m_parsed.scheme, m_string.characters16(),
+                                  protocol);
 }
 
-StringView KURL::stringViewForInvalidComponent() const {
-  return m_string.isNull() ? StringView() : StringView("", 0);
+String KURL::stringForInvalidComponent() const {
+  if (m_string.isNull())
+    return String();
+  return emptyString();
 }
 
-StringView KURL::componentStringView(const url::Component& component) const {
+String KURL::componentString(const url::Component& component) const {
   if (!m_isValid || component.len <= 0)
-    return stringViewForInvalidComponent();
+    return stringForInvalidComponent();
   // begin and len are in terms of bytes which do not match
   // if string() is UTF-16 and input contains non-ASCII characters.
   // However, the only part in urlString that can contain non-ASCII
@@ -842,14 +868,7 @@ StringView KURL::componentStringView(const url::Component& component) const {
   // byte) will be longer than what's needed by 'mid'. However, mid
   // truncates len to avoid go past the end of a string so that we can
   // get away without doing anything here.
-
-  int maxLength = getString().length() - component.begin;
-  return StringView(getString(), component.begin,
-                    component.len > maxLength ? maxLength : component.len);
-}
-
-String KURL::componentString(const url::Component& component) const {
-  return componentStringView(component).toString();
+  return getString().substring(component.begin, component.len);
 }
 
 template <typename CHAR>
@@ -863,7 +882,6 @@ void KURL::replaceComponents(const url::Replacements<CHAR>& replacements) {
 
   m_parsed = newParsed;
   m_string = AtomicString::fromUTF8(output.data(), output.length());
-  initProtocolMetadata();
 }
 
 bool KURL::isSafeToSendToAnotherThread() const {

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

@@ -196,24 +196,17 @@ class PLATFORM_EXPORT KURL {
             const String& relative,
             const WTF::TextEncoding* queryEncoding);
 
-  StringView componentStringView(const url::Component&) const;
   String componentString(const url::Component&) const;
-  StringView stringViewForInvalidComponent() const;
+  String stringForInvalidComponent() const;
 
   template <typename CHAR>
   void replaceComponents(const url::Replacements<CHAR>&);
 
   void initInnerURL();
-  void initProtocolMetadata();
+  void initProtocolIsInHTTPFamily();
 
   bool m_isValid;
   bool m_protocolIsInHTTPFamily;
-
-  // Keep a separate string for the protocol to avoid copious copies for
-  // protocol(). Normally this will be Atomic, except when constructed via
-  // KURL::copy(), which is deep.
-  String m_protocol;
-
   url::Parsed m_parsed;
   String m_string;
   std::unique_ptr<KURL> m_innerURL;

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

@@ -367,46 +367,46 @@ TEST(KURLTest, Valid_HTTP_FTP_URLsHaveHosts) {
   url::AddStandardScheme("http-so", url::SCHEME_WITH_PORT);
   url::AddStandardScheme("https-so", url::SCHEME_WITH_PORT);
 
-  KURL kurl(ParsedURLString, "foo://www.google.com/");
+  KURL kurl;
   EXPECT_TRUE(kurl.setProtocol("http"));
   EXPECT_TRUE(kurl.protocolIs("http"));
-  EXPECT_TRUE(kurl.protocolIsInHTTPFamily());
-  EXPECT_TRUE(kurl.isValid());
+  EXPECT_FALSE(kurl.isValid());
 
   EXPECT_TRUE(kurl.setProtocol("http-so"));
   EXPECT_TRUE(kurl.protocolIs("http-so"));
-  EXPECT_TRUE(kurl.isValid());
+  EXPECT_FALSE(kurl.isValid());
 
   EXPECT_TRUE(kurl.setProtocol("https"));
   EXPECT_TRUE(kurl.protocolIs("https"));
-  EXPECT_TRUE(kurl.isValid());
+  EXPECT_FALSE(kurl.isValid());
 
   EXPECT_TRUE(kurl.setProtocol("https-so"));
   EXPECT_TRUE(kurl.protocolIs("https-so"));
-  EXPECT_TRUE(kurl.isValid());
+  EXPECT_FALSE(kurl.isValid());
 
   EXPECT_TRUE(kurl.setProtocol("ftp"));
   EXPECT_TRUE(kurl.protocolIs("ftp"));
-  EXPECT_TRUE(kurl.isValid());
+  EXPECT_FALSE(kurl.isValid());
 
   kurl = KURL(KURL(), "http://");
-  EXPECT_FALSE(kurl.protocolIs("http"));
-
-  kurl = KURL(KURL(), "http://wide#鸡");
   EXPECT_TRUE(kurl.protocolIs("http"));
-  EXPECT_EQ(kurl.protocol(), "http");
+  EXPECT_FALSE(kurl.isValid());
 
-  kurl = KURL(KURL(), "http-so://foo");
+  kurl = KURL(KURL(), "http-so://");
   EXPECT_TRUE(kurl.protocolIs("http-so"));
+  EXPECT_FALSE(kurl.isValid());
 
-  kurl = KURL(KURL(), "https://foo");
+  kurl = KURL(KURL(), "https://");
   EXPECT_TRUE(kurl.protocolIs("https"));
+  EXPECT_FALSE(kurl.isValid());
 
-  kurl = KURL(KURL(), "https-so://foo");
+  kurl = KURL(KURL(), "https-so://");
   EXPECT_TRUE(kurl.protocolIs("https-so"));
+  EXPECT_FALSE(kurl.isValid());
 
-  kurl = KURL(KURL(), "ftp://foo");
+  kurl = KURL(KURL(), "ftp://");
   EXPECT_TRUE(kurl.protocolIs("ftp"));
+  EXPECT_FALSE(kurl.isValid());
 
   kurl = KURL(KURL(), "http://host/");
   EXPECT_TRUE(kurl.isValid());
@@ -699,6 +699,7 @@ TEST(KURLTest, ProtocolIs) {
 
   KURL invalidUTF8(ParsedURLString, "http://a@9%aa%:");
   EXPECT_FALSE(invalidUTF8.protocolIs("http"));
+  EXPECT_TRUE(invalidUTF8.protocolIs(""));
 
   KURL capital(KURL(), "HTTP://www.example.text");
   EXPECT_TRUE(capital.protocolIs("http"));

third_party/WebKit/Source/wtf/text/AtomicString.h

@@ -291,8 +291,6 @@ WTF_EXPORT extern const AtomicString& starAtom;
 WTF_EXPORT extern const AtomicString& xmlAtom;
 WTF_EXPORT extern const AtomicString& xmlnsAtom;
 WTF_EXPORT extern const AtomicString& xlinkAtom;
-WTF_EXPORT extern const AtomicString& httpAtom;
-WTF_EXPORT extern const AtomicString& httpsAtom;
 
 // AtomicStringHash is the default hash for AtomicString
 template <typename T>

third_party/WebKit/Source/wtf/text/StringStatics.cpp

@@ -54,8 +54,6 @@ WTF_EXPORT DEFINE_GLOBAL(AtomicString, starAtom);
 WTF_EXPORT DEFINE_GLOBAL(AtomicString, xmlAtom);
 WTF_EXPORT DEFINE_GLOBAL(AtomicString, xmlnsAtom);
 WTF_EXPORT DEFINE_GLOBAL(AtomicString, xlinkAtom);
-WTF_EXPORT DEFINE_GLOBAL(AtomicString, httpAtom);
-WTF_EXPORT DEFINE_GLOBAL(AtomicString, httpsAtom);
 
 // This is not an AtomicString because it is unlikely to be used as an
 // event/element/attribute name, so it shouldn't pollute the AtomicString hash
@@ -95,8 +93,6 @@ void StringStatics::init() {
   new (NotNull, (void*)&xmlnsAtom) AtomicString(addStaticASCIILiteral("xmlns"));
   new (NotNull, (void*)&xlinkAtom) AtomicString(addStaticASCIILiteral("xlink"));
   new (NotNull, (void*)&xmlnsWithColon) String("xmlns:");
-  new (NotNull, (void*)&httpAtom) AtomicString(addStaticASCIILiteral("http"));
-  new (NotNull, (void*)&httpsAtom) AtomicString(addStaticASCIILiteral("https"));
 }
 
 }  // namespace WTF