Changeset 227926 in webkit

Timestamp:

Jan 31, 2018 1:07:36 PM (6 years ago)

Author:

rniwa@webkit.org

Message:

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.

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/http/tests/performance/performance-resource-timing-resourcetimingbufferfull-queue-resource-entry-expected.txt (added)
• LayoutTests/http/tests/performance/performance-resource-timing-resourcetimingbufferfull-queue-resource-entry.html (added)
• LayoutTests/http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash-expected.txt (added)
• LayoutTests/http/tests/performance/performance-resource-timing-resourcetimingbufferfull-shrinking-buffer-crash.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/page/Performance.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/page/Performance.cpp

@@ -214 +214 @@
     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);
@@ -227 +225 @@
             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++;
         }
     }