Diff
Modified: trunk/LayoutTests/ChangeLog (102472 => 102473)
--- trunk/LayoutTests/ChangeLog 2011-12-09 21:41:58 UTC (rev 102472)
+++ trunk/LayoutTests/ChangeLog 2011-12-09 22:05:12 UTC (rev 102473)
@@ -1,3 +1,15 @@
+2011-12-09 David Levin <le...@chromium.org>
+
+ Regression(r53595): Sync xhr requests in workers aren't terminated on worker close.
+ https://bugs.webkit.org/show_bug.cgi?id=71695
+
+ Reviewed by Zoltan Herczeg.
+
+ * http/tests/workers/resources/worker-util.js: Added.
+ Copied from fast/workers/resources/worker-util.js
+ * http/tests/xmlhttprequest/workers/abort-exception-assert.html:
+ Make the test wait for all workers to exit before finishing.
+
2011-12-09 Eric Carlson <eric.carl...@apple.com>
JSC wrappers for TextTrack and TextTrackCue should not be collected during event dispatch or when owner is reachable
Added: trunk/LayoutTests/http/tests/workers/resources/worker-util.js (0 => 102473)
--- trunk/LayoutTests/http/tests/workers/resources/worker-util.js (rev 0)
+++ trunk/LayoutTests/http/tests/workers/resources/worker-util.js 2011-12-09 22:05:12 UTC (rev 102473)
@@ -0,0 +1,70 @@
+// Useful utilities for worker tests
+
+function log(message)
+{
+ document.getElementById("result").innerHTML += message + "<br>";
+}
+
+function gc(forceAlloc)
+{
+ if (typeof GCController !== "undefined")
+ GCController.collect();
+
+ if (typeof GCController == "undefined" || forceAlloc) {
+ function gcRec(n) {
+ if (n < 1)
+ return {};
+ var temp = {i: "ab" + i + (i / 100000)};
+ temp += "foo";
+ gcRec(n-1);
+ }
+ for (var i = 0; i < 1000; i++)
+ gcRec(10)
+ }
+}
+
+function waitUntilWorkerThreadsExit(callback)
+{
+ waitUntilThreadCountMatches(callback, 0);
+}
+
+function waitUntilThreadCountMatches(callback, count)
+{
+ // When running in a browser, just wait for one second then call the callback.
+ if (!window.layoutTestController) {
+ setTimeout(function() { gc(true); callback(); }, 1000);
+ return;
+ }
+
+ if (layoutTestController.workerThreadCount == count) {
+ // Worker threads have exited.
+ callback();
+ } else {
+ // Poll until worker threads have been GC'd/exited.
+ // Force a GC with a bunch of allocations to try to scramble the stack and force worker objects to be collected.
+ gc(true);
+ setTimeout(function() { waitUntilThreadCountMatches(callback, count); }, 10);
+ }
+}
+
+function ensureThreadCountMatches(callback, count)
+{
+ // Just wait until the thread count matches, then wait another 100ms to see if it changes, then fire the callback.
+ waitUntilThreadCountMatches(function() {
+ setTimeout(function() { waitUntilThreadCountMatches(callback, count); }, 100);
+ }, count);
+}
+
+function done()
+{
+ if (window.debug)
+ debug('<br><span class="pass">TEST COMPLETE</span>');
+ else
+ log("DONE");
+
+ // Call notifyDone via the queue so any pending console messages/exceptions are written out first.
+ setTimeout(function() {
+ if (window.layoutTestController)
+ layoutTestController.notifyDone();
+ }, 0);
+}
Property changes on: trunk/LayoutTests/http/tests/workers/resources/worker-util.js
___________________________________________________________________
Added: svn:eol-style
Modified: trunk/LayoutTests/http/tests/xmlhttprequest/workers/abort-exception-assert.html (102472 => 102473)
--- trunk/LayoutTests/http/tests/xmlhttprequest/workers/abort-exception-assert.html 2011-12-09 21:41:58 UTC (rev 102472)
+++ trunk/LayoutTests/http/tests/xmlhttprequest/workers/abort-exception-assert.html 2011-12-09 22:05:12 UTC (rev 102473)
@@ -2,6 +2,7 @@
<body>
<p>XmlHttpRequest abort exception shouldn't assert.</p>
<p>On success, you should see a single PASS below.</p>
+<script src=""
<script>
if (window.layoutTestController) {
layoutTestController.dumpAsText();
@@ -29,6 +30,11 @@
// This is an (unlikley) race condition here in that the worker may not have started
// the sync xhr call at this point, but it has been greatly lessened by the 100ms delay.
worker.terminate();
+ waitUntilWorkerThreadsExit(doneWithTest);
+ }
+
+ function doneWithTest()
+ {
log("PASS");
if (window.layoutTestController)
layoutTestController.notifyDone();
Modified: trunk/Source/_javascript_Core/ChangeLog (102472 => 102473)
--- trunk/Source/_javascript_Core/ChangeLog 2011-12-09 21:41:58 UTC (rev 102472)
+++ trunk/Source/_javascript_Core/ChangeLog 2011-12-09 22:05:12 UTC (rev 102473)
@@ -1,3 +1,15 @@
+2011-12-09 David Levin <le...@chromium.org>
+
+ Regression(r53595): Sync xhr requests in workers aren't terminated on worker close.
+ https://bugs.webkit.org/show_bug.cgi?id=71695
+
+ Reviewed by Zoltan Herczeg.
+
+ * wtf/MessageQueue.h:
+ (WTF::MessageQueue::tryGetMessageIgnoringKilled): Added a way to get messages
+ even after the queue has been killed. This is useful when one wants to
+ kill a queue but then go through it to run clean up tasks from it.
+
2011-12-09 Adrienne Walker <e...@google.com>
Fix HashMap<..., OwnPtr<...> >::add compilation errors
Modified: trunk/Source/_javascript_Core/wtf/MessageQueue.h (102472 => 102473)
--- trunk/Source/_javascript_Core/wtf/MessageQueue.h 2011-12-09 21:41:58 UTC (rev 102472)
+++ trunk/Source/_javascript_Core/wtf/MessageQueue.h 2011-12-09 22:05:12 UTC (rev 102473)
@@ -60,6 +60,7 @@
PassOwnPtr<DataType> waitForMessage();
PassOwnPtr<DataType> tryGetMessage();
+ PassOwnPtr<DataType> tryGetMessageIgnoringKilled();
template<typename Predicate>
PassOwnPtr<DataType> waitForMessageFilteredWithTimeout(MessageQueueWaitResult&, Predicate&, double absoluteTime);
@@ -168,6 +169,16 @@
}
template<typename DataType>
+ inline PassOwnPtr<DataType> MessageQueue<DataType>::tryGetMessageIgnoringKilled()
+ {
+ MutexLocker lock(m_mutex);
+ if (m_queue.isEmpty())
+ return nullptr;
+
+ return adoptPtr(m_queue.takeFirst());
+ }
+
+ template<typename DataType>
template<typename Predicate>
inline void MessageQueue<DataType>::removeIf(Predicate& predicate)
{
Modified: trunk/Source/WebCore/ChangeLog (102472 => 102473)
--- trunk/Source/WebCore/ChangeLog 2011-12-09 21:41:58 UTC (rev 102472)
+++ trunk/Source/WebCore/ChangeLog 2011-12-09 22:05:12 UTC (rev 102473)
@@ -1,3 +1,36 @@
+2011-12-09 David Levin <le...@chromium.org>
+
+ Regression(r53595): Sync xhr requests in workers aren't terminated on worker close.
+ https://bugs.webkit.org/show_bug.cgi?id=71695
+
+ Reviewed by Zoltan Herczeg.
+
+ Overview: Message loops rely on the message queue being killed in order
+ to exit. r53595 stopped this from happening because killing a message loop
+ would also stop it from doing database clean up tasks. The database clean up
+ tasks needed to be tasks due to ordering issues. (They wanted to run after
+ certain order tasks were run.) This was solved by once again terminating
+ the message queue but then still runnning clean-up tasks from the killed
+ message queue.
+
+ * workers/WorkerRunLoop.cpp:
+ (WebCore::WorkerRunLoop::run): Added the call to run clean-up tasks.
+ (WebCore::WorkerRunLoop::runInMode):
+ (WebCore::WorkerRunLoop::runCleanupTasks): Loop to simply clear out all clean up tasks.
+ (WebCore::WorkerRunLoop::Task::performTask): Stop non-clean up tasks
+ from running after the loop has been terminated.
+ * workers/WorkerRunLoop.h:
+ (WebCore::WorkerRunLoop::terminated): Just made it const.
+ * workers/WorkerThread.cpp:
+ (WebCore::WorkerThreadShutdownFinishTask::performTask): Removed
+ the terminate clause since it was put back in stop.
+ (WebCore::WorkerThread::stop): Terminate the run loop so
+ that all loops will exit and clean up tasks will run. Also removed a comment
+ about nested workers because nested workers are no longer imminent and the
+ issue mentioned is one of many that should logically be investigated -- behavior correctness
+ in the face of different orderings of shutdown between the document and each worker --
+ when implementing them.
+
2011-12-09 Tony Chang <t...@chromium.org>
Unreviewed, rolling out r102416.
Modified: trunk/Source/WebCore/workers/WorkerRunLoop.cpp (102472 => 102473)
--- trunk/Source/WebCore/workers/WorkerRunLoop.cpp 2011-12-09 21:41:58 UTC (rev 102472)
+++ trunk/Source/WebCore/workers/WorkerRunLoop.cpp 2011-12-09 22:05:12 UTC (rev 102473)
@@ -134,6 +134,7 @@
do {
result = runInMode(context, modePredicate);
} while (result != MessageQueueTerminated);
+ runCleanupTasks(context);
}
MessageQueueWaitResult WorkerRunLoop::runInMode(WorkerContext* context, const String& mode)
@@ -161,7 +162,7 @@
break;
case MessageQueueMessageReceived:
- task->performTask(context);
+ task->performTask(*this, context);
break;
case MessageQueueTimeout:
@@ -173,6 +174,21 @@
return result;
}
+void WorkerRunLoop::runCleanupTasks(WorkerContext* context)
+{
+ ASSERT(context);
+ ASSERT(context->thread());
+ ASSERT(context->thread()->threadID() == currentThread());
+ ASSERT(m_messageQueue.killed());
+
+ while (true) {
+ OwnPtr<WorkerRunLoop::Task> task = m_messageQueue.tryGetMessageIgnoringKilled();
+ if (!task)
+ return;
+ task->performTask(*this, context);
+ }
+}
+
void WorkerRunLoop::terminate()
{
m_messageQueue.kill();
@@ -193,10 +209,10 @@
return adoptPtr(new Task(task, mode));
}
-void WorkerRunLoop::Task::performTask(ScriptExecutionContext* context)
+void WorkerRunLoop::Task::performTask(const WorkerRunLoop& runLoop, ScriptExecutionContext* context)
{
WorkerContext* workerContext = static_cast<WorkerContext *>(context);
- if (!workerContext->isClosing() || m_task->isCleanupTask())
+ if ((!workerContext->isClosing() && !runLoop.terminated()) || m_task->isCleanupTask())
m_task->performTask(context);
}
@@ -206,7 +222,6 @@
{
}
-
} // namespace WebCore
#endif // ENABLE(WORKERS)
Modified: trunk/Source/WebCore/workers/WorkerRunLoop.h (102472 => 102473)
--- trunk/Source/WebCore/workers/WorkerRunLoop.h 2011-12-09 21:41:58 UTC (rev 102472)
+++ trunk/Source/WebCore/workers/WorkerRunLoop.h 2011-12-09 22:05:12 UTC (rev 102473)
@@ -56,7 +56,7 @@
MessageQueueWaitResult runInMode(WorkerContext*, const String& mode);
void terminate();
- bool terminated() { return m_messageQueue.killed(); }
+ bool terminated() const { return m_messageQueue.killed(); }
void postTask(PassOwnPtr<ScriptExecutionContext::Task>);
void postTaskForMode(PassOwnPtr<ScriptExecutionContext::Task>, const String& mode);
@@ -71,7 +71,7 @@
static PassOwnPtr<Task> create(PassOwnPtr<ScriptExecutionContext::Task> task, const String& mode);
~Task() { }
const String& mode() const { return m_mode; }
- void performTask(ScriptExecutionContext* context);
+ void performTask(const WorkerRunLoop&, ScriptExecutionContext*);
private:
Task(PassOwnPtr<ScriptExecutionContext::Task> task, const String& mode);
@@ -84,6 +84,10 @@
friend class RunLoopSetup;
MessageQueueWaitResult runInMode(WorkerContext*, const ModePredicate&);
+ // Runs any clean up tasks that are currently in the queue and returns.
+ // This should only be called when the context is closed or loop has been terminated.
+ void runCleanupTasks(WorkerContext*);
+
MessageQueue<Task> m_messageQueue;
OwnPtr<WorkerSharedTimer> m_sharedTimer;
int m_nestedCount;
Modified: trunk/Source/WebCore/workers/WorkerThread.cpp (102472 => 102473)
--- trunk/Source/WebCore/workers/WorkerThread.cpp 2011-12-09 21:41:58 UTC (rev 102472)
+++ trunk/Source/WebCore/workers/WorkerThread.cpp 2011-12-09 22:05:12 UTC (rev 102473)
@@ -195,7 +195,6 @@
#endif
// It's not safe to call clearScript until all the cleanup tasks posted by functions initiated by WorkerThreadShutdownStartTask have completed.
workerContext->clearScript();
- workerContext->thread()->runLoop().terminate();
}
virtual bool isCleanupTask() const { return true; }
@@ -252,13 +251,9 @@
#if ENABLE(SQL_DATABASE)
DatabaseTracker::tracker().interruptAllDatabasesForContext(m_workerContext.get());
#endif
-
- // FIXME: Rudely killing the thread won't work when we allow nested workers, because they will try to post notifications of their destruction.
- // This can likely use the same mechanism as used for databases above.
-
m_runLoop.postTask(WorkerThreadShutdownStartTask::create());
- } else
- m_runLoop.terminate();
+ }
+ m_runLoop.terminate();
}
} // namespace WebCore
