Title: [120909] branches/safari-536-branch/Source

Diff

Modified: branches/safari-536-branch/Source/WebCore/ChangeLog (120908 => 120909)


--- branches/safari-536-branch/Source/WebCore/ChangeLog	2012-06-21 08:07:56 UTC (rev 120908)
+++ branches/safari-536-branch/Source/WebCore/ChangeLog	2012-06-21 08:15:20 UTC (rev 120909)
@@ -1,3 +1,31 @@
+2012-06-20  Lucas Forschler  <[email protected]>
+
+    Merge 120879
+
+    2012-06-20  Brady Eidson  <[email protected]>
+
+            <rdar://problem/11653784> and https://bugs.webkit.org/show_bug.cgi?id=89590
+            showModalDialog message handling is flaky in WebKit2
+
+            Because RunLoop::performWork() swaps the function queue to a temporary Vector before calling
+            the functions an inner run-loop - such as we see with running a modal dialog - does not have
+            a change to handle any of the functions that were queued after the WebPageProxy::RunModal message.
+
+            By servicing the functions in the queue one at a time we can give the RunLoop a chance to pick up
+            where it left off if RunLoop::performWork is re-entered.
+
+            To guarantee RunLoop::performWork is re-entered to handle those functions we also need to signal
+            its source before entering the modal run loop so our RunLoop is woken up.
+
+            Reviewed by Darin Adler.
+
+            * WebCore.exp.in:
+            * platform/RunLoop.cpp:
+            (WebCore::RunLoop::performWork): Take the first function off the queue one at a time so subsequent
+              functions remain in the queue and can be handled by an inner modal run loop.
+            * platform/RunLoop.h:
+            (RunLoop): Change the function queue to be a Deque to efficiently support "takeFirst" 
+
 2012-06-12  Lucas Forschler  <[email protected]>
 
     Merge 120364

Modified: branches/safari-536-branch/Source/WebCore/WebCore.exp.in (120908 => 120909)


--- branches/safari-536-branch/Source/WebCore/WebCore.exp.in	2012-06-21 08:07:56 UTC (rev 120908)
+++ branches/safari-536-branch/Source/WebCore/WebCore.exp.in	2012-06-21 08:15:20 UTC (rev 120909)
@@ -945,6 +945,7 @@
 __ZN7WebCore7RunLoop3runEv
 __ZN7WebCore7RunLoop4mainEv
 __ZN7WebCore7RunLoop4stopEv
+__ZN7WebCore7RunLoop6wakeUpEv
 __ZN7WebCore7RunLoop7currentEv
 __ZN7WebCore7RunLoop8dispatchERKN3WTF8FunctionIFvvEEE
 __ZN7WebCore7RunLoop9TimerBase4stopEv

Modified: branches/safari-536-branch/Source/WebCore/platform/RunLoop.cpp (120908 => 120909)


--- branches/safari-536-branch/Source/WebCore/platform/RunLoop.cpp	2012-06-21 08:07:56 UTC (rev 120908)
+++ branches/safari-536-branch/Source/WebCore/platform/RunLoop.cpp	2012-06-21 08:15:20 UTC (rev 120909)
@@ -57,14 +57,23 @@
 
 void RunLoop::performWork()
 {
-    Vector<Function<void()> > functionQueue;
-    {
-        MutexLocker locker(m_functionQueueLock);
-        m_functionQueue.swap(functionQueue);
+    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.
+
+        {
+            MutexLocker locker(m_functionQueueLock);
+            if (m_functionQueue.isEmpty())
+                break;
+
+            function = m_functionQueue.takeFirst();
+        }
+        
+        function();
     }
-
-    for (size_t i = 0; i < functionQueue.size(); ++i)
-        functionQueue[i]();
 }
 
 void RunLoop::dispatch(const Function<void()>& function)

Modified: branches/safari-536-branch/Source/WebCore/platform/RunLoop.h (120908 => 120909)


--- branches/safari-536-branch/Source/WebCore/platform/RunLoop.h	2012-06-21 08:07:56 UTC (rev 120908)
+++ branches/safari-536-branch/Source/WebCore/platform/RunLoop.h	2012-06-21 08:15:20 UTC (rev 120909)
@@ -28,12 +28,12 @@
 #ifndef RunLoop_h
 #define RunLoop_h
 
+#include <wtf/Deque.h>
 #include <wtf/Forward.h>
 #include <wtf/Functional.h>
 #include <wtf/HashMap.h>
 #include <wtf/ThreadSpecific.h>
 #include <wtf/Threading.h>
-#include <wtf/Vector.h>
 
 #if PLATFORM(GTK)
 #include <wtf/gobject/GRefPtr.h>
@@ -58,6 +58,7 @@
 
     static void run();
     void stop();
+    void wakeUp();
 
 #if PLATFORM(MAC)
     void runForDuration(double duration);
@@ -132,10 +133,9 @@
     ~RunLoop();
 
     void performWork();
-    void wakeUp();
 
     Mutex m_functionQueueLock;
-    Vector<Function<void()> > m_functionQueue;
+    Deque<Function<void()> > m_functionQueue;
 
 #if PLATFORM(WIN)
     static bool registerRunLoopMessageWindowClass();

Modified: branches/safari-536-branch/Source/WebKit2/ChangeLog (120908 => 120909)


--- branches/safari-536-branch/Source/WebKit2/ChangeLog	2012-06-21 08:07:56 UTC (rev 120908)
+++ branches/safari-536-branch/Source/WebKit2/ChangeLog	2012-06-21 08:15:20 UTC (rev 120909)
@@ -1,5 +1,34 @@
 2012-06-20  Lucas Forschler  <[email protected]>
 
+    Merge 120879
+
+    2012-06-20  Brady Eidson  <[email protected]>
+
+            <rdar://problem/11653784> and https://bugs.webkit.org/show_bug.cgi?id=89590
+            showModalDialog message handling is flaky in WebKit2
+
+            Because RunLoop::performWork() swaps the function queue to a temporary Vector before calling
+            the functions an inner run-loop - such as we see with running a modal dialog - does not have
+            a change to handle any of the functions that were queued after the WebPageProxy::RunModal message.
+
+            By servicing the functions in the queue one at a time we can give the RunLoop a chance to pick up
+            where it left off if RunLoop::performWork is re-entered.
+
+            To guarantee RunLoop::performWork is re-entered to handle those functions we also need to signal
+            its source before entering the modal run loop so our RunLoop is woken up.
+
+            Reviewed by Darin Adler.
+
+            * Platform/CoreIPC/Connection.cpp:
+            (CoreIPC::Connection::wakeUpRunLoop): Added so the WebPageProxy can signal the runloop to be woken up
+              before it enters the modal dialog run loop.
+            * Platform/CoreIPC/Connection.h:
+
+            * UIProcess/WebPageProxy.cpp:
+            (WebKit::WebPageProxy::runModal): Call wakeUpRunLoop() before entering the modal dialog run loop.
+
+2012-06-20  Lucas Forschler  <[email protected]>
+
     Merge 120376
 
     2012-06-14  Tim Horton  <[email protected]>

Modified: branches/safari-536-branch/Source/WebKit2/Platform/CoreIPC/Connection.cpp (120908 => 120909)


--- branches/safari-536-branch/Source/WebKit2/Platform/CoreIPC/Connection.cpp	2012-06-21 08:07:56 UTC (rev 120908)
+++ branches/safari-536-branch/Source/WebKit2/Platform/CoreIPC/Connection.cpp	2012-06-21 08:15:20 UTC (rev 120909)
@@ -718,4 +718,9 @@
     dispatchMessage(incomingMessage);
 }
 
+void Connection::wakeUpRunLoop()
+{
+    m_clientRunLoop->wakeUp();
+}
+
 } // namespace CoreIPC

Modified: branches/safari-536-branch/Source/WebKit2/Platform/CoreIPC/Connection.h (120908 => 120909)


--- branches/safari-536-branch/Source/WebKit2/Platform/CoreIPC/Connection.h	2012-06-21 08:07:56 UTC (rev 120908)
+++ branches/safari-536-branch/Source/WebKit2/Platform/CoreIPC/Connection.h	2012-06-21 08:15:20 UTC (rev 120909)
@@ -167,6 +167,8 @@
     // All clients should move to the overloads that take a message type.
     template<typename E, typename T> bool deprecatedSend(E messageID, uint64_t destinationID, const T& arguments);
     template<typename E, typename T, typename U> bool deprecatedSendSync(E messageID, uint64_t destinationID, const T& arguments, const U& reply, double timeout = NoTimeout);
+    
+    void wakeUpRunLoop();
 
 private:
     template<typename T> class Message {

Modified: branches/safari-536-branch/Source/WebKit2/UIProcess/WebPageProxy.cpp (120908 => 120909)


--- branches/safari-536-branch/Source/WebKit2/UIProcess/WebPageProxy.cpp	2012-06-21 08:07:56 UTC (rev 120908)
+++ branches/safari-536-branch/Source/WebKit2/UIProcess/WebPageProxy.cpp	2012-06-21 08:15:20 UTC (rev 120909)
@@ -3588,6 +3588,12 @@
     // Since runModal() can (and probably will) spin a nested run loop we need to turn off the responsiveness timer.
     process()->responsivenessTimer()->stop();
 
+    // Our Connection's run loop might have more messages waiting to be handled after this RunModal message.
+    // To make sure they are handled inside of the the nested modal run loop we must first signal the Connection's
+    // run loop so we're guaranteed that it has a chance to wake up.
+    // See http://webkit.org/b/89590 for more discussion.
+    process()->connection()->wakeUpRunLoop();
+
     m_uiClient.runModal(this);
 }
 
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to