Release assertion in Performance::resourceTimingBufferFullTimerFired when the resourc...
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 31 Jan 2018 21:07:36 +0000 (21:07 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 31 Jan 2018 21:07:36 +0000 (21:07 +0000)
https://bugs.webkit.org/show_bug.cgi?id=182319
<rdar://problem/36904312>

Reviewed by Chris Dumez.

Source/WebCore:

The crash was caused by a wrong release assertion. Handle author scripts shrinking the resource timing buffer
while resourcetimingbufferfull event is being dispatched.

Also fixed a bug that a superflous resourcetimingbufferfull event will be fired when new resource timing entries
are queued while resourcetimingbufferfull event is being dispatched.

Test: http/tests/performance/performance-resource-timing-resourcetimingbufferfull-queue-resource-entry.html
      http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html

* page/Performance.cpp:
(WebCore::Performance::resourceTimingBufferFullTimerFired):

LayoutTests:

Added regression tests for shrinking the resoruce timing buffer and queuing a new resource timing entry while
resourcetimingbufferfull event is being dispatched.

* http/tests/performance/performance-resource-timing-resourcetimingbufferfull-queue-resource-entry-expected.txt: Added.
* http/tests/performance/performance-resource-timing-resourcetimingbufferfull-queue-resource-entry.html: Added.
* http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash-expected.txt: Added.
* http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html: Added.

* http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash-expected.txt: Added.
* http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/http/tests/performance/performance-resource-timing-resourcetimingbufferfull-queue-resource-entry-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/performance/performance-resource-timing-resourcetimingbufferfull-queue-resource-entry.html [new file with mode: 0644]
LayoutTests/http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/Performance.cpp

index 66b4aa1..e12c332 100644 (file)
@@ -1,3 +1,22 @@
+2018-01-31  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Release assertion in Performance::resourceTimingBufferFullTimerFired when the resource timing buffer is shrunk
+        https://bugs.webkit.org/show_bug.cgi?id=182319
+        <rdar://problem/36904312>
+
+        Reviewed by Chris Dumez.
+
+        Added regression tests for shrinking the resoruce timing buffer and queuing a new resource timing entry while
+        resourcetimingbufferfull event is being dispatched.
+
+        * http/tests/performance/performance-resource-timing-resourcetimingbufferfull-queue-resource-entry-expected.txt: Added.
+        * http/tests/performance/performance-resource-timing-resourcetimingbufferfull-queue-resource-entry.html: Added.
+        * http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash-expected.txt: Added.
+        * http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html: Added.
+
+        * http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash-expected.txt: Added.
+        * http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html: Added.
+
 2018-01-31  Matt Lewis  <jlewis3@apple.com>
 
         Adjusted expectations for fast/forms/searchfield-heights.html.
diff --git a/LayoutTests/http/tests/performance/performance-resource-timing-resourcetimingbufferfull-queue-resource-entry-expected.txt b/LayoutTests/http/tests/performance/performance-resource-timing-resourcetimingbufferfull-queue-resource-entry-expected.txt
new file mode 100644 (file)
index 0000000..1518a8c
--- /dev/null
@@ -0,0 +1,35 @@
+This tests clearing the resource timing buffer during a resourcetimingbufferfull event.
+WebKit should not hit a release assertion.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS performance.clearResourceTimings(); performance.getEntriesByType("resource").length is 0
+performance.setResourceTimingBufferSize(2); fetchImages(0, 3)
+PASS originalResources = performance.getEntriesByType("resource"); originalResources.length is 2
+PASS originalResources[0].initiatorType is "css"
+PASS new URL(originalResources[0].name).search is "?resource=0"
+PASS originalResources[1].initiatorType is "css"
+PASS new URL(originalResources[1].name).search is "?resource=1"
+PASS resourcetimingbufferfullEventCount is 0
+
+Inside resourcetimingbufferfull 1
+performance.setResourceTimingBufferSize(10); fetchImages(3, 4)
+PASS resourcesAfterShrinkingBuffer = performance.getEntriesByType("resource"); resourcesAfterShrinkingBuffer.length is 2
+PASS resourcesAfterShrinkingBuffer[0] is originalResources[0]
+PASS resourcesAfterShrinkingBuffer[1] is originalResources[1]
+
+After resourcetimingbufferfull
+PASS finalResources = performance.getEntriesByType("resource"); finalResources.length is 4
+PASS resourcetimingbufferfullEventCount is 1
+PASS finalResources[0] is originalResources[0]
+PASS finalResources[1] is originalResources[1]
+PASS finalResources[2].initiatorType is "css"
+PASS new URL(finalResources[2].name).search is "?resource=2"
+PASS finalResources[3].initiatorType is "css"
+PASS new URL(finalResources[3].name).search is "?resource=3"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
+
diff --git a/LayoutTests/http/tests/performance/performance-resource-timing-resourcetimingbufferfull-queue-resource-entry.html b/LayoutTests/http/tests/performance/performance-resource-timing-resourcetimingbufferfull-queue-resource-entry.html
new file mode 100644 (file)
index 0000000..449e1fd
--- /dev/null
@@ -0,0 +1,84 @@
+<!DOCTYPE html>
+<html>
+<body>
+<style>
+#container div { width: 10px; height: 10px; }
+</style>
+<script src="/resources/js-test-pre.js"></script>
+<div id="container"></div>
+<script>
+
+description('This tests clearing the resource timing buffer during a resourcetimingbufferfull event.<br>'
+    + 'WebKit should not hit a release assertion.');
+
+window.onload = () => {
+    const iframe = document.createElement('iframe');
+    document.getElementById('container').appendChild(iframe);
+    const doc = iframe.contentDocument;
+
+    const promises = [];
+    for (let i = 0; i < 5; i++) {
+        const img = doc.createElement('img');
+        img.src = `../../resources/square100.png?resource=${i}`;
+        promises[i] = new Promise((resolve) => {
+            img.onload = resolve;
+        });
+        doc.body.appendChild(img);
+    }
+    Promise.all(promises).then(runTest);
+}
+
+function runTest()
+{
+    shouldBe('performance.clearResourceTimings(); performance.getEntriesByType("resource").length', '0');
+    evalAndLog('performance.setResourceTimingBufferSize(2); fetchImages(0, 3)');
+    shouldBe('originalResources = performance.getEntriesByType("resource"); originalResources.length', '2');
+    shouldBeEqualToString('originalResources[0].initiatorType', 'css');
+    shouldBeEqualToString('new URL(originalResources[0].name).search', '?resource=0');
+    shouldBeEqualToString('originalResources[1].initiatorType', 'css');
+    shouldBeEqualToString('new URL(originalResources[1].name).search', '?resource=1');
+    shouldBe('resourcetimingbufferfullEventCount', '0');
+}
+
+let resourceIndex = 0;
+function fetchImages(startingIndex, pastEndingIndex)
+{
+    const container = document.getElementById('container');
+    for (let i = startingIndex; i < pastEndingIndex; i++) {
+        const div = document.createElement('div');
+        div.style.backgroundImage = `url('../../resources/square100.png?resource=${i}')`;
+        container.appendChild(div);
+    }
+    container.getBoundingClientRect();
+}
+
+let resourcetimingbufferfullEventCount = 0;
+performance.onresourcetimingbufferfull = () => {
+    resourcetimingbufferfullEventCount++;
+    debug('');
+    debug(`Inside resourcetimingbufferfull ${resourcetimingbufferfullEventCount}`);
+    evalAndLog('performance.setResourceTimingBufferSize(10); fetchImages(3, 4)');
+    shouldBe('resourcesAfterShrinkingBuffer = performance.getEntriesByType("resource"); resourcesAfterShrinkingBuffer.length', '2');
+    shouldBe('resourcesAfterShrinkingBuffer[0]', 'originalResources[0]');
+    shouldBe('resourcesAfterShrinkingBuffer[1]', 'originalResources[1]');
+    setTimeout(() => {
+        debug('');
+        debug('After resourcetimingbufferfull');
+        shouldBe('finalResources = performance.getEntriesByType("resource"); finalResources.length', '4');
+        shouldBe('resourcetimingbufferfullEventCount', '1');
+        shouldBe('finalResources[0]', 'originalResources[0]');
+        shouldBe('finalResources[1]', 'originalResources[1]');
+        shouldBeEqualToString('finalResources[2].initiatorType', 'css');
+        shouldBeEqualToString('new URL(finalResources[2].name).search', '?resource=2');
+        shouldBeEqualToString('finalResources[3].initiatorType', 'css');
+        shouldBeEqualToString('new URL(finalResources[3].name).search', '?resource=3');        
+        finishJSTest();
+    }, 0);
+}
+
+var jsTestIsAsync = true;
+
+</script>
+<script src="/resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash-expected.txt b/LayoutTests/http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash-expected.txt
new file mode 100644 (file)
index 0000000..d0961e9
--- /dev/null
@@ -0,0 +1,34 @@
+This tests clearing the resource timing buffer during a resourcetimingbufferfull event.
+WebKit should not hit a release assertion.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS performance.getEntriesByType("resource").length is 0
+performance.setResourceTimingBufferSize(2)
+fetchImages(3).then(checkInitialState)
+PASS originalResources = performance.getEntriesByType("resource"); originalResources.length is 2
+PASS originalResources[0].initiatorType is "fetch"
+PASS new URL(originalResources[0].name).search is "?resource=0"
+PASS originalResources[1].initiatorType is "fetch"
+PASS new URL(originalResources[1].name).search is "?resource=1"
+PASS resourcetimingbufferfullEventCount is 0
+
+Inside resourcetimingbufferfull 1
+performance.setResourceTimingBufferSize(1)
+PASS resourcesAfterShrinkingBuffer = performance.getEntriesByType("resource"); resourcesAfterShrinkingBuffer.length is 2
+PASS resourcesAfterShrinkingBuffer[0] is originalResources[0]
+PASS resourcesAfterShrinkingBuffer[1] is originalResources[1]
+
+Inside resourcetimingbufferfull 2
+PASS performance.clearResourceTimings(); performance.getEntriesByType("resource").length is 0
+
+After resourcetimingbufferfull
+PASS resourcetimingbufferfullEventCount is 2
+PASS resourcesAfterClearing = performance.getEntriesByType("resource"); resourcesAfterClearing.length is 1
+PASS resourcesAfterClearing[0].initiatorType is "fetch"
+PASS new URL(resourcesAfterClearing[0].name).search is "?resource=2"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html b/LayoutTests/http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
new file mode 100644 (file)
index 0000000..eb22404
--- /dev/null
@@ -0,0 +1,66 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="/resources/js-test-pre.js"></script>
+<script>
+
+description('This tests clearing the resource timing buffer during a resourcetimingbufferfull event.<br>'
+    + 'WebKit should not hit a release assertion.');
+
+function startTest()
+{
+    performance.clearResourceTimings();
+    shouldBe('performance.getEntriesByType("resource").length', '0');
+    evalAndLog('performance.setResourceTimingBufferSize(2)');
+    evalAndLog('fetchImages(3).then(checkInitialState)');
+}
+
+let resourceIndex = 0;
+async function fetchImages(count)
+{
+    for (let i = 0; i < count; i++, resourceIndex++)
+        await fetch(`../../resources/square100.png?resource=${resourceIndex}`);
+}
+
+async function checkInitialState()
+{
+    shouldBe('originalResources = performance.getEntriesByType("resource"); originalResources.length', '2');
+    shouldBeEqualToString('originalResources[0].initiatorType', 'fetch');
+    shouldBeEqualToString('new URL(originalResources[0].name).search', '?resource=0');
+    shouldBeEqualToString('originalResources[1].initiatorType', 'fetch');
+    shouldBeEqualToString('new URL(originalResources[1].name).search', '?resource=1');
+    shouldBe('resourcetimingbufferfullEventCount', '0');
+}
+
+let resourcetimingbufferfullEventCount = 0;
+performance.onresourcetimingbufferfull = () => {
+    resourcetimingbufferfullEventCount++;
+    debug('');
+    debug(`Inside resourcetimingbufferfull ${resourcetimingbufferfullEventCount}`);
+    if (resourcetimingbufferfullEventCount == 1) {
+        evalAndLog('performance.setResourceTimingBufferSize(1)');
+        shouldBe('resourcesAfterShrinkingBuffer = performance.getEntriesByType("resource"); resourcesAfterShrinkingBuffer.length', '2');
+        shouldBe('resourcesAfterShrinkingBuffer[0]', 'originalResources[0]');
+        shouldBe('resourcesAfterShrinkingBuffer[1]', 'originalResources[1]');
+    } else if (resourcetimingbufferfullEventCount == 2) {
+        shouldBe('performance.clearResourceTimings(); performance.getEntriesByType("resource").length', '0');
+        setTimeout(() => {
+            debug('');
+            debug('After resourcetimingbufferfull');
+            shouldBe('resourcetimingbufferfullEventCount', '2');
+            shouldBe('resourcesAfterClearing = performance.getEntriesByType("resource"); resourcesAfterClearing.length', '1');
+            shouldBeEqualToString('resourcesAfterClearing[0].initiatorType', 'fetch');
+            shouldBeEqualToString('new URL(resourcesAfterClearing[0].name).search', '?resource=2');
+            finishJSTest();
+        }, 0);
+    }
+}
+
+var jsTestIsAsync = true;
+
+window.onload = startTest;
+
+</script>
+<script src="/resources/js-test-post.js"></script>
+</body>
+</html>
index d7481cd..e6ec4e8 100644 (file)
@@ -1,3 +1,23 @@
+2018-01-31  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Release assertion in Performance::resourceTimingBufferFullTimerFired when the resource timing buffer is shrunk
+        https://bugs.webkit.org/show_bug.cgi?id=182319
+        <rdar://problem/36904312>
+
+        Reviewed by Chris Dumez.
+
+        The crash was caused by a wrong release assertion. Handle author scripts shrinking the resource timing buffer
+        while resourcetimingbufferfull event is being dispatched.
+
+        Also fixed a bug that a superflous resourcetimingbufferfull event will be fired when new resource timing entries
+        are queued while resourcetimingbufferfull event is being dispatched.
+
+        Test: http/tests/performance/performance-resource-timing-resourcetimingbufferfull-queue-resource-entry.html
+              http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html
+
+        * page/Performance.cpp:
+        (WebCore::Performance::resourceTimingBufferFullTimerFired): 
+
 2018-01-31  Youenn Fablet  <youenn@apple.com>
 
         com.apple.WebKit.Storage crashing at com.apple.WebCore: WebCore::SWServerRegistration::removeClientUsingRegistration
index e745cd6..4ea29c1 100644 (file)
@@ -213,30 +213,31 @@ void Performance::resourceTimingBufferFullTimerFired()
 {
     while (!m_backupResourceTimingBuffer.isEmpty()) {
         auto backupBuffer = WTFMove(m_backupResourceTimingBuffer);
+        ASSERT(m_backupResourceTimingBuffer.isEmpty());
 
         m_resourceTimingBufferFullFlag = true;
         dispatchEvent(Event::create(eventNames().resourcetimingbufferfullEvent, true, false));
 
-        RELEASE_ASSERT(m_resourceTimingBufferSize >= m_resourceTimingBuffer.size());
-        unsigned remainingBufferSize = m_resourceTimingBufferSize - m_resourceTimingBuffer.size();
-        bool bufferIsStillFullAfterDispatchingEvent = !remainingBufferSize;
-        if (bufferIsStillFullAfterDispatchingEvent) {
+        if (m_resourceTimingBufferFullFlag) {
             for (auto& entry : backupBuffer)
                 queueEntry(*entry);
             // Dispatching resourcetimingbufferfull event may have inserted more entries.
             for (auto& entry : m_backupResourceTimingBuffer)
                 queueEntry(*entry);
+            m_backupResourceTimingBuffer.clear();
             break;
         }
 
-        unsigned i = 0;
+        // More entries may have added while dispatching resourcetimingbufferfull event.
+        backupBuffer.appendVector(m_backupResourceTimingBuffer);
+        m_backupResourceTimingBuffer.clear();
+
         for (auto& entry : backupBuffer) {
-            if (i < remainingBufferSize) {
+            if (!isResourceTimingBufferFull()) {
                 m_resourceTimingBuffer.append(entry.copyRef());
                 queueEntry(*entry);
             } else
                 m_backupResourceTimingBuffer.append(entry.copyRef());
-            i++;
         }
     }
     m_waitingForBackupBufferToBeProcessed = false;