Reduce size of WebCore::URL
authorachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 13 Jul 2018 00:28:36 +0000 (00:28 +0000)
committerachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 13 Jul 2018 00:28:36 +0000 (00:28 +0000)
https://bugs.webkit.org/show_bug.cgi?id=186820

Reviewed by Yusuke Suzuki and Youenn Fablet.

Source/WebCore:

We were using 32 bits for the length of the port, which is always between 0 and 5 inclusive
because port numbers are missing or between 0 and 65535.  Let's just use 3 bits here.
We were using 32 bits for the length of the scheme, which is usually 3-5 characters and can be
longer for some custom schemes, but I've never seen one more than 20 characters.  If we assume
schemes are always less than 64MB, we can save 8 bytes per URL!

No change in behavior, just less memory use!

To restore the IPC encoding to how it was before r221165, I just encode the string and reparse it.

* platform/URL.cpp:
(WebCore::URL::invalidate):
(WebCore::URL::lastPathComponent const):
(WebCore::URL::port const):
(WebCore::URL::protocolHostAndPort const):
(WebCore::URL::path const):
(WebCore::URL::removePort):
(WebCore::URL::setPort):
(WebCore::URL::setHostAndPort):
(WebCore::URL::setPath):
* platform/URL.h:
(WebCore::URL::encode const):
(WebCore::URL::decode):
(WebCore::URL::hasPath const):
(WebCore::URL::pathStart const):
* platform/URLParser.cpp:
(WebCore::URLParser::copyBaseWindowsDriveLetter):
(WebCore::URLParser::urlLengthUntilPart):
(WebCore::URLParser::copyURLPartsUntil):
(WebCore::URLParser::shouldPopPath):
(WebCore::URLParser::popPath):
(WebCore::URLParser::parse):
(WebCore::URLParser::parsePort):
(WebCore::URLParser::parseHostAndPort):
(WebCore::URLParser::allValuesEqual):
(WebCore::URLParser::internalValuesConsistent):
* workers/service/server/RegistrationDatabase.cpp:
Increment the service worker registration schema version because of the URL encoding change.

Source/WebKit:

* NetworkProcess/cache/NetworkCacheStorage.h:
Increment cache version because of URL encoding change.

Tools:

* TestWebKitAPI/Tests/WebCore/URLParser.cpp:
(TestWebKitAPI::TEST_F):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@233789 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/URL.cpp
Source/WebCore/platform/URL.h
Source/WebCore/platform/URLParser.cpp
Source/WebCore/workers/service/server/RegistrationDatabase.cpp
Source/WebKit/ChangeLog
Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.h
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp

index a401082..804d11f 100644 (file)
@@ -1,3 +1,49 @@
+2018-07-12  Alex Christensen  <achristensen@webkit.org>
+
+        Reduce size of WebCore::URL
+        https://bugs.webkit.org/show_bug.cgi?id=186820
+
+        Reviewed by Yusuke Suzuki and Youenn Fablet.
+
+        We were using 32 bits for the length of the port, which is always between 0 and 5 inclusive
+        because port numbers are missing or between 0 and 65535.  Let's just use 3 bits here.
+        We were using 32 bits for the length of the scheme, which is usually 3-5 characters and can be
+        longer for some custom schemes, but I've never seen one more than 20 characters.  If we assume
+        schemes are always less than 64MB, we can save 8 bytes per URL!
+
+        No change in behavior, just less memory use!
+        
+        To restore the IPC encoding to how it was before r221165, I just encode the string and reparse it.
+
+        * platform/URL.cpp:
+        (WebCore::URL::invalidate):
+        (WebCore::URL::lastPathComponent const):
+        (WebCore::URL::port const):
+        (WebCore::URL::protocolHostAndPort const):
+        (WebCore::URL::path const):
+        (WebCore::URL::removePort):
+        (WebCore::URL::setPort):
+        (WebCore::URL::setHostAndPort):
+        (WebCore::URL::setPath):
+        * platform/URL.h:
+        (WebCore::URL::encode const):
+        (WebCore::URL::decode):
+        (WebCore::URL::hasPath const):
+        (WebCore::URL::pathStart const):
+        * platform/URLParser.cpp:
+        (WebCore::URLParser::copyBaseWindowsDriveLetter):
+        (WebCore::URLParser::urlLengthUntilPart):
+        (WebCore::URLParser::copyURLPartsUntil):
+        (WebCore::URLParser::shouldPopPath):
+        (WebCore::URLParser::popPath):
+        (WebCore::URLParser::parse):
+        (WebCore::URLParser::parsePort):
+        (WebCore::URLParser::parseHostAndPort):
+        (WebCore::URLParser::allValuesEqual):
+        (WebCore::URLParser::internalValuesConsistent):
+        * workers/service/server/RegistrationDatabase.cpp:
+        Increment the service worker registration schema version because of the URL encoding change.
+
 2018-07-12  Youenn Fablet  <youenn@apple.com>
 
         Add a FrameLoaderClient willInjectUserScriptForFrame callback
index cb66916..f98f576 100644 (file)
@@ -84,7 +84,7 @@ void URL::invalidate()
     m_userEnd = 0;
     m_passwordEnd = 0;
     m_hostEnd = 0;
-    m_portEnd = 0;
+    m_portLength = 0;
     m_pathEnd = 0;
     m_pathAfterLastSlash = 0;
     m_queryEnd = 0;
@@ -144,7 +144,7 @@ String URL::lastPathComponent() const
         --end;
 
     size_t start = m_string.reverseFind('/', end);
-    if (start < static_cast<unsigned>(m_portEnd))
+    if (start < static_cast<unsigned>(m_hostEnd + m_portLength))
         return String();
     ++start;
 
@@ -164,15 +164,15 @@ StringView URL::host() const
 
 std::optional<uint16_t> URL::port() const
 {
-    if (!m_portEnd || m_hostEnd >= m_portEnd - 1)
+    if (!m_portLength)
         return std::nullopt;
 
     bool ok = false;
     unsigned number;
     if (m_string.is8Bit())
-        number = charactersToUIntStrict(m_string.characters8() + m_hostEnd + 1, m_portEnd - m_hostEnd - 1, &ok);
+        number = charactersToUIntStrict(m_string.characters8() + m_hostEnd + 1, m_portLength - 1, &ok);
     else
-        number = charactersToUIntStrict(m_string.characters16() + m_hostEnd + 1, m_portEnd - m_hostEnd - 1, &ok);
+        number = charactersToUIntStrict(m_string.characters16() + m_hostEnd + 1, m_portLength - 1, &ok);
     if (!ok || number > std::numeric_limits<uint16_t>::max())
         return std::nullopt;
     return number;
@@ -187,7 +187,7 @@ String URL::hostAndPort() const
 
 String URL::protocolHostAndPort() const
 {
-    String result = m_string.substring(0, m_portEnd);
+    String result = m_string.substring(0, m_hostEnd + m_portLength);
 
     if (m_passwordEnd - m_userStart > 0) {
         const int allowForTrailingAtSign = 1;
@@ -370,7 +370,8 @@ String URL::query() const
 
 String URL::path() const
 {
-    return m_string.substring(m_portEnd, m_pathEnd - m_portEnd);
+    unsigned portEnd = m_hostEnd + m_portLength;
+    return m_string.substring(portEnd, m_pathEnd - portEnd);
 }
 
 bool URL::setProtocol(const String& s)
@@ -455,9 +456,9 @@ void URL::setHost(const String& s)
 
 void URL::removePort()
 {
-    if (m_hostEnd == m_portEnd)
+    if (!m_portLength)
         return;
-    URLParser parser(m_string.left(m_hostEnd) + m_string.substring(m_portEnd));
+    URLParser parser(makeString(StringView(m_string).left(m_hostEnd), StringView(m_string).substring(m_hostEnd + m_portLength)));
     *this = parser.result();
 }
 
@@ -466,10 +467,10 @@ void URL::setPort(unsigned short i)
     if (!m_isValid)
         return;
 
-    bool colonNeeded = m_portEnd == m_hostEnd;
+    bool colonNeeded = !m_portLength;
     unsigned portStart = (colonNeeded ? m_hostEnd : m_hostEnd + 1);
 
-    URLParser parser(makeString(m_string.left(portStart), (colonNeeded ? ":" : ""), String::number(i), m_string.substring(m_portEnd)));
+    URLParser parser(makeString(StringView(m_string).left(portStart), (colonNeeded ? ":" : ""), String::number(i), StringView(m_string).substring(m_hostEnd + m_portLength)));
     *this = parser.result();
 }
 
@@ -509,7 +510,7 @@ void URL::setHostAndPort(const String& hostAndPort)
         builder.appendLiteral(":");
         builder.append(port);
     }
-    builder.append(m_string.substring(m_portEnd));
+    builder.append(StringView(m_string).substring(m_hostEnd + m_portLength));
 
     URLParser parser(builder.toString());
     *this = parser.result();
@@ -658,7 +659,7 @@ void URL::setPath(const String& s)
     auto questionMarkOrNumberSign = [] (UChar character) {
         return character == '?' || character == '#';
     };
-    URLParser parser(makeString(StringView(m_string).left(m_portEnd), percentEncodeCharacters(path, questionMarkOrNumberSign), StringView(m_string).substring(m_pathEnd)));
+    URLParser parser(makeString(StringView(m_string).left(m_hostEnd + m_portLength), percentEncodeCharacters(path, questionMarkOrNumberSign), StringView(m_string).substring(m_pathEnd)));
     *this = parser.result();
 }
 
index f587578..e96a41e 100644 (file)
@@ -219,38 +219,32 @@ private:
     void copyToBuffer(Vector<char, 512>& buffer) const;
 
     String m_string;
-    bool m_isValid : 1;
-    bool m_protocolIsInHTTPFamily : 1;
-    bool m_cannotBeABaseURL : 1;
 
-    unsigned m_schemeEnd;
+    unsigned m_isValid : 1;
+    unsigned m_protocolIsInHTTPFamily : 1;
+    unsigned m_cannotBeABaseURL : 1;
+
+    // This is out of order to align the bits better. The port is after the host.
+    unsigned m_portLength : 3;
+    static constexpr unsigned maxPortLength = (1 << 3) - 1;
+
+    static constexpr unsigned maxSchemeLength = (1 << 26) - 1;
+    unsigned m_schemeEnd : 26;
     unsigned m_userStart;
     unsigned m_userEnd;
     unsigned m_passwordEnd;
     unsigned m_hostEnd;
-    unsigned m_portEnd;
     unsigned m_pathAfterLastSlash;
     unsigned m_pathEnd;
     unsigned m_queryEnd;
 };
 
+static_assert(sizeof(URL) == sizeof(String) + 8 * sizeof(unsigned), "URL should stay small");
+
 template <class Encoder>
 void URL::encode(Encoder& encoder) const
 {
     encoder << m_string;
-    encoder << static_cast<bool>(m_isValid);
-    if (!m_isValid)
-        return;
-    encoder << static_cast<bool>(m_protocolIsInHTTPFamily);
-    encoder << m_schemeEnd;
-    encoder << m_userStart;
-    encoder << m_userEnd;
-    encoder << m_passwordEnd;
-    encoder << m_hostEnd;
-    encoder << m_portEnd;
-    encoder << m_pathAfterLastSlash;
-    encoder << m_pathEnd;
-    encoder << m_queryEnd;
 }
 
 template <class Decoder>
@@ -266,38 +260,10 @@ bool URL::decode(Decoder& decoder, URL& url)
 template <class Decoder>
 std::optional<URL> URL::decode(Decoder& decoder)
 {
-    URL url;
-    if (!decoder.decode(url.m_string))
-        return std::nullopt;
-    bool isValid;
-    if (!decoder.decode(isValid))
-        return std::nullopt;
-    url.m_isValid = isValid;
-    if (!isValid)
-        return WTFMove(url);
-    bool protocolIsInHTTPFamily;
-    if (!decoder.decode(protocolIsInHTTPFamily))
-        return std::nullopt;
-    url.m_protocolIsInHTTPFamily = protocolIsInHTTPFamily;
-    if (!decoder.decode(url.m_schemeEnd))
-        return std::nullopt;
-    if (!decoder.decode(url.m_userStart))
-        return std::nullopt;
-    if (!decoder.decode(url.m_userEnd))
-        return std::nullopt;
-    if (!decoder.decode(url.m_passwordEnd))
-        return std::nullopt;
-    if (!decoder.decode(url.m_hostEnd))
-        return std::nullopt;
-    if (!decoder.decode(url.m_portEnd))
-        return std::nullopt;
-    if (!decoder.decode(url.m_pathAfterLastSlash))
-        return std::nullopt;
-    if (!decoder.decode(url.m_pathEnd))
-        return std::nullopt;
-    if (!decoder.decode(url.m_queryEnd))
+    String string;
+    if (!decoder.decode(string))
         return std::nullopt;
-    return WTFMove(url);
+    return URL(URL(), string);
 }
 
 bool operator==(const URL&, const URL&);
@@ -400,7 +366,7 @@ inline bool URL::isValid() const
 
 inline bool URL::hasPath() const
 {
-    return m_pathEnd != m_portEnd;
+    return m_pathEnd != m_hostEnd + m_portLength;
 }
 
 inline bool URL::hasUsername() const
@@ -440,7 +406,7 @@ inline unsigned URL::hostEnd() const
 
 inline unsigned URL::pathStart() const
 {
-    return m_portEnd;
+    return m_hostEnd + m_portLength;
 }
 
 inline unsigned URL::pathEnd() const
index 44ad4ec..9830246 100644 (file)
@@ -502,17 +502,17 @@ void URLParser::appendWindowsDriveLetter(CodePointIterator<CharacterType>& itera
 bool URLParser::copyBaseWindowsDriveLetter(const URL& base)
 {
     if (base.protocolIs("file")) {
-        RELEASE_ASSERT(base.m_portEnd < base.m_string.length());
+        RELEASE_ASSERT(base.m_hostEnd + base.m_portLength < base.m_string.length());
         if (base.m_string.is8Bit()) {
             const LChar* begin = base.m_string.characters8();
-            CodePointIterator<LChar> c(begin + base.m_portEnd + 1, begin + base.m_string.length());
+            CodePointIterator<LChar> c(begin + base.m_hostEnd + base.m_portLength + 1, begin + base.m_string.length());
             if (isWindowsDriveLetter(c)) {
                 appendWindowsDriveLetter(c);
                 return true;
             }
         } else {
             const UChar* begin = base.m_string.characters16();
-            CodePointIterator<UChar> c(begin + base.m_portEnd + 1, begin + base.m_string.length());
+            CodePointIterator<UChar> c(begin + base.m_hostEnd + base.m_portLength + 1, begin + base.m_string.length());
             if (isWindowsDriveLetter(c)) {
                 appendWindowsDriveLetter(c);
                 return true;
@@ -846,7 +846,7 @@ size_t URLParser::urlLengthUntilPart(const URL& url, URLPart part)
     case URLPart::PathAfterLastSlash:
         return url.m_pathAfterLastSlash;
     case URLPart::PortEnd:
-        return url.m_portEnd;
+        return url.m_hostEnd + url.m_portLength;
     case URLPart::HostEnd:
         return url.m_hostEnd;
     case URLPart::PasswordEnd:
@@ -898,7 +898,7 @@ void URLParser::copyURLPartsUntil(const URL& base, URLPart part, const CodePoint
         m_url.m_pathAfterLastSlash = base.m_pathAfterLastSlash;
         FALLTHROUGH;
     case URLPart::PortEnd:
-        m_url.m_portEnd = base.m_portEnd;
+        m_url.m_portLength = base.m_portLength;
         FALLTHROUGH;
     case URLPart::HostEnd:
         m_url.m_hostEnd = base.m_hostEnd;
@@ -1043,7 +1043,7 @@ bool URLParser::shouldPopPath(unsigned newPathAfterLastSlash)
 
     ASSERT(m_url.m_pathAfterLastSlash <= m_asciiBuffer.size());
     CodePointIterator<LChar> componentToPop(&m_asciiBuffer[newPathAfterLastSlash], &m_asciiBuffer[0] + m_url.m_pathAfterLastSlash);
-    if (newPathAfterLastSlash == m_url.m_portEnd + 1 && isWindowsDriveLetter(componentToPop))
+    if (newPathAfterLastSlash == m_url.m_hostEnd + m_url.m_portLength + 1 && isWindowsDriveLetter(componentToPop))
         return false;
     return true;
 }
@@ -1051,11 +1051,11 @@ bool URLParser::shouldPopPath(unsigned newPathAfterLastSlash)
 void URLParser::popPath()
 {
     ASSERT(m_didSeeSyntaxViolation);
-    if (m_url.m_pathAfterLastSlash > m_url.m_portEnd + 1) {
+    if (m_url.m_pathAfterLastSlash > m_url.m_hostEnd + m_url.m_portLength + 1) {
         auto newPathAfterLastSlash = m_url.m_pathAfterLastSlash - 1;
         if (m_asciiBuffer[newPathAfterLastSlash] == '/')
             newPathAfterLastSlash--;
-        while (newPathAfterLastSlash > m_url.m_portEnd && m_asciiBuffer[newPathAfterLastSlash] != '/')
+        while (newPathAfterLastSlash > m_url.m_hostEnd + m_url.m_portLength && m_asciiBuffer[newPathAfterLastSlash] != '/')
             newPathAfterLastSlash--;
         newPathAfterLastSlash++;
         if (shouldPopPath(newPathAfterLastSlash))
@@ -1271,7 +1271,12 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
                     syntaxViolation(c);
                 appendToASCIIBuffer(toASCIILower(*c));
             } else if (*c == ':') {
-                m_url.m_schemeEnd = currentPosition(c);
+                unsigned schemeEnd = currentPosition(c);
+                if (schemeEnd > URL::maxSchemeLength) {
+                    failure();
+                    return;
+                }
+                m_url.m_schemeEnd = schemeEnd;
                 StringView urlScheme = parsedDataView(0, m_url.m_schemeEnd);
                 appendToASCIIBuffer(':');
                 switch (scheme(urlScheme)) {
@@ -1321,7 +1326,7 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
                         m_url.m_userEnd = m_url.m_userStart;
                         m_url.m_passwordEnd = m_url.m_userStart;
                         m_url.m_hostEnd = m_url.m_userStart;
-                        m_url.m_portEnd = m_url.m_userStart;
+                        m_url.m_portLength = 0;
                         m_url.m_pathAfterLastSlash = m_url.m_userStart;
                         m_url.m_cannotBeABaseURL = true;
                         state = State::CannotBeABaseURLPath;
@@ -1395,7 +1400,7 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
                 m_url.m_userEnd = m_url.m_userStart;
                 m_url.m_passwordEnd = m_url.m_userStart;
                 m_url.m_hostEnd = m_url.m_userStart;
-                m_url.m_portEnd = m_url.m_userStart;
+                m_url.m_portLength = 0;
                 m_url.m_pathAfterLastSlash = m_url.m_userStart + 1;
                 state = State::Path;
             }
@@ -1451,7 +1456,7 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
             } else {
                 copyURLPartsUntil(base, URLPart::PortEnd, c, isUTF8Encoding);
                 appendToASCIIBuffer('/');
-                m_url.m_pathAfterLastSlash = base.m_portEnd + 1;
+                m_url.m_pathAfterLastSlash = base.m_hostEnd + base.m_portLength + 1;
                 state = State::Path;
             }
             break;
@@ -1520,7 +1525,7 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
                         m_url.m_userEnd = currentPosition(c);
                         m_url.m_passwordEnd = m_url.m_userEnd;
                         m_url.m_hostEnd = m_url.m_userEnd;
-                        m_url.m_portEnd = m_url.m_userEnd;
+                        m_url.m_portLength = 0;
                         m_url.m_pathAfterLastSlash = m_url.m_userEnd;
                     } else {
                         m_url.m_userEnd = currentPosition(authorityOrHostBegin);
@@ -1590,7 +1595,7 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
                     m_url.m_userEnd = m_url.m_userStart;
                     m_url.m_passwordEnd = m_url.m_userStart;
                     m_url.m_hostEnd = m_url.m_userStart;
-                    m_url.m_portEnd = m_url.m_userStart;
+                    m_url.m_portLength = 0;
                     m_url.m_pathAfterLastSlash = m_url.m_userStart + 1;
                     m_url.m_pathEnd = m_url.m_pathAfterLastSlash;
                 }
@@ -1612,7 +1617,7 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
                     m_url.m_userEnd = m_url.m_userStart;
                     m_url.m_passwordEnd = m_url.m_userStart;
                     m_url.m_hostEnd = m_url.m_userStart;
-                    m_url.m_portEnd = m_url.m_userStart;
+                    m_url.m_portLength = 0;
                     m_url.m_pathAfterLastSlash = m_url.m_userStart + 1;
                     m_url.m_pathEnd = m_url.m_pathAfterLastSlash;
                     m_url.m_queryEnd = m_url.m_pathAfterLastSlash;
@@ -1630,7 +1635,7 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
                     m_url.m_userEnd = m_url.m_userStart;
                     m_url.m_passwordEnd = m_url.m_userStart;
                     m_url.m_hostEnd = m_url.m_userStart;
-                    m_url.m_portEnd = m_url.m_userStart;
+                    m_url.m_portLength = 0;
                     m_url.m_pathAfterLastSlash = m_url.m_userStart + 1;
                     if (isWindowsDriveLetter(c))
                         appendWindowsDriveLetter(c);
@@ -1650,7 +1655,7 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
                 m_url.m_userEnd = m_url.m_userStart;
                 m_url.m_passwordEnd = m_url.m_userStart;
                 m_url.m_hostEnd = m_url.m_userStart;
-                m_url.m_portEnd = m_url.m_userStart;
+                m_url.m_portLength = 0;
                 authorityOrHostBegin = c;
                 state = State::FileHost;
                 break;
@@ -1661,7 +1666,7 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
             m_url.m_userEnd = m_url.m_userStart;
             m_url.m_passwordEnd = m_url.m_userStart;
             m_url.m_hostEnd = m_url.m_userStart;
-            m_url.m_portEnd = m_url.m_userStart;
+            m_url.m_portLength = 0;
             if (isWindowsDriveLetter(c)) {
                 appendWindowsDriveLetter(c);
                 m_url.m_pathAfterLastSlash = m_url.m_userStart + 1;
@@ -1720,7 +1725,7 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
                         syntaxViolation(c);
                         m_asciiBuffer.shrink(m_url.m_passwordEnd);
                         m_url.m_hostEnd = currentPosition(c);
-                        m_url.m_portEnd = m_url.m_hostEnd;
+                        m_url.m_portLength = 0;
                     }
                     
                     state = State::PathStart;
@@ -1875,7 +1880,7 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
         m_url.m_userEnd = m_url.m_userStart;
         m_url.m_passwordEnd = m_url.m_userStart;
         m_url.m_hostEnd = m_url.m_userStart;
-        m_url.m_portEnd = m_url.m_userStart;
+        m_url.m_portLength = 0;
         m_url.m_pathAfterLastSlash = m_url.m_userStart + 1;
         m_url.m_pathEnd = m_url.m_pathAfterLastSlash;
         m_url.m_queryEnd = m_url.m_pathAfterLastSlash;
@@ -1887,7 +1892,7 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
         LOG_FINAL_STATE("RelativeSlash");
         copyURLPartsUntil(base, URLPart::PortEnd, c, isUTF8Encoding);
         appendToASCIIBuffer('/');
-        m_url.m_pathAfterLastSlash = base.m_portEnd + 1;
+        m_url.m_pathAfterLastSlash = m_url.m_hostEnd + m_url.m_portLength + 1;
         m_url.m_pathEnd = m_url.m_pathAfterLastSlash;
         m_url.m_queryEnd = m_url.m_pathAfterLastSlash;
         break;
@@ -1897,7 +1902,7 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
         m_url.m_userEnd = m_url.m_userStart;
         m_url.m_passwordEnd = m_url.m_userStart;
         m_url.m_hostEnd = m_url.m_userStart;
-        m_url.m_portEnd = m_url.m_userStart;
+        m_url.m_portLength = 0;
         m_url.m_pathAfterLastSlash = m_url.m_userStart;
         m_url.m_pathEnd = m_url.m_userStart;
         m_url.m_queryEnd = m_url.m_userStart;
@@ -1914,7 +1919,7 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
             m_url.m_userEnd = m_url.m_userStart;
             m_url.m_passwordEnd = m_url.m_userStart;
             m_url.m_hostEnd = m_url.m_userStart;
-            m_url.m_portEnd = m_url.m_userStart;
+            m_url.m_portLength = 0;
             m_url.m_pathEnd = m_url.m_userStart;
         } else if (!parseHostAndPort(authorityOrHostBegin)) {
             failure();
@@ -1923,9 +1928,9 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
             if (m_urlIsSpecial) {
                 syntaxViolation(c);
                 appendToASCIIBuffer('/');
-                m_url.m_pathEnd = m_url.m_portEnd + 1;
+                m_url.m_pathEnd = m_url.m_hostEnd + m_url.m_portLength + 1;
             } else
-                m_url.m_pathEnd = m_url.m_portEnd;
+                m_url.m_pathEnd = m_url.m_hostEnd + m_url.m_portLength;
         }
         m_url.m_pathAfterLastSlash = m_url.m_pathEnd;
         m_url.m_queryEnd = m_url.m_pathEnd;
@@ -1939,9 +1944,9 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
         if (m_urlIsSpecial) {
             syntaxViolation(c);
             appendToASCIIBuffer('/');
-            m_url.m_pathEnd = m_url.m_portEnd + 1;
+            m_url.m_pathEnd = m_url.m_hostEnd + m_url.m_portLength + 1;
         } else
-            m_url.m_pathEnd = m_url.m_portEnd;
+            m_url.m_pathEnd = m_url.m_hostEnd + m_url.m_portLength;
         m_url.m_pathAfterLastSlash = m_url.m_pathEnd;
         m_url.m_queryEnd = m_url.m_pathEnd;
         break;
@@ -1957,7 +1962,7 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
         m_url.m_userEnd = m_url.m_userStart;
         m_url.m_passwordEnd = m_url.m_userStart;
         m_url.m_hostEnd = m_url.m_userStart;
-        m_url.m_portEnd = m_url.m_userStart;
+        m_url.m_portLength = 0;
         m_url.m_pathAfterLastSlash = m_url.m_userStart + 1;
         m_url.m_pathEnd = m_url.m_pathAfterLastSlash;
         m_url.m_queryEnd = m_url.m_pathAfterLastSlash;
@@ -1970,7 +1975,7 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
         m_url.m_userEnd = m_url.m_userStart;
         m_url.m_passwordEnd = m_url.m_userStart;
         m_url.m_hostEnd = m_url.m_userStart;
-        m_url.m_portEnd = m_url.m_userStart;
+        m_url.m_portLength = 0;
         if (copyBaseWindowsDriveLetter(base)) {
             appendToASCIIBuffer('/');
             m_url.m_pathAfterLastSlash = m_url.m_userStart + 4;
@@ -1999,7 +2004,7 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
             m_url.m_userEnd = m_url.m_userStart;
             m_url.m_passwordEnd = m_url.m_userStart;
             m_url.m_hostEnd = m_url.m_userStart;
-            m_url.m_portEnd = m_url.m_userStart;
+            m_url.m_portLength = 0;
             m_url.m_pathAfterLastSlash = m_url.m_userStart + 1;
             m_url.m_pathEnd = m_url.m_pathAfterLastSlash;
             m_url.m_queryEnd = m_url.m_pathAfterLastSlash;
@@ -2015,10 +2020,10 @@ void URLParser::parse(const CharacterType* input, const unsigned length, const U
         if (isLocalhost(parsedDataView(m_url.m_passwordEnd, currentPosition(c) - m_url.m_passwordEnd))) {
             m_asciiBuffer.shrink(m_url.m_passwordEnd);
             m_url.m_hostEnd = currentPosition(c);
-            m_url.m_portEnd = m_url.m_hostEnd;
+            m_url.m_portLength = 0;
         }
         appendToASCIIBuffer('/');
-        m_url.m_pathAfterLastSlash = m_url.m_portEnd + 1;
+        m_url.m_pathAfterLastSlash = m_url.m_hostEnd + m_url.m_portLength + 1;
         m_url.m_pathEnd = m_url.m_pathAfterLastSlash;
         m_url.m_queryEnd = m_url.m_pathAfterLastSlash;
         break;
@@ -2601,7 +2606,9 @@ bool URLParser::parsePort(CodePointIterator<CharacterType>& iterator)
     advance(iterator, colonIterator);
     uint32_t port = 0;
     if (UNLIKELY(iterator.atEnd())) {
-        m_url.m_portEnd = currentPosition(colonIterator);
+        unsigned portLength = currentPosition(colonIterator) - m_url.m_hostEnd;
+        RELEASE_ASSERT(portLength <= URL::maxPortLength);
+        m_url.m_portLength = portLength;
         syntaxViolation(colonIterator);
         return true;
     }
@@ -2638,7 +2645,9 @@ bool URLParser::parsePort(CodePointIterator<CharacterType>& iterator)
         appendNumberToASCIIBuffer<uint16_t>(static_cast<uint16_t>(port));
     }
 
-    m_url.m_portEnd = currentPosition(iterator);
+    unsigned portLength = currentPosition(iterator) - m_url.m_hostEnd;
+    RELEASE_ASSERT(portLength <= URL::maxPortLength);
+    m_url.m_portLength = portLength;
     return true;
 }
 
@@ -2664,7 +2673,7 @@ bool URLParser::parseHostAndPort(CodePointIterator<CharacterType> iterator)
                     return parsePort(ipv6End);
                 }
                 m_url.m_hostEnd = currentPosition(ipv6End);
-                m_url.m_portEnd = m_url.m_hostEnd;
+                m_url.m_portLength = 0;
                 return true;
             }
             m_url.m_hostEnd = currentPosition(ipv6End);
@@ -2687,7 +2696,7 @@ bool URLParser::parseHostAndPort(CodePointIterator<CharacterType> iterator)
         }
         m_url.m_hostEnd = currentPosition(iterator);
         if (iterator.atEnd()) {
-            m_url.m_portEnd = currentPosition(iterator);
+            m_url.m_portLength = 0;
             return true;
         }
         return parsePort(iterator);
@@ -2708,7 +2717,7 @@ bool URLParser::parseHostAndPort(CodePointIterator<CharacterType> iterator)
             serializeIPv4(address.value());
             m_url.m_hostEnd = currentPosition(iterator);
             if (iterator.atEnd()) {
-                m_url.m_portEnd = currentPosition(iterator);
+                m_url.m_portLength = 0;
                 return true;
             }
             return parsePort(iterator);
@@ -2727,7 +2736,9 @@ bool URLParser::parseHostAndPort(CodePointIterator<CharacterType> iterator)
         m_url.m_hostEnd = currentPosition(iterator);
         if (!hostIterator.atEnd())
             return parsePort(hostIterator);
-        m_url.m_portEnd = currentPosition(iterator);
+        unsigned portLength = currentPosition(iterator) - m_url.m_hostEnd;
+        RELEASE_ASSERT(portLength <= URL::maxPortLength);
+        m_url.m_portLength = portLength;
         return true;
     }
     
@@ -2769,7 +2780,7 @@ bool URLParser::parseHostAndPort(CodePointIterator<CharacterType> iterator)
         serializeIPv4(address.value());
         m_url.m_hostEnd = currentPosition(iterator);
         if (iterator.atEnd()) {
-            m_url.m_portEnd = currentPosition(iterator);
+            m_url.m_portLength = 0;
             return true;
         }
         return parsePort(iterator);
@@ -2781,7 +2792,7 @@ bool URLParser::parseHostAndPort(CodePointIterator<CharacterType> iterator)
     m_url.m_hostEnd = currentPosition(iterator);
     if (!iterator.atEnd())
         return parsePort(iterator);
-    m_url.m_portEnd = currentPosition(iterator);
+    m_url.m_portLength = 0;
     return true;
 }
 
@@ -2868,29 +2879,29 @@ const UIDNA& URLParser::internationalDomainNameTranscoder()
 
 bool URLParser::allValuesEqual(const URL& a, const URL& b)
 {
-    // FIXME: m_cannotBeABaseURL is not compared because the old URL::parse did not use it,
-    // but once we get rid of URL::parse its value should be tested.
-    URL_PARSER_LOG("%d %d %d %d %d %d %d %d %d %d %d %s\n%d %d %d %d %d %d %d %d %d %d %d %s",
+    URL_PARSER_LOG("%d %d %d %d %d %d %d %d %d %d %d %d %s\n%d %d %d %d %d %d %d %d %d %d %d %d %s",
         a.m_isValid,
+        a.m_cannotBeABaseURL,
         a.m_protocolIsInHTTPFamily,
         a.m_schemeEnd,
         a.m_userStart,
         a.m_userEnd,
         a.m_passwordEnd,
         a.m_hostEnd,
-        a.m_portEnd,
+        a.m_hostEnd + a.m_portLength,
         a.m_pathAfterLastSlash,
         a.m_pathEnd,
         a.m_queryEnd,
         a.m_string.utf8().data(),
         b.m_isValid,
+        b.m_cannotBeABaseURL,
         b.m_protocolIsInHTTPFamily,
         b.m_schemeEnd,
         b.m_userStart,
         b.m_userEnd,
         b.m_passwordEnd,
         b.m_hostEnd,
-        b.m_portEnd,
+        b.m_hostEnd + b.m_portLength,
         b.m_pathAfterLastSlash,
         b.m_pathEnd,
         b.m_queryEnd,
@@ -2898,13 +2909,14 @@ bool URLParser::allValuesEqual(const URL& a, const URL& b)
 
     return a.m_string == b.m_string
         && a.m_isValid == b.m_isValid
+        && a.m_cannotBeABaseURL == b.m_cannotBeABaseURL
         && a.m_protocolIsInHTTPFamily == b.m_protocolIsInHTTPFamily
         && a.m_schemeEnd == b.m_schemeEnd
         && a.m_userStart == b.m_userStart
         && a.m_userEnd == b.m_userEnd
         && a.m_passwordEnd == b.m_passwordEnd
         && a.m_hostEnd == b.m_hostEnd
-        && a.m_portEnd == b.m_portEnd
+        && a.m_portLength == b.m_portLength
         && a.m_pathAfterLastSlash == b.m_pathAfterLastSlash
         && a.m_pathEnd == b.m_pathEnd
         && a.m_queryEnd == b.m_queryEnd;
@@ -2916,8 +2928,7 @@ bool URLParser::internalValuesConsistent(const URL& url)
         && url.m_userStart <= url.m_userEnd
         && url.m_userEnd <= url.m_passwordEnd
         && url.m_passwordEnd <= url.m_hostEnd
-        && url.m_hostEnd <= url.m_portEnd
-        && url.m_portEnd <= url.m_pathAfterLastSlash
+        && url.m_hostEnd + url.m_portLength <= url.m_pathAfterLastSlash
         && url.m_pathAfterLastSlash <= url.m_pathEnd
         && url.m_pathEnd <= url.m_queryEnd
         && url.m_queryEnd <= url.m_string.length();
index 265a963..7c096b8 100644 (file)
@@ -48,7 +48,7 @@
 
 namespace WebCore {
 
-static const uint64_t schemaVersion = 2;
+static const uint64_t schemaVersion = 3;
 
 static const String recordsTableSchema(const String& tableName)
 {
index 7c88f7d..3a8a40f 100644 (file)
@@ -1,3 +1,13 @@
+2018-07-12  Alex Christensen  <achristensen@webkit.org>
+
+        Reduce size of WebCore::URL
+        https://bugs.webkit.org/show_bug.cgi?id=186820
+
+        Reviewed by Yusuke Suzuki and Youenn Fablet.
+
+        * NetworkProcess/cache/NetworkCacheStorage.h:
+        Increment cache version because of URL encoding change.
+
 2018-07-12  Chris Dumez  <cdumez@apple.com>
 
         [Cocoa] Make sure NetworkProcess::createNetworkConnectionToWebProcess() returns a valid mach port
index 640fb53..be3adfc 100644 (file)
@@ -108,7 +108,7 @@ public:
     size_t approximateSize() const;
 
     // Incrementing this number will delete all existing cache content for everyone. Do you really need to do it?
-    static const unsigned version = 12;
+    static const unsigned version = 13;
 #if PLATFORM(MAC)
     /// Allow the last stable version of the cache to co-exist with the latest development one.
     static const unsigned lastStableVersion = 12;
index 0a5b650..5ffacc6 100644 (file)
@@ -1,3 +1,13 @@
+2018-07-12  Alex Christensen  <achristensen@webkit.org>
+
+        Reduce size of WebCore::URL
+        https://bugs.webkit.org/show_bug.cgi?id=186820
+
+        Reviewed by Yusuke Suzuki and Youenn Fablet.
+
+        * TestWebKitAPI/Tests/WebCore/URLParser.cpp:
+        (TestWebKitAPI::TEST_F):
+
 2018-07-12  Brady Eidson  <beidson@apple.com>
 
         Make process-swap-on-navigation an experimental feature.
index 0514dcf..aec4d73 100644 (file)
@@ -1179,6 +1179,9 @@ TEST_F(URLParserTest, DefaultPort)
     checkURLDifferences("file://:0/path",
         {"", "", "", "", 0, "", "", "", "file://:0/path"},
         {"file", "", "", "", 0, "/path", "", "", "file://:0/path"});
+    
+    checkURL("http://example.com:0000000000000077", {"http", "", "", "example.com", 77, "/", "", "", "http://example.com:77/"});
+    checkURL("http://example.com:0000000000000080", {"http", "", "", "example.com", 0, "/", "", "", "http://example.com/"});
 }
 
 TEST_F(URLParserTest, ParserFailures)