CachedRawResource is not handling incremental data computation correctly

https://bugs.webkit.org/show_bug.cgi?id=184936

Reviewed by Chris Dumez.

Source/WebCore:

Covered by updated test.

The previous logic was handling the case of only one additional segment being added to the SharedBuffer.
In service worker case, a SharedBuffer may contain more than one segment.
This is fixed by iterating until all new data is sent to clients.

* loader/cache/CachedRawResource.cpp:
(WebCore::CachedRawResource::updateBuffer):

LayoutTests:

* http/tests/workers/service/resources/service-worker-fetch-worker.js:
(stringToBuffer):
(event.event.request.url.endsWith):
* http/tests/workers/service/resources/service-worker-fetch.js:
(async.test):
* http/tests/workers/service/service-worker-fetch.https-expected.txt:


Canonical link: https://commits.webkit.org/200477@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@231006 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog

@@ -1,3 +1,17 @@
+2018-04-25  Youenn Fablet  <youenn@apple.com>
+
+        CachedRawResource is not handling incremental data computation correctly
+        https://bugs.webkit.org/show_bug.cgi?id=184936
+
+        Reviewed by Chris Dumez.
+
+        * http/tests/workers/service/resources/service-worker-fetch-worker.js:
+        (stringToBuffer):
+        (event.event.request.url.endsWith):
+        * http/tests/workers/service/resources/service-worker-fetch.js:
+        (async.test):
+        * http/tests/workers/service/service-worker-fetch.https-expected.txt:
+
 2018-04-25  Youenn Fablet  <youenn@apple.com>
 
         Ensure DNT is set for redirections handled in NetworkProcess

LayoutTests/http/tests/workers/service/resources/service-worker-fetch-worker.js

@@ -1,5 +1,13 @@
 var status = "no status";
 
+function stringToBuffer(value) {
+    var result = new ArrayBuffer(value.length*2);
+    var view = new Uint16Array(result);
+    for (var i = 0; i < value.length; ++i)
+        view[i] = value.charCodeAt(i);
+    return result;
+}
+
 self.addEventListener("fetch", (event) => {
     if (event.request.url.indexOf("status") !== -1) {
         event.respondWith(new Response(null, {status: 200, statusText: status}));
@@ -16,6 +24,22 @@ self.addEventListener("fetch", (event) => {
         event.respondWith(new Response(stream, {status : 200, statusText : "Empty stream"}));
         return;
     }
+    if (event.request.url.endsWith(".bodyasstream")) {
+        var stream = new ReadableStream({ start : async controller => {
+            await controller.enqueue(stringToBuffer("This "));
+            await controller.enqueue(stringToBuffer("test "));
+            await controller.enqueue(stringToBuffer("passes "));
+            await controller.enqueue(stringToBuffer("if "));
+            await controller.enqueue(stringToBuffer("the sentence "));
+            await controller.enqueue(stringToBuffer("is "));
+            await controller.enqueue(stringToBuffer("complete "));
+            await controller.enqueue(stringToBuffer("with "));
+            await controller.enqueue(stringToBuffer("PASS."));
+            controller.close();
+        }});
+        event.respondWith(new Response(stream, {status : 200, statusText : "Empty stream"}));
+        return;
+    }
     state = "unknown url";
     event.respondWith(new Response(null, {status: 404, statusText: "Not Found"}));
     return;

LayoutTests/http/tests/workers/service/resources/service-worker-fetch.js

@@ -1,9 +1,12 @@
 async function test()
 {
     try {
-        var frame = await interceptedFrame("resources/service-worker-fetch-worker.js", "/workers/service/resources/");
+        var frame = await interceptedFrame("resources/service-worker-fetch-worker.js", "/workers/service/resources/test.bodyasstream");
         var fetch = frame.contentWindow.fetch;
 
+        var expectedFrameContent = "This test passes if the sentence is complete with PASS.";
+        log("Checking frame content received as chunks: " + (frame.contentWindow.document.body.innerHTML === expectedFrameContent ? "PASS" : "FAIL"));
+
         var response = await fetch("status");
         log("Status is " + response.statusText);
 

LayoutTests/http/tests/workers/service/service-worker-fetch.https-expected.txt

@@ -1,4 +1,5 @@
 
+Checking frame content received as chunks: PASS
 Status is no status
 Got image response with buffer byte length being 12940
 Got should-be-empty response with buffer byte length being 0 and status is Empty stream

Source/WebCore/ChangeLog

@@ -1,3 +1,19 @@
+2018-04-25  Youenn Fablet  <youenn@apple.com>
+
+        CachedRawResource is not handling incremental data computation correctly
+        https://bugs.webkit.org/show_bug.cgi?id=184936
+
+        Reviewed by Chris Dumez.
+
+        Covered by updated test.
+
+        The previous logic was handling the case of only one additional segment being added to the SharedBuffer.
+        In service worker case, a SharedBuffer may contain more than one segment.
+        This is fixed by iterating until all new data is sent to clients.
+
+        * loader/cache/CachedRawResource.cpp:
+        (WebCore::CachedRawResource::updateBuffer):
+
 2018-04-25  Andy Estes  <aestes@apple.com>
 
         [Mac] Number of drop items is always 0 when performing a DHTML drag

Source/WebCore/loader/cache/CachedRawResource.cpp

@@ -64,12 +64,15 @@ void CachedRawResource::updateBuffer(SharedBuffer& data)
     ASSERT(dataBufferingPolicy() == BufferData);
     m_data = &data;
 
-    auto incrementalData = calculateIncrementalDataChunk(&data);
-    setEncodedSize(data.size());
-    if (incrementalData) {
+    auto previousDataSize = encodedSize();
+    while (data.size() > previousDataSize) {
+        auto incrementalData = data.getSomeData(previousDataSize);
+        previousDataSize += incrementalData.size();
+
         SetForScope<bool> notifyScope(m_inIncrementalDataNotify, true);
-        notifyClientsDataWasReceived(incrementalData->data(), incrementalData->size());
-    }
+        notifyClientsDataWasReceived(incrementalData.data(), incrementalData.size());
+    };
+    setEncodedSize(data.size());
 
     if (dataBufferingPolicy() == DoNotBufferData) {
         if (m_loader)