Diff
Modified: branches/safari-605-branch/LayoutTests/ChangeLog (226687 => 226688)
--- branches/safari-605-branch/LayoutTests/ChangeLog 2018-01-10 04:31:37 UTC (rev 226687)
+++ branches/safari-605-branch/LayoutTests/ChangeLog 2018-01-10 04:31:41 UTC (rev 226688)
@@ -1,5 +1,26 @@
2018-01-09 Jason Marcell <[email protected]>
+ Cherry-pick r226617. rdar://problem/36392336
+
+ 2018-01-09 Ryosuke Niwa <[email protected]>
+
+ Release assert in addResourceTiming when a cache resource is requested during style recalc
+ https://bugs.webkit.org/show_bug.cgi?id=181137
+ <rdar://problem/35666574>
+
+ Reviewed by Simon Fraser.
+
+ Added a regression test for the crash.
+
+ Also fixed test cases in rt-performance-extensions.js which were incorrectly asserting and assuming that
+ resourcetimingbufferfull event will be fired when there are exactly the same number of entries as the buffer size.
+
+ * http/tests/performance/performance-resource-timing-resourcetimingbufferfull -crash-expected.txt: Added.
+ * http/tests/performance/performance-resource-timing-resourcetimingbufferfull-crash.html: Added.
+ * http/wpt/resource-timing/rt-performance-extensions.js: Fixed the test cases.
+
+2018-01-09 Jason Marcell <[email protected]>
+
Cherry-pick r226526. rdar://problem/36392384
2018-01-08 Youenn Fablet <[email protected]>
Added: branches/safari-605-branch/LayoutTests/http/tests/performance/performance-resource-timing-resourcetimingbufferfull-crash-expected.txt (0 => 226688)
--- branches/safari-605-branch/LayoutTests/http/tests/performance/performance-resource-timing-resourcetimingbufferfull-crash-expected.txt (rev 0)
+++ branches/safari-605-branch/LayoutTests/http/tests/performance/performance-resource-timing-resourcetimingbufferfull-crash-expected.txt 2018-01-10 04:31:41 UTC (rev 226688)
@@ -0,0 +1,36 @@
+This tests causing resource timing buffer to be full during a style recalc.
+WebKit should not hit a release assertion by dispatching an event synchronously.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+loadImagesInAnotherFrame(7)
+container.innerHTML = ""; resourcetimingbufferfullFired = false
+performance.setResourceTimingBufferSize(3); performance.clearResourceTimings()
+PASS performance.getEntriesByType("resource").length is 0
+divsWithBackgroundImages(7); container.getBoundingClientRect()
+PASS resources = performance.getEntriesByType("resource"); resources.length is 3
+PASS resources[0].initiatorType is "css"
+PASS new URL(resources[0].name).search is "?resource=0"
+PASS new URL(resources[1].name).search is "?resource=1"
+PASS new URL(resources[2].name).search is "?resource=2"
+PASS resourcetimingbufferfullEventCount is 0
+
+Inside resourcetimingbufferfull 1
+PASS performance.clearResourceTimings(); performance.getEntriesByType("resource").length is 0
+
+Inside resourcetimingbufferfull 2
+PASS resources = performance.getEntriesByType("resource"); resources.length is 3
+PASS new URL(resources[0].name).search is "?resource=3"
+PASS new URL(resources[1].name).search is "?resource=4"
+PASS new URL(resources[2].name).search is "?resource=5"
+PASS performance.clearResourceTimings(); performance.getEntriesByType("resource").length is 0
+
+After resourcetimingbufferfull
+PASS resources = performance.getEntriesByType("resource"); resources.length is 1
+PASS resources[0].initiatorType is "css"
+PASS new URL(resources[0].name).search is "?resource=6"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: branches/safari-605-branch/LayoutTests/http/tests/performance/performance-resource-timing-resourcetimingbufferfull-crash.html (0 => 226688)
--- branches/safari-605-branch/LayoutTests/http/tests/performance/performance-resource-timing-resourcetimingbufferfull-crash.html (rev 0)
+++ branches/safari-605-branch/LayoutTests/http/tests/performance/performance-resource-timing-resourcetimingbufferfull-crash.html 2018-01-10 04:31:41 UTC (rev 226688)
@@ -0,0 +1,92 @@
+<!DOCTYPE html>
+<html>
+<body>
+<style>
+#container div { width: 10px; height: 10px; }
+</style>
+<script src=""
+<div id="container">
+</div>
+<script>
+
+description('This tests causing resource timing buffer to be full during a style recalc.<br>'
+ + 'WebKit should not hit a release assertion by dispatching an event synchronously.');
+
+function loadImagesInAnotherFrame(count)
+{
+ const iframe = document.createElement('iframe');
+ document.body.appendChild(iframe);
+ const doc = iframe.contentDocument;
+ let loadCount = 0;
+ for (let i = 0; i < count; i++) {
+ const image = doc.createElement('img');
+ image.src = '' + i;
+ doc.body.appendChild(image);
+ image._onload_ = () => {
+ loadCount++;
+ if (loadCount == count)
+ startTest();
+ }
+ }
+}
+
+function startTest()
+{
+ evalAndLog('container.innerHTML = ""; resourcetimingbufferfullFired = false');
+ evalAndLog('performance.setResourceTimingBufferSize(3); performance.clearResourceTimings()');
+ shouldBe('performance.getEntriesByType("resource").length', '0');
+ evalAndLog('divsWithBackgroundImages(7); container.getBoundingClientRect()');
+ shouldBe('resources = performance.getEntriesByType("resource"); resources.length', '3');
+ shouldBeEqualToString('resources[0].initiatorType', 'css');
+ shouldBeEqualToString('new URL(resources[0].name).search', '?resource=0');
+ shouldBeEqualToString('new URL(resources[1].name).search', '?resource=1');
+ shouldBeEqualToString('new URL(resources[2].name).search', '?resource=2');
+ shouldBe('resourcetimingbufferfullEventCount', '0');
+}
+
+function divsWithBackgroundImages(count)
+{
+ const container = document.getElementById('container');
+ for (let i = 0; i < count; i++) {
+ const div = document.createElement('div');
+ div.style.backgroundImage = `url('../../resources/square100.png?resource=${i}')`;
+ container.appendChild(div);
+ }
+}
+
+setTimeout(finishJSTest, 3000);
+
+let resourcetimingbufferfullEventCount = 0;
+performance._onresourcetimingbufferfull_ = () => {
+ resourcetimingbufferfullEventCount++;
+ debug('');
+ debug('Inside resourcetimingbufferfull ' + resourcetimingbufferfullEventCount);
+ if (resourcetimingbufferfullEventCount == 2) {
+ shouldBe('resources = performance.getEntriesByType("resource"); resources.length', '3');
+ shouldBeEqualToString('new URL(resources[0].name).search', '?resource=3');
+ shouldBeEqualToString('new URL(resources[1].name).search', '?resource=4');
+ shouldBeEqualToString('new URL(resources[2].name).search', '?resource=5');
+ }
+ shouldBe('performance.clearResourceTimings(); performance.getEntriesByType("resource").length', '0');
+
+ if (resourcetimingbufferfullEventCount < 2)
+ return;
+
+ setTimeout(() => {
+ debug('');
+ debug('After resourcetimingbufferfull');
+ shouldBe('resources = performance.getEntriesByType("resource"); resources.length', '1');
+ shouldBeEqualToString('resources[0].initiatorType', 'css');
+ shouldBeEqualToString('new URL(resources[0].name).search', '?resource=6');
+ finishJSTest();
+ }, 0);
+}
+
+evalAndLog('loadImagesInAnotherFrame(7)');
+
+var jsTestIsAsync = true;
+
+</script>
+<script src=""
+</body>
+</html>
Modified: branches/safari-605-branch/LayoutTests/http/wpt/resource-timing/rt-performance-extensions.js (226687 => 226688)
--- branches/safari-605-branch/LayoutTests/http/wpt/resource-timing/rt-performance-extensions.js 2018-01-10 04:31:37 UTC (rev 226687)
+++ branches/safari-605-branch/LayoutTests/http/wpt/resource-timing/rt-performance-extensions.js 2018-01-10 04:31:41 UTC (rev 226688)
@@ -66,7 +66,7 @@
return loadResources(3).then(function(entries) {
assert_equals(entries.length, 3, "context should have observed 3 resources");
assert_equals(performance.getEntriesByType("resource").length, 3, "context global buffer should be full at 3 resources");
- assert_true(bufferFullEventDispatched, "context should not have dispatched buffer full event");
+ assert_false(bufferFullEventDispatched, "context should not have dispatched buffer full event");
});
}, "resourcetimingbufferfull event should not trigger if exactly the BufferSizeLimit number of resources are added to the buffer", {timeout: 3000});
@@ -98,8 +98,8 @@
bufferFullEvent = event;
};
- return loadResources(1).then(function(entries) {
- assert_equals(entries.length, 1, "context should have observed 1 resource");
+ return loadResources(2).then(function(entries) {
+ assert_equals(entries.length, 2, "context should have observed 1 resource");
assert_equals(performance.getEntriesByType("resource").length, 1, "context global buffer should be full at 1 resource");
assert_equals(bufferFullEvent.target, performance, "event should dispatch at the performance object");
assert_true(bufferFullEvent.bubbles, "event should bubble");
Modified: branches/safari-605-branch/Source/WebCore/ChangeLog (226687 => 226688)
--- branches/safari-605-branch/Source/WebCore/ChangeLog 2018-01-10 04:31:37 UTC (rev 226687)
+++ branches/safari-605-branch/Source/WebCore/ChangeLog 2018-01-10 04:31:41 UTC (rev 226688)
@@ -1,5 +1,44 @@
2018-01-09 Jason Marcell <[email protected]>
+ Cherry-pick r226617. rdar://problem/36392336
+
+ 2018-01-09 Ryosuke Niwa <[email protected]>
+
+ Release assert in addResourceTiming when a cache resource is requested during style recalc
+ https://bugs.webkit.org/show_bug.cgi?id=181137
+ <rdar://problem/35666574>
+
+ Reviewed by Simon Fraser.
+
+ Make the dispatching of resourcetimingbufferfull event asynchronous to avoid dispatching it
+ synchronously during a style resolution when CachedResourceLoader::requestImage requests
+ a previously loaded image.
+
+ We now schedule a timer when the resource timing buffer becomes full, and dispatch the event
+ when the timer fires. Meanwhile, we have a backup buffer to which additional resource timing
+ entries would be added. Once the event is dispatched, we refill the buffer exposed to author
+ scripts. When refilling the buffer results in it becoming full again, we keep repeating the
+ process of firing resourcetimingbufferfull and re-filling the buffer until either we stop
+ making progress (i.e. the script didn't increase the number of empty entires in the buffer)
+ or the backup buffer (at the time we started this process) becomes empty.
+
+ Also fixed a bug that we were firing resourcetimingbufferfull event when the last entry that
+ fits within the buffer size was added instead of when an entry is being added to an already
+ full buffer. To make this work, the patch introduces m_resourceTimingBufferFullFlag,
+ representing the concept "resource timing buffer full" flag in the resource timing specification.
+
+ Test: http/tests/performance/performance-resource-timing-resourcetimingbufferfull-crash.html
+
+ * page/Performance.cpp:
+ (WebCore::Performance::Performance):
+ (WebCore::Performance::clearResourceTimings):
+ (WebCore::Performance::setResourceTimingBufferSize):
+ (WebCore::Performance::addResourceTiming):
+ (WebCore::Performance::resourceTimingBufferFullTimerFired):
+ * page/Performance.h:
+
+2018-01-09 Jason Marcell <[email protected]>
+
Cherry-pick r226542. rdar://problem/36392364
2018-01-08 John Wilander <[email protected]>
Modified: branches/safari-605-branch/Source/WebCore/page/Performance.cpp (226687 => 226688)
--- branches/safari-605-branch/Source/WebCore/page/Performance.cpp 2018-01-10 04:31:37 UTC (rev 226687)
+++ branches/safari-605-branch/Source/WebCore/page/Performance.cpp 2018-01-10 04:31:41 UTC (rev 226688)
@@ -52,6 +52,7 @@
Performance::Performance(ScriptExecutionContext& context, MonotonicTime timeOrigin)
: ContextDestructionObserver(&context)
+ , m_resourceTimingBufferFullTimer(*this, &Performance::resourceTimingBufferFullTimerFired)
, m_timeOrigin(timeOrigin)
, m_performanceTimelineTaskQueue(context)
{
@@ -166,26 +167,41 @@
void Performance::clearResourceTimings()
{
m_resourceTimingBuffer.clear();
+ m_resourceTimingBufferFullFlag = false;
}
void Performance::setResourceTimingBufferSize(unsigned size)
{
m_resourceTimingBufferSize = size;
+ m_resourceTimingBufferFullFlag = false;
}
void Performance::addResourceTiming(ResourceTiming&& resourceTiming)
{
- RefPtr<PerformanceResourceTiming> entry = PerformanceResourceTiming::create(m_timeOrigin, WTFMove(resourceTiming));
+ auto entry = PerformanceResourceTiming::create(m_timeOrigin, WTFMove(resourceTiming));
- queueEntry(*entry);
+ if (m_waitingForBackupBufferToBeProcessed) {
+ m_backupResourceTimingBuffer.append(WTFMove(entry));
+ return;
+ }
- if (isResourceTimingBufferFull())
+ if (m_resourceTimingBufferFullFlag) {
+ // We fired resourcetimingbufferfull evnet but the author script didn't clear the buffer.
+ // Notify performance observers but don't add it to the buffer.
+ queueEntry(entry.get());
return;
+ }
- m_resourceTimingBuffer.append(entry);
+ if (isResourceTimingBufferFull()) {
+ ASSERT(!m_resourceTimingBufferFullTimer.isActive());
+ m_backupResourceTimingBuffer.append(WTFMove(entry));
+ m_waitingForBackupBufferToBeProcessed = true;
+ m_resourceTimingBufferFullTimer.startOneShot(0_s);
+ return;
+ }
- if (isResourceTimingBufferFull())
- dispatchEvent(Event::create(eventNames().resourcetimingbufferfullEvent, true, false));
+ queueEntry(entry.get());
+ m_resourceTimingBuffer.append(WTFMove(entry));
}
bool Performance::isResourceTimingBufferFull() const
@@ -193,6 +209,39 @@
return m_resourceTimingBuffer.size() >= m_resourceTimingBufferSize;
}
+void Performance::resourceTimingBufferFullTimerFired()
+{
+ while (!m_backupResourceTimingBuffer.isEmpty()) {
+ auto backupBuffer = WTFMove(m_backupResourceTimingBuffer);
+
+ 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) {
+ for (auto& entry : backupBuffer)
+ queueEntry(*entry);
+ // Dispatching resourcetimingbufferfull event may have inserted more entries.
+ for (auto& entry : m_backupResourceTimingBuffer)
+ queueEntry(*entry);
+ break;
+ }
+
+ unsigned i = 0;
+ for (auto& entry : backupBuffer) {
+ if (i < remainingBufferSize) {
+ m_resourceTimingBuffer.append(entry.copyRef());
+ queueEntry(*entry);
+ } else
+ m_backupResourceTimingBuffer.append(entry.copyRef());
+ i++;
+ }
+ }
+ m_waitingForBackupBufferToBeProcessed = false;
+}
+
ExceptionOr<void> Performance::mark(const String& markName)
{
if (!m_userTiming)
Modified: branches/safari-605-branch/Source/WebCore/page/Performance.h (226687 => 226688)
--- branches/safari-605-branch/Source/WebCore/page/Performance.h 2018-01-10 04:31:37 UTC (rev 226687)
+++ branches/safari-605-branch/Source/WebCore/page/Performance.h 2018-01-10 04:31:41 UTC (rev 226688)
@@ -101,6 +101,7 @@
void derefEventTarget() final { deref(); }
bool isResourceTimingBufferFull() const;
+ void resourceTimingBufferFullTimerFired();
void queueEntry(PerformanceEntry&);
@@ -111,6 +112,13 @@
Vector<RefPtr<PerformanceEntry>> m_resourceTimingBuffer;
unsigned m_resourceTimingBufferSize { 150 };
+ Timer m_resourceTimingBufferFullTimer;
+ Vector<RefPtr<PerformanceEntry>> m_backupResourceTimingBuffer;
+
+ // https://w3c.github.io/resource-timing/#dfn-resource-timing-buffer-full-flag
+ bool m_resourceTimingBufferFullFlag { false };
+ bool m_waitingForBackupBufferToBeProcessed { false };
+
MonotonicTime m_timeOrigin;
std::unique_ptr<UserTiming> m_userTiming;
