Title: [124113] branches/safari-536.26-branch/Source/WebCore
Revision
124113
Author
lforsch...@apple.com
Date
2012-07-30 16:19:40 -0700 (Mon, 30 Jul 2012)

Log Message

Merged r120954.  <rdar://problem/11932588>

Modified Paths

Diff

Modified: branches/safari-536.26-branch/Source/WebCore/ChangeLog (124112 => 124113)


--- branches/safari-536.26-branch/Source/WebCore/ChangeLog	2012-07-30 23:16:24 UTC (rev 124112)
+++ branches/safari-536.26-branch/Source/WebCore/ChangeLog	2012-07-30 23:19:40 UTC (rev 124113)
@@ -1,5 +1,27 @@
 2012-07-30  Lucas Forschler  <lforsch...@apple.com>
 
+    Merge 120954
+
+    2012-06-21  Brady Eidson  <beid...@apple.com>
+
+            <rdar://problem/11718988> and https://bugs.webkit.org/show_bug.cgi?id=89673
+            showModalDialog fix creates risk of never returning from RunLoop::performWork, potentially blocking other event sources
+
+            In case handling a function on the queue places additional functions on the queue, we should
+            limit the number of functions each invocation of performWork() performs so it can return and
+            other event sources have a chance to spin.
+
+            The showModalDialog fix in question is http://trac.webkit.org/changeset/120879
+
+            Reviewed by Darin Adler and Anders Carlson.
+
+            * platform/RunLoop.cpp:
+            (WebCore::RunLoop::performWork): If there are only N functions in the queue when performWork is called,
+              only handle up to N functions before returning. Any additional functions will be handled the next time
+              the runloop spins.
+
+2012-07-30  Lucas Forschler  <lforsch...@apple.com>
+
     Merge 120662
 
     2012-06-18  Mike Lawther  <mikelawt...@chromium.org>

Modified: branches/safari-536.26-branch/Source/WebCore/platform/RunLoop.cpp (124112 => 124113)


--- branches/safari-536.26-branch/Source/WebCore/platform/RunLoop.cpp	2012-07-30 23:16:24 UTC (rev 124112)
+++ branches/safari-536.26-branch/Source/WebCore/platform/RunLoop.cpp	2012-07-30 23:19:40 UTC (rev 124113)
@@ -57,15 +57,42 @@
 
 void RunLoop::performWork()
 {
+    // It is important to handle the functions in the queue one at a time because while inside one of these
+    // functions we might re-enter RunLoop::performWork() and we need to be able to pick up where we left off.
+    // See http://webkit.org/b/89590 for more discussion.
+
+    // One possible scenario when handling the function queue is as follows:
+    // - RunLoop::performWork() is invoked with 1 function on the queue
+    // - Handling that function results in 1 more function being enqueued
+    // - Handling that one results in yet another being enqueued
+    // - And so on
+    //
+    // In this situation one invocation of performWork() never returns so all other event sources are blocked.
+    // By only handling up to the number of functions that were in the queue when performWork() is called
+    // we guarantee to occasionally return from the run loop so other event sources will be allowed to spin.
+
     Function<void()> function;
-    
-    while (true) {
-        // It is important to handle the functions in the queue one at a time because while inside one of these
-        // functions we might re-enter RunLoop::performWork() and we need to be able to pick up where we left off.
-        // See http://webkit.org/b/89590 for more discussion.
+    size_t functionsToHandle = 0;
 
+    {
+        MutexLocker locker(m_functionQueueLock);
+        functionsToHandle = m_functionQueue.size();
+
+        if (m_functionQueue.isEmpty())
+            return;
+
+        function = m_functionQueue.takeFirst();
+    }
+
+    function();
+
+    for (size_t functionsHandled = 1; functionsHandled < functionsToHandle; ++functionsHandled) {
         {
             MutexLocker locker(m_functionQueueLock);
+
+            // Even if we start off with N functions to handle and we've only handled less than N functions, the queue
+            // still might be empty because those functions might have been handled in an inner RunLoop::performWork().
+            // In that case we should bail here.
             if (m_functionQueue.isEmpty())
                 break;
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to