Serialize ResourceResponses using WebKit types
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 6 Sep 2014 15:33:06 +0000 (15:33 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 6 Sep 2014 15:33:06 +0000 (15:33 +0000)
https://bugs.webkit.org/show_bug.cgi?id=136545

Reviewed by Alexey Proskuryakov.

Source/WebCore:

Serialization is faster and we can mostly avoid having NSURLResponses in web process.

* WebCore.exp.in:
* platform/network/ResourceLoadTiming.h:
(WebCore::ResourceLoadTiming::encode):
(WebCore::ResourceLoadTiming::decode):
* platform/network/ResourceResponseBase.h:
(WebCore::ResourceResponseBase::encode):
(WebCore::ResourceResponseBase::decode):

    Serialize from the WebCore data instead of serializing NSURLResponse.

* platform/network/cf/ResourceResponseCFNet.cpp:
(WebCore::ResourceResponse::cfURLResponse):

    Synthesize CFURLResponse by creating NSURLResponse on Cocoa platforms so we don't need copy code.
    This has negligible performance impact, NSURLResponse is just a wrapper around CFURLResponse.

* platform/network/mac/ResourceResponseMac.mm:
(WebCore::ResourceResponse::nsURLResponse):
(WebCore::ResourceResponse::setCertificateChain):

    Avoid unnecessary NSURLRequest instantiation in debug builds.

Source/WebKit2:

Remove the WK2 serialization code for responses. It moves to the types itself in WebCore where it is
close to the data being serialized and less likely to get out of sync.

* Shared/WebCoreArgumentCoders.cpp:
(IPC::ArgumentCoder<ResourceResponse>::encode): Deleted.
(IPC::ArgumentCoder<ResourceResponse>::decode): Deleted.
* Shared/WebCoreArgumentCoders.h:
* Shared/mac/WebCoreArgumentCodersMac.mm:
(IPC::ArgumentCoder<ResourceResponse>::encodePlatformData): Deleted.
(IPC::ArgumentCoder<ResourceResponse>::decodePlatformData): Deleted.

LayoutTests:

Remove failure expectations for tests fixed by this patch.

http/tests/xmlhttprequest/web-apps/012.html
http/tests/xmlhttprequest/web-apps/013.html

* platform/mac-wk2/TestExpectations:

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

14 files changed:
LayoutTests/ChangeLog
LayoutTests/platform/mac-wk2/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/WebCore.exp.in
Source/WebCore/platform/network/ResourceLoadTiming.h
Source/WebCore/platform/network/ResourceResponseBase.h
Source/WebCore/platform/network/cf/ResourceResponseCFNet.cpp
Source/WebCore/platform/network/mac/ResourceResponseMac.mm
Source/WebCore/platform/network/soup/ResourceResponse.h
Source/WebKit2/ChangeLog
Source/WebKit2/Shared/WebCoreArgumentCoders.cpp
Source/WebKit2/Shared/WebCoreArgumentCoders.h
Source/WebKit2/Shared/mac/WebCoreArgumentCodersMac.mm
Source/WebKit2/Shared/soup/WebCoreArgumentCodersSoup.cpp

index ba5fffa..57f24c9 100644 (file)
@@ -1,3 +1,17 @@
+2014-09-06  Antti Koivisto  <antti@apple.com>
+
+        Serialize ResourceResponses using WebKit types
+        https://bugs.webkit.org/show_bug.cgi?id=136545
+
+        Reviewed by Alexey Proskuryakov.
+
+        Remove failure expectations for tests fixed by this patch.
+
+        http/tests/xmlhttprequest/web-apps/012.html
+        http/tests/xmlhttprequest/web-apps/013.html
+
+        * platform/mac-wk2/TestExpectations:
+
 2014-09-06  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r173335.
index de04e19..6698074 100644 (file)
@@ -639,10 +639,6 @@ http/tests/security/cross-origin-worker-indexeddb.html [ Skip ]
 # PPT: PingLoader is still in WebProcess. As a result, this test fails and also fails to delete its cookie, breaking many other cookie tests.
 [ Mavericks ] http/tests/navigation/ping-cookie.html [ Skip ]
 
-# <rdar://problem/13751647> PPT: Status text not passed over IPC
-[ Mavericks ] http/tests/xmlhttprequest/web-apps/012.html [ Failure ]
-[ Mavericks ] http/tests/xmlhttprequest/web-apps/013.html [ Failure ]
-
 # https://bugs.webkit.org/show_bug.cgi?id=133227
 plugins/snapshotting/set-plugin-size-to-tiny.html
 plugins/snapshotting/simple.html
index 9b0ea0a..ad0a0cd 100644 (file)
@@ -1,3 +1,34 @@
+2014-09-06  Antti Koivisto  <antti@apple.com>
+
+        Serialize ResourceResponses using WebKit types
+        https://bugs.webkit.org/show_bug.cgi?id=136545
+
+        Reviewed by Alexey Proskuryakov.
+
+        Serialization is faster and we can mostly avoid having NSURLResponses in web process.
+
+        * WebCore.exp.in:
+        * platform/network/ResourceLoadTiming.h:
+        (WebCore::ResourceLoadTiming::encode):
+        (WebCore::ResourceLoadTiming::decode):
+        * platform/network/ResourceResponseBase.h:
+        (WebCore::ResourceResponseBase::encode):
+        (WebCore::ResourceResponseBase::decode):
+
+            Serialize from the WebCore data instead of serializing NSURLResponse.
+
+        * platform/network/cf/ResourceResponseCFNet.cpp:
+        (WebCore::ResourceResponse::cfURLResponse):
+
+            Synthesize CFURLResponse by creating NSURLResponse on Cocoa platforms so we don't need copy code.
+            This has negligible performance impact, NSURLResponse is just a wrapper around CFURLResponse.
+
+        * platform/network/mac/ResourceResponseMac.mm:
+        (WebCore::ResourceResponse::nsURLResponse):
+        (WebCore::ResourceResponse::setCertificateChain):
+
+            Avoid unnecessary NSURLRequest instantiation in debug builds.
+
 2014-09-06  David Kilzer  <ddkilzer@apple.com>
 
         HTMLElement.cpp does not compile with new clang
index 0f3f0a3..a991117 100644 (file)
@@ -1837,6 +1837,7 @@ __ZNK7WebCore20ResourceResponseBase17suggestedFilenameEv
 __ZNK7WebCore20ResourceResponseBase21expectedContentLengthEv
 __ZNK7WebCore20ResourceResponseBase3urlEv
 __ZNK7WebCore20ResourceResponseBase6isHTTPEv
+__ZNK7WebCore20ResourceResponseBase8lazyInitENS0_9InitLevelE
 __ZNK7WebCore20ResourceResponseBase8mimeTypeEv
 __ZNK7WebCore20TransformationMatrix7inverseEv
 __ZNK7WebCore20TransformationMatrix7mapRectERKNS_7IntRectE
index 0322252..d717bd1 100644 (file)
@@ -80,7 +80,10 @@ public:
     {
         return !(*this == other);
     }
-    
+
+    template<class Encoder> void encode(Encoder&) const;
+    template<class Decoder> static bool decode(Decoder&, ResourceLoadTiming&);
+
     // These are millisecond deltas from the navigation start.
     int domainLookupStart;
     int domainLookupEnd;
@@ -91,6 +94,30 @@ public:
     int secureConnectionStart;
 };
 
+template<class Encoder>
+void ResourceLoadTiming::encode(Encoder& encoder) const
+{
+    encoder << domainLookupStart;
+    encoder << domainLookupEnd;
+    encoder << connectStart;
+    encoder << connectEnd;
+    encoder << requestStart;
+    encoder << responseStart;
+    encoder << secureConnectionStart;
+}
+
+template<class Decoder>
+bool ResourceLoadTiming::decode(Decoder& decoder, ResourceLoadTiming& timing)
+{
+    return decoder.decode(timing.domainLookupStart)
+        && decoder.decode(timing.domainLookupEnd)
+        && decoder.decode(timing.connectStart)
+        && decoder.decode(timing.connectEnd)
+        && decoder.decode(timing.requestStart)
+        && decoder.decode(timing.responseStart)
+        && decoder.decode(timing.secureConnectionStart);
+}
+
 }
 
 #endif
index 3aa4591..72a243e 100644 (file)
@@ -124,6 +124,9 @@ public:
 
     static bool compare(const ResourceResponse&, const ResourceResponse&);
 
+    template<class Encoder> void encode(Encoder&) const;
+    template<class Decoder> static bool decode(Decoder&, ResourceResponseBase&);
+
 protected:
     enum InitLevel {
         Uninitialized,
@@ -186,6 +189,62 @@ private:
 inline bool operator==(const ResourceResponse& a, const ResourceResponse& b) { return ResourceResponseBase::compare(a, b); }
 inline bool operator!=(const ResourceResponse& a, const ResourceResponse& b) { return !(a == b); }
 
+template<class Encoder>
+void ResourceResponseBase::encode(Encoder& encoder) const
+{
+    encoder << m_isNull;
+    if (m_isNull)
+        return;
+    lazyInit(AllFields);
+
+    encoder << m_url.string();
+    encoder << m_mimeType;
+    encoder << static_cast<int64_t>(m_expectedContentLength);
+    encoder << m_textEncodingName;
+    encoder << m_httpStatusText;
+    encoder << m_httpHeaderFields;
+    encoder << m_resourceLoadTiming;
+    encoder << m_httpStatusCode;
+    encoder << m_connectionID;
+}
+
+template<class Decoder>
+bool ResourceResponseBase::decode(Decoder& decoder, ResourceResponseBase& response)
+{
+    ASSERT(response.m_isNull);
+    bool responseIsNull;
+    if (!decoder.decode(responseIsNull))
+        return false;
+    if (responseIsNull)
+        return true;
+
+    String url;
+    if (!decoder.decode(url))
+        return false;
+    response.m_url = URL(URL(), url);
+    if (!decoder.decode(response.m_mimeType))
+        return false;
+    int64_t expectedContentLength;
+    if (!decoder.decode(expectedContentLength))
+        return false;
+    response.m_expectedContentLength = expectedContentLength;
+    if (!decoder.decode(response.m_textEncodingName))
+        return false;
+    if (!decoder.decode(response.m_httpStatusText))
+        return false;
+    if (!decoder.decode(response.m_httpHeaderFields))
+        return false;
+    if (!decoder.decode(response.m_resourceLoadTiming))
+        return false;
+    if (!decoder.decode(response.m_httpStatusCode))
+        return false;
+    if (!decoder.decode(response.m_connectionID))
+        return false;
+    response.m_isNull = false;
+
+    return true;
+}
+
 struct CrossThreadResourceResponseDataBase {
     WTF_MAKE_NONCOPYABLE(CrossThreadResourceResponseDataBase); WTF_MAKE_FAST_ALLOCATED;
 public:
index c18fd2a..3148735 100644 (file)
@@ -48,11 +48,13 @@ static const int numCommonHeaderFields = sizeof(commonHeaderFields) / sizeof(CFS
 CFURLResponseRef ResourceResponse::cfURLResponse() const
 {
     if (!m_cfResponse && !m_isNull) {
+#if PLATFORM(COCOA)
+        nsURLResponse();
+#else
         RetainPtr<CFURLRef> url = m_url.createCFURL();
-
         // FIXME: This creates a very incomplete CFURLResponse, which does not even have a status code.
-
         m_cfResponse = adoptCF(CFURLResponseCreate(0, url.get(), m_mimeType.string().createCFString().get(), m_expectedContentLength, m_textEncodingName.string().createCFString().get(), kCFURLCacheStorageAllowed));
+#endif
     }
 
     return m_cfResponse.get();
index 8fbe016..1adf2b7 100644 (file)
@@ -58,6 +58,8 @@ void ResourceResponse::initNSURLResponse() const
         m_nsResponse = adoptNS([[NSURLResponse alloc] initWithURL:m_url MIMEType:m_mimeType expectedContentLength:expectedContentLength textEncodingName:m_textEncodingName]);
         return;
     }
+
+    // FIXME: We lose the status text and the HTTP version here.
     NSMutableDictionary* headerDictionary = [NSMutableDictionary dictionary];
     for (auto& header : m_httpHeaderFields)
         [headerDictionary setObject:(NSString *)header.value forKey:(NSString *)header.key];
@@ -185,7 +187,7 @@ bool ResourceResponse::platformCompare(const ResourceResponse& a, const Resource
 
 void ResourceResponse::setCertificateChain(CFArrayRef certificateChain)
 {
-    ASSERT(!wkCopyNSURLResponseCertificateChain(nsURLResponse()));
+    ASSERT(!m_nsResponse || !wkCopyNSURLResponseCertificateChain(m_nsResponse.get()));
     m_externalCertificateChain = certificateChain;
 }
 
index 7e830ed..a8e58f6 100644 (file)
@@ -76,6 +76,9 @@ public:
 
     bool platformResponseIsUpToDate() const { return false; }
 
+    template<class Encoder> void encode(Encoder&) const;
+    template<class Decoder> static bool decode(Decoder&, ResourceResponse&);
+
 private:
     friend class ResourceResponseBase;
 
@@ -91,6 +94,23 @@ private:
     void doPlatformAdopt(PassOwnPtr<CrossThreadResourceResponseData>) { }
 };
 
+template<class Encoder>
+void ResourceResponse::encode(Encoder& encoder) const
+{
+    ResourceResponseBase::encode(encoder);
+    encoder.encodeEnum(m_soupFlags);
+}
+
+template<class Decoder>
+bool ResourceResponse::decode(Decoder& decoder, ResourceResponse& response)
+{
+    if (!ResourceResponseBase::decode(decoder, response))
+        return false;
+    if (!decoder.decodeEnum(response.m_soupFlags))
+        return false;
+    return true;
+}
+
 struct CrossThreadResourceResponseData : public CrossThreadResourceResponseDataBase {
 };
 
index 8c379ed..676c4d3 100644 (file)
@@ -1,3 +1,21 @@
+2014-09-06  Antti Koivisto  <antti@apple.com>
+
+        Serialize ResourceResponses using WebKit types
+        https://bugs.webkit.org/show_bug.cgi?id=136545
+
+        Reviewed by Alexey Proskuryakov.
+
+        Remove the WK2 serialization code for responses. It moves to the types itself in WebCore where it is
+        close to the data being serialized and less likely to get out of sync.
+
+        * Shared/WebCoreArgumentCoders.cpp:
+        (IPC::ArgumentCoder<ResourceResponse>::encode): Deleted.
+        (IPC::ArgumentCoder<ResourceResponse>::decode): Deleted.
+        * Shared/WebCoreArgumentCoders.h:
+        * Shared/mac/WebCoreArgumentCodersMac.mm:
+        (IPC::ArgumentCoder<ResourceResponse>::encodePlatformData): Deleted.
+        (IPC::ArgumentCoder<ResourceResponse>::decodePlatformData): Deleted.
+
 2014-09-06  Ryuan Choi  <ryuan.choi@gmail.com>
 
         [EFL] Drop evas object cursor support
index 816671e..a56fc3f 100644 (file)
@@ -674,123 +674,6 @@ bool ArgumentCoder<ResourceRequest>::decode(ArgumentDecoder& decoder, ResourceRe
     return resourceRequest.decodeWithoutPlatformData(decoder);
 }
 
-void ArgumentCoder<ResourceResponse>::encode(ArgumentEncoder& encoder, const ResourceResponse& resourceResponse)
-{
-#if PLATFORM(COCOA)
-    bool shouldSerializeWebCoreData = !resourceResponse.platformResponseIsUpToDate();
-    encoder << shouldSerializeWebCoreData;
-#else
-    bool shouldSerializeWebCoreData = true;
-#endif
-
-    encodePlatformData(encoder, resourceResponse);
-
-    if (shouldSerializeWebCoreData) {
-        bool responseIsNull = resourceResponse.isNull();
-        encoder << responseIsNull;
-        if (responseIsNull)
-            return;
-
-        encoder << resourceResponse.url().string();
-        encoder << static_cast<int32_t>(resourceResponse.httpStatusCode());
-        encoder << resourceResponse.httpHeaderFields();
-
-        encoder << resourceResponse.mimeType();
-        encoder << resourceResponse.textEncodingName();
-        encoder << static_cast<int64_t>(resourceResponse.expectedContentLength());
-        encoder << resourceResponse.httpStatusText();
-    }
-    
-#if ENABLE(WEB_TIMING)
-    const ResourceLoadTiming& timing = resourceResponse.resourceLoadTiming();
-    encoder << timing.domainLookupStart;
-    encoder << timing.domainLookupEnd;
-    encoder << timing.connectStart;
-    encoder << timing.connectEnd;
-    encoder << timing.requestStart;
-    encoder << timing.responseStart;
-    encoder << timing.secureConnectionStart;
-#endif
-}
-
-bool ArgumentCoder<ResourceResponse>::decode(ArgumentDecoder& decoder, ResourceResponse& resourceResponse)
-{
-#if PLATFORM(COCOA)
-    bool hasSerializedWebCoreData;
-    if (!decoder.decode(hasSerializedWebCoreData))
-        return false;
-#else
-    bool hasSerializedWebCoreData = true;
-#endif
-
-    ResourceResponse response;
-
-    if (!decodePlatformData(decoder, response))
-        return false;
-
-    if (hasSerializedWebCoreData) {
-        bool responseIsNull;
-        if (!decoder.decode(responseIsNull))
-            return false;
-        if (responseIsNull) {
-            resourceResponse = ResourceResponse();
-            return true;
-        }
-
-        String url;
-        if (!decoder.decode(url))
-            return false;
-        response.setURL(URL(URL(), url));
-
-        int32_t httpStatusCode;
-        if (!decoder.decode(httpStatusCode))
-            return false;
-        response.setHTTPStatusCode(httpStatusCode);
-
-        HTTPHeaderMap headers;
-        if (!decoder.decode(headers))
-            return false;
-        for (HTTPHeaderMap::const_iterator it = headers.begin(), end = headers.end(); it != end; ++it)
-            response.setHTTPHeaderField(it->key, it->value);
-
-        String mimeType;
-        if (!decoder.decode(mimeType))
-            return false;
-        response.setMimeType(mimeType);
-
-        String textEncodingName;
-        if (!decoder.decode(textEncodingName))
-            return false;
-        response.setTextEncodingName(textEncodingName);
-
-        int64_t contentLength;
-        if (!decoder.decode(contentLength))
-            return false;
-        response.setExpectedContentLength(contentLength);
-
-        String httpStatusText;
-        if (!decoder.decode(httpStatusText))
-            return false;
-        response.setHTTPStatusText(httpStatusText);
-    }
-    
-#if ENABLE(WEB_TIMING)
-    ResourceLoadTiming& timing = response.resourceLoadTiming();
-    if (!decoder.decode(timing.domainLookupStart)
-        || !decoder.decode(timing.domainLookupEnd)
-        || !decoder.decode(timing.connectStart)
-        || !decoder.decode(timing.connectEnd)
-        || !decoder.decode(timing.requestStart)
-        || !decoder.decode(timing.responseStart)
-        || !decoder.decode(timing.secureConnectionStart))
-        return false;
-#endif
-
-    resourceResponse = response;
-
-    return true;
-}
-
 void ArgumentCoder<ResourceError>::encode(ArgumentEncoder& encoder, const ResourceError& resourceError)
 {
     encodePlatformData(encoder, resourceError);
index 9c76ac6..3c12887 100644 (file)
@@ -251,13 +251,6 @@ template<> struct ArgumentCoder<WebCore::ResourceRequest> {
     static bool decodePlatformData(ArgumentDecoder&, WebCore::ResourceRequest&);
 };
 
-template<> struct ArgumentCoder<WebCore::ResourceResponse> {
-    static void encode(ArgumentEncoder&, const WebCore::ResourceResponse&);
-    static bool decode(ArgumentDecoder&, WebCore::ResourceResponse&);
-    static void encodePlatformData(ArgumentEncoder&, const WebCore::ResourceResponse&);
-    static bool decodePlatformData(ArgumentDecoder&, WebCore::ResourceResponse&);
-};
-
 template<> struct ArgumentCoder<WebCore::ResourceError> {
     static void encode(ArgumentEncoder&, const WebCore::ResourceError&);
     static bool decode(ArgumentDecoder&, WebCore::ResourceError&);
index ec1d318..f4add85 100644 (file)
@@ -143,42 +143,6 @@ bool ArgumentCoder<ResourceRequest>::decodePlatformData(ArgumentDecoder& decoder
     return true;
 }
 
-void ArgumentCoder<ResourceResponse>::encodePlatformData(ArgumentEncoder& encoder, const ResourceResponse& resourceResponse)
-{
-    bool responseIsPresent = resourceResponse.platformResponseIsUpToDate() && resourceResponse.nsURLResponse();
-    encoder << responseIsPresent;
-
-    if (!responseIsPresent)
-        return;
-
-    RetainPtr<CFDictionaryRef> dictionary = adoptCF(WKNSURLResponseCreateSerializableRepresentation(resourceResponse.nsURLResponse(), IPC::tokenNullTypeRef()));
-    IPC::encode(encoder, dictionary.get());
-}
-
-bool ArgumentCoder<ResourceResponse>::decodePlatformData(ArgumentDecoder& decoder, ResourceResponse& resourceResponse)
-{
-    bool responseIsPresent;
-    if (!decoder.decode(responseIsPresent))
-        return false;
-
-    if (!responseIsPresent) {
-        resourceResponse = ResourceResponse();
-        return true;
-    }
-
-    RetainPtr<CFDictionaryRef> dictionary;
-    if (!IPC::decode(decoder, dictionary))
-        return false;
-
-    RetainPtr<NSURLResponse> nsURLResponse = WKNSURLResponseFromSerializableRepresentation(dictionary.get(), IPC::tokenNullTypeRef());
-
-    if (!nsURLResponse)
-        return false;
-
-    resourceResponse = ResourceResponse(nsURLResponse.get());
-    return true;
-}
-
 void ArgumentCoder<CertificateInfo>::encode(ArgumentEncoder& encoder, const CertificateInfo& certificateInfo)
 {
     CFArrayRef certificateChain = certificateInfo.certificateChain();
index 5ae8032..3d0515b 100644 (file)
@@ -104,20 +104,6 @@ bool ArgumentCoder<ResourceRequest>::decodePlatformData(ArgumentDecoder& decoder
     return true;
 }
 
-void ArgumentCoder<ResourceResponse>::encodePlatformData(ArgumentEncoder& encoder, const ResourceResponse& resourceResponse)
-{
-    encoder << static_cast<uint32_t>(resourceResponse.soupMessageFlags());
-}
-
-bool ArgumentCoder<ResourceResponse>::decodePlatformData(ArgumentDecoder& decoder, ResourceResponse& resourceResponse)
-{
-    uint32_t soupMessageFlags;
-    if (!decoder.decode(soupMessageFlags))
-        return false;
-    resourceResponse.setSoupMessageFlags(static_cast<SoupMessageFlags>(soupMessageFlags));
-    return true;
-}
-
 void ArgumentCoder<CertificateInfo>::encode(ArgumentEncoder& encoder, const CertificateInfo& certificateInfo)
 {
     if (!certificateInfo.certificate()) {