Title: [168183] trunk/Source/WebCore
Revision
168183
Author
a...@apple.com
Date
2014-05-02 12:43:57 -0700 (Fri, 02 May 2014)

Log Message

Don't abuse Blob deserialization constructor in WebSocket
https://bugs.webkit.org/show_bug.cgi?id=132478

Reviewed by Sam Weinig.

* Modules/websockets/WebSocketChannel.cpp:
(WebCore::WebSocketChannel::send):
(WebCore::WebSocketChannel::enqueueBlobFrame): This is the important change -
Blob::create was called for no reason. If the blob came from a worker, it was
already cloned for cross-thread messaging, otherwise there is no reason to make
a new one.

* fileapi/Blob.h:
(WebCore::Blob::deserialize):
(WebCore::Blob::create): Deleted.
* fileapi/File.h:
(WebCore::File::deserialize):
(WebCore::File::create): Deleted.
Renamed a special case of "create" function to avoid explaining what it is for.

* Modules/websockets/ThreadableWebSocketChannel.h:
* Modules/websockets/WebSocket.cpp:
(WebCore::WebSocket::send):
* Modules/websockets/WebSocketChannel.h:
* Modules/websockets/WorkerThreadableWebSocketChannel.cpp:
(WebCore::WorkerThreadableWebSocketChannel::send): Print a full URL in LOG(),
not one shortened to 1024 characters.
(WebCore::WorkerThreadableWebSocketChannel::Peer::send):
(WebCore::WorkerThreadableWebSocketChannel::mainThreadSendBlob):
(WebCore::WorkerThreadableWebSocketChannel::Bridge::send):
* Modules/websockets/WorkerThreadableWebSocketChannel.h:
* bindings/js/SerializedScriptValue.cpp:
(WebCore::CloneDeserializer::readFile):
(WebCore::CloneDeserializer::readTerminal):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (168182 => 168183)


--- trunk/Source/WebCore/ChangeLog	2014-05-02 19:42:57 UTC (rev 168182)
+++ trunk/Source/WebCore/ChangeLog	2014-05-02 19:43:57 UTC (rev 168183)
@@ -1,3 +1,40 @@
+2014-05-02  Alexey Proskuryakov  <a...@apple.com>
+
+        Don't abuse Blob deserialization constructor in WebSocket
+        https://bugs.webkit.org/show_bug.cgi?id=132478
+
+        Reviewed by Sam Weinig.
+
+        * Modules/websockets/WebSocketChannel.cpp:
+        (WebCore::WebSocketChannel::send):
+        (WebCore::WebSocketChannel::enqueueBlobFrame): This is the important change -
+        Blob::create was called for no reason. If the blob came from a worker, it was
+        already cloned for cross-thread messaging, otherwise there is no reason to make
+        a new one.
+
+        * fileapi/Blob.h:
+        (WebCore::Blob::deserialize):
+        (WebCore::Blob::create): Deleted.
+        * fileapi/File.h:
+        (WebCore::File::deserialize):
+        (WebCore::File::create): Deleted.
+        Renamed a special case of "create" function to avoid explaining what it is for.
+
+        * Modules/websockets/ThreadableWebSocketChannel.h:
+        * Modules/websockets/WebSocket.cpp:
+        (WebCore::WebSocket::send):
+        * Modules/websockets/WebSocketChannel.h:
+        * Modules/websockets/WorkerThreadableWebSocketChannel.cpp:
+        (WebCore::WorkerThreadableWebSocketChannel::send): Print a full URL in LOG(),
+        not one shortened to 1024 characters.
+        (WebCore::WorkerThreadableWebSocketChannel::Peer::send):
+        (WebCore::WorkerThreadableWebSocketChannel::mainThreadSendBlob):
+        (WebCore::WorkerThreadableWebSocketChannel::Bridge::send):
+        * Modules/websockets/WorkerThreadableWebSocketChannel.h:
+        * bindings/js/SerializedScriptValue.cpp:
+        (WebCore::CloneDeserializer::readFile):
+        (WebCore::CloneDeserializer::readTerminal):
+
 2014-05-02  Anders Carlsson  <ander...@apple.com>
 
         Add and implement KeyedDecoder::decodeBytes

Modified: trunk/Source/WebCore/Modules/websockets/ThreadableWebSocketChannel.h (168182 => 168183)


--- trunk/Source/WebCore/Modules/websockets/ThreadableWebSocketChannel.h	2014-05-02 19:42:57 UTC (rev 168182)
+++ trunk/Source/WebCore/Modules/websockets/ThreadableWebSocketChannel.h	2014-05-02 19:43:57 UTC (rev 168183)
@@ -66,7 +66,7 @@
     virtual String extensions() = 0; // Will be available after didConnect() callback is invoked.
     virtual SendResult send(const String& message) = 0;
     virtual SendResult send(const JSC::ArrayBuffer&, unsigned byteOffset, unsigned byteLength) = 0;
-    virtual SendResult send(const Blob&) = 0;
+    virtual SendResult send(Blob&) = 0;
     virtual unsigned long bufferedAmount() const = 0;
     virtual void close(int code, const String& reason) = 0;
     // Log the reason text and close the connection. Will call didClose().

Modified: trunk/Source/WebCore/Modules/websockets/WebSocket.cpp (168182 => 168183)


--- trunk/Source/WebCore/Modules/websockets/WebSocket.cpp	2014-05-02 19:42:57 UTC (rev 168182)
+++ trunk/Source/WebCore/Modules/websockets/WebSocket.cpp	2014-05-02 19:43:57 UTC (rev 168183)
@@ -350,7 +350,6 @@
 void WebSocket::send(Blob* binaryData, ExceptionCode& ec)
 {
     LOG(Network, "WebSocket %p send() Sending Blob '%s'", this, binaryData->url().stringCenterEllipsizedToLength().utf8().data());
-    ASSERT(binaryData);
     if (m_state == CONNECTING) {
         ec = INVALID_STATE_ERR;
         return;

Modified: trunk/Source/WebCore/Modules/websockets/WebSocketChannel.cpp (168182 => 168183)


--- trunk/Source/WebCore/Modules/websockets/WebSocketChannel.cpp	2014-05-02 19:42:57 UTC (rev 168182)
+++ trunk/Source/WebCore/Modules/websockets/WebSocketChannel.cpp	2014-05-02 19:43:57 UTC (rev 168183)
@@ -153,9 +153,9 @@
     return ThreadableWebSocketChannel::SendSuccess;
 }
 
-ThreadableWebSocketChannel::SendResult WebSocketChannel::send(const Blob& binaryData)
+ThreadableWebSocketChannel::SendResult WebSocketChannel::send(Blob& binaryData)
 {
-    LOG(Network, "WebSocketChannel %p send() Sending Blob '%s'", this, binaryData.url().stringCenterEllipsizedToLength().utf8().data());
+    LOG(Network, "WebSocketChannel %p send() Sending Blob '%s'", this, binaryData.url().string().utf8().data());
     enqueueBlobFrame(WebSocketFrame::OpCodeBinary, binaryData);
     processOutgoingFrameQueue();
     return ThreadableWebSocketChannel::SendSuccess;
@@ -695,13 +695,13 @@
     m_outgoingFrameQueue.append(frame.release());
 }
 
-void WebSocketChannel::enqueueBlobFrame(WebSocketFrame::OpCode opCode, const Blob& blob)
+void WebSocketChannel::enqueueBlobFrame(WebSocketFrame::OpCode opCode, Blob& blob)
 {
     ASSERT(m_outgoingFrameQueueStatus == OutgoingFrameQueueOpen);
     OwnPtr<QueuedFrame> frame = adoptPtr(new QueuedFrame);
     frame->opCode = opCode;
     frame->frameType = QueuedFrameTypeBlob;
-    frame->blobData = Blob::create(blob.url(), blob.type(), blob.size());
+    frame->blobData = &blob;
     m_outgoingFrameQueue.append(frame.release());
 }
 

Modified: trunk/Source/WebCore/Modules/websockets/WebSocketChannel.h (168182 => 168183)


--- trunk/Source/WebCore/Modules/websockets/WebSocketChannel.h	2014-05-02 19:42:57 UTC (rev 168182)
+++ trunk/Source/WebCore/Modules/websockets/WebSocketChannel.h	2014-05-02 19:43:57 UTC (rev 168183)
@@ -73,7 +73,7 @@
     virtual String extensions() override;
     virtual ThreadableWebSocketChannel::SendResult send(const String& message) override;
     virtual ThreadableWebSocketChannel::SendResult send(const JSC::ArrayBuffer&, unsigned byteOffset, unsigned byteLength) override;
-    virtual ThreadableWebSocketChannel::SendResult send(const Blob&) override;
+    virtual ThreadableWebSocketChannel::SendResult send(Blob&) override;
     virtual unsigned long bufferedAmount() const override;
     virtual void close(int code, const String& reason) override; // Start closing handshake.
     virtual void fail(const String& reason) override;
@@ -161,7 +161,7 @@
     };
     void enqueueTextFrame(const CString&);
     void enqueueRawFrame(WebSocketFrame::OpCode, const char* data, size_t dataLength);
-    void enqueueBlobFrame(WebSocketFrame::OpCode, const Blob&);
+    void enqueueBlobFrame(WebSocketFrame::OpCode, Blob&);
 
     void processOutgoingFrameQueue();
     void abortOutgoingFrameQueue();

Modified: trunk/Source/WebCore/Modules/websockets/WorkerThreadableWebSocketChannel.cpp (168182 => 168183)


--- trunk/Source/WebCore/Modules/websockets/WorkerThreadableWebSocketChannel.cpp	2014-05-02 19:42:57 UTC (rev 168182)
+++ trunk/Source/WebCore/Modules/websockets/WorkerThreadableWebSocketChannel.cpp	2014-05-02 19:43:57 UTC (rev 168183)
@@ -98,7 +98,7 @@
     return m_bridge->send(binaryData, byteOffset, byteLength);
 }
 
-ThreadableWebSocketChannel::SendResult WorkerThreadableWebSocketChannel::send(const Blob& binaryData)
+ThreadableWebSocketChannel::SendResult WorkerThreadableWebSocketChannel::send(Blob& binaryData)
 {
     if (!m_bridge)
         return ThreadableWebSocketChannel::SendFail;
@@ -192,7 +192,7 @@
     m_loaderProxy.postTaskForModeToWorkerGlobalScope(createCallbackTask(&workerGlobalScopeDidSend, m_workerClientWrapper, sendRequestResult), m_taskMode);
 }
 
-void WorkerThreadableWebSocketChannel::Peer::send(const Blob& binaryData)
+void WorkerThreadableWebSocketChannel::Peer::send(Blob& binaryData)
 {
     ASSERT(isMainThread());
     if (!m_mainWebSocketChannel || !m_workerClientWrapper)
@@ -438,7 +438,7 @@
     ASSERT_UNUSED(context, context->isDocument());
     ASSERT(peer);
 
-    RefPtr<Blob> blob = Blob::create(url, type, size);
+    RefPtr<Blob> blob = Blob::deserialize(url, type, size);
     peer->send(*blob);
 }
 
@@ -474,7 +474,7 @@
     return clientWrapper->sendRequestResult();
 }
 
-ThreadableWebSocketChannel::SendResult WorkerThreadableWebSocketChannel::Bridge::send(const Blob& binaryData)
+ThreadableWebSocketChannel::SendResult WorkerThreadableWebSocketChannel::Bridge::send(Blob& binaryData)
 {
     if (!m_workerClientWrapper || !m_peer)
         return ThreadableWebSocketChannel::SendFail;

Modified: trunk/Source/WebCore/Modules/websockets/WorkerThreadableWebSocketChannel.h (168182 => 168183)


--- trunk/Source/WebCore/Modules/websockets/WorkerThreadableWebSocketChannel.h	2014-05-02 19:42:57 UTC (rev 168182)
+++ trunk/Source/WebCore/Modules/websockets/WorkerThreadableWebSocketChannel.h	2014-05-02 19:43:57 UTC (rev 168183)
@@ -67,7 +67,7 @@
     virtual String extensions() override;
     virtual ThreadableWebSocketChannel::SendResult send(const String& message) override;
     virtual ThreadableWebSocketChannel::SendResult send(const JSC::ArrayBuffer&, unsigned byteOffset, unsigned byteLength) override;
-    virtual ThreadableWebSocketChannel::SendResult send(const Blob&) override;
+    virtual ThreadableWebSocketChannel::SendResult send(Blob&) override;
     virtual unsigned long bufferedAmount() const override;
     virtual void close(int code, const String& reason) override;
     virtual void fail(const String& reason) override;
@@ -89,7 +89,7 @@
         void connect(const URL&, const String& protocol);
         void send(const String& message);
         void send(const JSC::ArrayBuffer&);
-        void send(const Blob&);
+        void send(Blob&);
         void bufferedAmount();
         void close(int code, const String& reason);
         void fail(const String& reason);
@@ -135,7 +135,7 @@
         void connect(const URL&, const String& protocol);
         ThreadableWebSocketChannel::SendResult send(const String& message);
         ThreadableWebSocketChannel::SendResult send(const JSC::ArrayBuffer&, unsigned byteOffset, unsigned byteLength);
-        ThreadableWebSocketChannel::SendResult send(const Blob&);
+        ThreadableWebSocketChannel::SendResult send(Blob&);
         unsigned long bufferedAmount();
         void close(int code, const String& reason);
         void fail(const String& reason);

Modified: trunk/Source/WebCore/bindings/js/SerializedScriptValue.cpp (168182 => 168183)


--- trunk/Source/WebCore/bindings/js/SerializedScriptValue.cpp	2014-05-02 19:42:57 UTC (rev 168182)
+++ trunk/Source/WebCore/bindings/js/SerializedScriptValue.cpp	2014-05-02 19:43:57 UTC (rev 168183)
@@ -1732,7 +1732,7 @@
         if (!readStringData(type))
             return 0;
         if (m_isDOMGlobalObject)
-            file = File::create(path->string(), URL(URL(), url->string()), type->string());
+            file = File::deserialize(path->string(), URL(URL(), url->string()), type->string());
         return true;
     }
 
@@ -2231,7 +2231,7 @@
                 return JSValue();
             if (!m_isDOMGlobalObject)
                 return jsNull();
-            return getJSValue(Blob::create(URL(URL(), url->string()), type->string(), size).get());
+            return getJSValue(Blob::deserialize(URL(URL(), url->string()), type->string(), size).get());
         }
         case StringTag: {
             CachedStringRef cachedString;

Modified: trunk/Source/WebCore/fileapi/Blob.h (168182 => 168183)


--- trunk/Source/WebCore/fileapi/Blob.h	2014-05-02 19:42:57 UTC (rev 168182)
+++ trunk/Source/WebCore/fileapi/Blob.h	2014-05-02 19:43:57 UTC (rev 168183)
@@ -55,8 +55,7 @@
         return adoptRef(new Blob(std::move(blobData)));
     }
 
-    // For deserialization.
-    static PassRefPtr<Blob> create(const URL& srcURL, const String& type, long long size)
+    static PassRefPtr<Blob> deserialize(const URL& srcURL, const String& type, long long size)
     {
         ASSERT(Blob::isNormalizedContentType(type));
         return adoptRef(new Blob(srcURL, type, size));

Modified: trunk/Source/WebCore/fileapi/File.h (168182 => 168183)


--- trunk/Source/WebCore/fileapi/File.h	2014-05-02 19:42:57 UTC (rev 168182)
+++ trunk/Source/WebCore/fileapi/File.h	2014-05-02 19:43:57 UTC (rev 168183)
@@ -49,8 +49,7 @@
         return adoptRef(new File(path, policy));
     }
 
-    // For deserialization.
-    static PassRefPtr<File> create(const String& path, const URL& srcURL, const String& type)
+    static PassRefPtr<File> deserialize(const String& path, const URL& srcURL, const String& type)
     {
         return adoptRef(new File(path, srcURL, type));
     }
@@ -76,10 +75,10 @@
 
 private:
     File(const String& path, ContentTypeLookupPolicy);
+    File(const String& path, const String& name, ContentTypeLookupPolicy);
 
     // For deserialization.
     File(const String& path, const URL& srcURL, const String& type);
-    File(const String& path, const String& name, ContentTypeLookupPolicy);
 
     String m_path;
     String m_name;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to