Diff
Modified: trunk/LayoutTests/ChangeLog (114373 => 114374)
--- trunk/LayoutTests/ChangeLog 2012-04-17 13:44:43 UTC (rev 114373)
+++ trunk/LayoutTests/ChangeLog 2012-04-17 14:02:07 UTC (rev 114374)
@@ -1,3 +1,17 @@
+2012-04-17 Allan Sandfeld Jensen <[email protected]>
+
+ Asserts in XMLHttpRequestProgressEventThrottle
+ https://bugs.webkit.org/show_bug.cgi?id=81506
+
+ Reviewed by Julien Chaffraix.
+
+ Tests that xmlhttprequest.open does not assert in when called from onpagehide.
+
+ * fast/events/pagehide-xhr-open-expected.txt: Added.
+ * fast/events/pagehide-xhr-open.html: Added.
+ * fast/events/resources/subframe-xmlhttprequest.html: Added.
+ * platform/chromium/test_expectations.txt:
+
2012-04-17 Mike Reed <[email protected]>
Need to rebaseline these images after skia 3695 lands
Added: trunk/LayoutTests/fast/events/pagehide-xhr-open-expected.txt (0 => 114374)
--- trunk/LayoutTests/fast/events/pagehide-xhr-open-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/events/pagehide-xhr-open-expected.txt 2012-04-17 14:02:07 UTC (rev 114374)
@@ -0,0 +1,12 @@
+CONSOLE MESSAGE: line 6: XMLHttpRequest in subframe-1 created
+CONSOLE MESSAGE: line 5: Loaded pagehide-timeout-go-back.html, going back
+CONSOLE MESSAGE: line 17: Restored page from page cache.
+Tries to trigger an assert in xmlhttprequest during pagehide.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/events/pagehide-xhr-open.html (0 => 114374)
--- trunk/LayoutTests/fast/events/pagehide-xhr-open.html (rev 0)
+++ trunk/LayoutTests/fast/events/pagehide-xhr-open.html 2012-04-17 14:02:07 UTC (rev 114374)
@@ -0,0 +1,34 @@
+<html>
+<script src=""
+<body>
+
+<iframe src="" id="a-frame"></iframe> <iframe src="" id="b-frame"></iframe>
+
+<script type="text/_javascript_">
+description('Tries to trigger an assert in xmlhttprequest during pagehide.');
+
+if (window.layoutTestController)
+ layoutTestController.overridePreference('WebKitUsesPageCachePreferenceKey', 1);
+
+
+_onpageshow_ = function(event)
+{
+ if (event.persisted) {
+ console.log('Restored page from page cache.');
+ if (!window.wasFinishJSTestCalled)
+ finishJSTest();
+ }
+}
+
+_onload_ = function()
+{
+ setTimeout(function() {
+ location.href = '';
+ }, 0);
+}
+var successfullyParsed = true;
+var jsTestIsAsync = true;
+</script>
+<script src=""
+</body>
+</html>
Added: trunk/LayoutTests/fast/events/resources/subframe-xmlhttprequest.html (0 => 114374)
--- trunk/LayoutTests/fast/events/resources/subframe-xmlhttprequest.html (rev 0)
+++ trunk/LayoutTests/fast/events/resources/subframe-xmlhttprequest.html 2012-04-17 14:02:07 UTC (rev 114374)
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <script>
+ function test() {
+ window.top.console.log('XMLHttpRequest in subframe-1 created');
+ var xhr = new XMLHttpRequest();
+ xhr.open('GET', 'about:empty', true);
+ }
+ </script>
+</head>
+<body>
+I am subframe-1
+</body>
+</html>
Modified: trunk/LayoutTests/platform/chromium/test_expectations.txt (114373 => 114374)
--- trunk/LayoutTests/platform/chromium/test_expectations.txt 2012-04-17 13:44:43 UTC (rev 114373)
+++ trunk/LayoutTests/platform/chromium/test_expectations.txt 2012-04-17 14:02:07 UTC (rev 114374)
@@ -511,6 +511,7 @@
// on which renderer process to use for each of them.
WONTFIX SKIP : fast/dom/Window/timer-resume-on-navigation-back.html = TIMEOUT FAIL
WONTFIX SKIP : fast/events/pagehide-timeout.html = TIMEOUT
+WONTFIX SKIP : fast/events/pagehide-xhr-open.html = TIMEOUT
WONTFIX SKIP : fast/events/pageshow-pagehide-on-back-cached-with-frames.html = TIMEOUT
WONTFIX SKIP : fast/events/pageshow-pagehide-on-back-cached.html = TIMEOUT FAIL
WONTFIX SKIP : fast/events/suspend-timers.html = TIMEOUT
Modified: trunk/Source/WebCore/ChangeLog (114373 => 114374)
--- trunk/Source/WebCore/ChangeLog 2012-04-17 13:44:43 UTC (rev 114373)
+++ trunk/Source/WebCore/ChangeLog 2012-04-17 14:02:07 UTC (rev 114374)
@@ -1,3 +1,39 @@
+2012-04-17 Allan Sandfeld Jensen <[email protected]>
+
+ Asserts in XMLHttpRequestProgressEventThrottle
+ https://bugs.webkit.org/show_bug.cgi?id=81506
+
+ Reviewed by Julien Chaffraix.
+
+ The asserts were incorrectly triggered because suspending active DOM objects
+ (which suspends the XMLHttpRequestProgressEventThrottle) doesn't stop _javascript_
+ from running or suspend any running loader we may have. The previous code would
+ assume those 2 cases were impossible.
+
+ When XmlHttpRequest::open is called or data is received while the XmlHttpRequest object
+ is suspended the object may attempt to dispatch events. This patch defers these events
+ until the object is resumed.
+
+ Progress events are coalesced similar to normal throttling, and readystate-change events
+ are coalesced to avoid identical events emitted right after eachother.
+
+ On resume the events are dispatched after a timer to avoid interfering with
+ ScriptExecutionContext which is iterating over suspended objects.
+
+ Test: fast/events/pagehide-xhr-open.html
+
+ * xml/XMLHttpRequestProgressEventThrottle.cpp:
+ (WebCore::XMLHttpRequestProgressEventThrottle::XMLHttpRequestProgressEventThrottle):
+ (WebCore::XMLHttpRequestProgressEventThrottle::dispatchProgressEvent):
+ (WebCore::XMLHttpRequestProgressEventThrottle::dispatchEvent):
+ (WebCore::XMLHttpRequestProgressEventThrottle::flushProgressEvent):
+ (WebCore::XMLHttpRequestProgressEventThrottle::dispatchDeferredEvents):
+ (WebCore::XMLHttpRequestProgressEventThrottle::fired):
+ (WebCore::XMLHttpRequestProgressEventThrottle::suspend):
+ (WebCore::XMLHttpRequestProgressEventThrottle::resume):
+ * xml/XMLHttpRequestProgressEventThrottle.h:
+ (XMLHttpRequestProgressEventThrottle):
+
2012-04-16 Vsevolod Vlasov <[email protected]>
Web Inspector: [Chromium] Crash when inspecting empty IndexedDB object store.
Modified: trunk/Source/WebCore/xml/XMLHttpRequestProgressEventThrottle.cpp (114373 => 114374)
--- trunk/Source/WebCore/xml/XMLHttpRequestProgressEventThrottle.cpp 2012-04-17 13:44:43 UTC (rev 114373)
+++ trunk/Source/WebCore/xml/XMLHttpRequestProgressEventThrottle.cpp 2012-04-17 14:02:07 UTC (rev 114374)
@@ -1,6 +1,6 @@
/*
- * Copyright (C) 2010 Julien Chaffraix <[email protected]>
- * All right reserved.
+ * Copyright (C) 2010 Julien Chaffraix <[email protected]> All right reserved.
+ * Copyright (C) 2012 Nokia Corporation and/or its subsidiary(-ies)
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -38,7 +38,8 @@
: m_target(target)
, m_loaded(0)
, m_total(0)
- , m_suspended(false)
+ , m_deferEvents(false)
+ , m_dispatchDeferredEventsTimer(this, &XMLHttpRequestProgressEventThrottle::dispatchDeferredEvents)
{
ASSERT(target);
}
@@ -49,7 +50,12 @@
void XMLHttpRequestProgressEventThrottle::dispatchProgressEvent(bool lengthComputable, unsigned long long loaded, unsigned long long total)
{
- ASSERT(!suspended());
+ if (m_deferEvents) {
+ // Only store the latest progress event while suspended.
+ m_deferredProgressEvent = XMLHttpRequestProgressEvent::create(eventNames().progressEvent, lengthComputable, loaded, total);
+ return;
+ }
+
if (!isActive()) {
// The timer is not active so the least frequent event for now is every byte.
// Just go ahead and dispatch the event.
@@ -79,11 +85,15 @@
void XMLHttpRequestProgressEventThrottle::dispatchEvent(PassRefPtr<Event> event)
{
- ASSERT(!suspended());
- // We should not have any pending events from a previous resume.
- ASSERT(!m_pausedEvent);
-
- m_target->dispatchEvent(event);
+ ASSERT(event);
+ if (m_deferEvents) {
+ if (m_deferredEvents.size() > 1 && event->type() == eventNames().readystatechangeEvent && event->type() == m_deferredEvents.last()->type()) {
+ // Readystatechange events are state-less so avoid repeating two identical events in a row on resume.
+ return;
+ }
+ m_deferredEvents.append(event);
+ } else
+ m_target->dispatchEvent(event);
}
void XMLHttpRequestProgressEventThrottle::dispatchEventAndLoadEnd(PassRefPtr<Event> event)
@@ -96,6 +106,13 @@
void XMLHttpRequestProgressEventThrottle::flushProgressEvent()
{
+ if (m_deferEvents && m_deferredProgressEvent) {
+ // Move the progress event to the queue, to get it in the right order on resume.
+ m_deferredEvents.append(m_deferredProgressEvent);
+ m_deferredProgressEvent = 0;
+ return;
+ }
+
if (!hasEventToDispatch())
return;
@@ -109,21 +126,33 @@
dispatchEvent(event);
}
-void XMLHttpRequestProgressEventThrottle::dispatchPausedEvent()
+void XMLHttpRequestProgressEventThrottle::dispatchDeferredEvents(Timer<XMLHttpRequestProgressEventThrottle>* timer)
{
- ASSERT(!suspended());
- if (!m_pausedEvent)
- return;
+ ASSERT_UNUSED(timer, timer == &m_dispatchDeferredEventsTimer);
+ ASSERT(m_deferEvents);
+ m_deferEvents = false;
- dispatchEvent(m_pausedEvent);
- m_pausedEvent = 0;
+ // Take over the deferred events before dispatching them which can potentially add more.
+ Vector<RefPtr<Event> > deferredEvents;
+ m_deferredEvents.swap(deferredEvents);
+
+ RefPtr<Event> deferredProgressEvent = m_deferredProgressEvent;
+ m_deferredProgressEvent = 0;
+
+ Vector<RefPtr<Event> >::const_iterator it = deferredEvents.begin();
+ const Vector<RefPtr<Event> >::const_iterator end = deferredEvents.end();
+ for (; it != end; ++it)
+ dispatchEvent(*it);
+
+ // The progress event will be in the m_deferredEvents vector if the load was finished while suspended.
+ // If not, just send the most up-to-date progress on resume.
+ if (deferredProgressEvent)
+ dispatchEvent(deferredProgressEvent);
}
void XMLHttpRequestProgressEventThrottle::fired()
{
ASSERT(isActive());
- ASSERT(!suspended());
- ASSERT(!m_pausedEvent);
if (!hasEventToDispatch()) {
// No progress event was queued since the previous dispatch, we can safely stop the timer.
stop();
@@ -142,13 +171,22 @@
void XMLHttpRequestProgressEventThrottle::suspend()
{
- ASSERT(!m_pausedEvent);
+ // If re-suspended before deferred events have been dispatched, just stop the dispatch
+ // and continue the last suspend.
+ if (m_dispatchDeferredEventsTimer.isActive()) {
+ ASSERT(m_deferEvents);
+ m_dispatchDeferredEventsTimer.stop();
+ return;
+ }
+ ASSERT(!m_deferredProgressEvent);
+ ASSERT(m_deferredEvents.isEmpty());
+ ASSERT(!m_deferEvents);
- m_suspended = true;
+ m_deferEvents = true;
// If we have a progress event waiting to be dispatched,
- // just queue it.
+ // just defer it.
if (hasEventToDispatch()) {
- m_pausedEvent = XMLHttpRequestProgressEvent::create(eventNames().progressEvent, m_lengthComputable, m_loaded, m_total);
+ m_deferredProgressEvent = XMLHttpRequestProgressEvent::create(eventNames().progressEvent, m_lengthComputable, m_loaded, m_total);
m_total = 0;
m_loaded = 0;
}
@@ -160,8 +198,11 @@
ASSERT(!m_loaded);
ASSERT(!m_total);
- m_suspended = false;
- dispatchPausedEvent();
+ // Do not dispatch events inline here, since ScriptExecutionContext is iterating over
+ // the list of active DOM objects to resume them, and any activated JS event-handler
+ // could insert new active DOM objects to the list.
+ // m_deferEvents is kept true until all deferred events have been dispatched.
+ m_dispatchDeferredEventsTimer.startOneShot(0);
}
} // namespace WebCore
Modified: trunk/Source/WebCore/xml/XMLHttpRequestProgressEventThrottle.h (114373 => 114374)
--- trunk/Source/WebCore/xml/XMLHttpRequestProgressEventThrottle.h 2012-04-17 13:44:43 UTC (rev 114373)
+++ trunk/Source/WebCore/xml/XMLHttpRequestProgressEventThrottle.h 2012-04-17 14:02:07 UTC (rev 114374)
@@ -56,13 +56,11 @@
void suspend();
void resume();
- bool suspended() const { return m_suspended; }
-
private:
static const double minimumProgressEventDispatchingIntervalInSeconds;
virtual void fired();
- void dispatchPausedEvent();
+ void dispatchDeferredEvents(Timer<XMLHttpRequestProgressEventThrottle>*);
void flushProgressEvent();
bool hasEventToDispatch() const;
@@ -74,8 +72,10 @@
unsigned long long m_loaded;
unsigned long long m_total;
- bool m_suspended;
- RefPtr<Event> m_pausedEvent;
+ bool m_deferEvents;
+ RefPtr<Event> m_deferredProgressEvent;
+ Vector<RefPtr<Event> > m_deferredEvents;
+ Timer<XMLHttpRequestProgressEventThrottle> m_dispatchDeferredEventsTimer;
};
} // namespace WebCore
