- Revision
- 120879
- Author
- [email protected]
- Date
- 2012-06-20 16:05:04 -0700 (Wed, 20 Jun 2012)
Log Message
<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.
Source/WebCore:
* 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"
Source/WebKit2:
* 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.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (120878 => 120879)
--- trunk/Source/WebCore/ChangeLog 2012-06-20 23:01:14 UTC (rev 120878)
+++ trunk/Source/WebCore/ChangeLog 2012-06-20 23:05:04 UTC (rev 120879)
@@ -1,3 +1,27 @@
+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-20 Varun Jain <[email protected]>
Account for device scale factor when creating image for dragging.
Modified: trunk/Source/WebCore/WebCore.exp.in (120878 => 120879)
--- trunk/Source/WebCore/WebCore.exp.in 2012-06-20 23:01:14 UTC (rev 120878)
+++ trunk/Source/WebCore/WebCore.exp.in 2012-06-20 23:05:04 UTC (rev 120879)
@@ -802,6 +802,7 @@
__ZN7WebCore7RunLoop3runEv
__ZN7WebCore7RunLoop4mainEv
__ZN7WebCore7RunLoop4stopEv
+__ZN7WebCore7RunLoop6wakeUpEv
__ZN7WebCore7RunLoop7currentEv
__ZN7WebCore7RunLoop8dispatchERKN3WTF8FunctionIFvvEEE
__ZN7WebCore7RunLoop9TimerBase4stopEv
Modified: trunk/Source/WebCore/platform/RunLoop.cpp (120878 => 120879)
--- trunk/Source/WebCore/platform/RunLoop.cpp 2012-06-20 23:01:14 UTC (rev 120878)
+++ trunk/Source/WebCore/platform/RunLoop.cpp 2012-06-20 23:05:04 UTC (rev 120879)
@@ -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: trunk/Source/WebCore/platform/RunLoop.h (120878 => 120879)
--- trunk/Source/WebCore/platform/RunLoop.h 2012-06-20 23:01:14 UTC (rev 120878)
+++ trunk/Source/WebCore/platform/RunLoop.h 2012-06-20 23:05:04 UTC (rev 120879)
@@ -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: trunk/Source/WebKit2/ChangeLog (120878 => 120879)
--- trunk/Source/WebKit2/ChangeLog 2012-06-20 23:01:14 UTC (rev 120878)
+++ trunk/Source/WebKit2/ChangeLog 2012-06-20 23:05:04 UTC (rev 120879)
@@ -1,3 +1,28 @@
+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 Julien Chaffraix <[email protected]>
Use IntSize in RenderLayer to represent scroll offsets
Modified: trunk/Source/WebKit2/Platform/CoreIPC/Connection.cpp (120878 => 120879)
--- trunk/Source/WebKit2/Platform/CoreIPC/Connection.cpp 2012-06-20 23:01:14 UTC (rev 120878)
+++ trunk/Source/WebKit2/Platform/CoreIPC/Connection.cpp 2012-06-20 23:05:04 UTC (rev 120879)
@@ -717,4 +717,9 @@
dispatchMessage(incomingMessage);
}
+void Connection::wakeUpRunLoop()
+{
+ m_clientRunLoop->wakeUp();
+}
+
} // namespace CoreIPC
Modified: trunk/Source/WebKit2/Platform/CoreIPC/Connection.h (120878 => 120879)
--- trunk/Source/WebKit2/Platform/CoreIPC/Connection.h 2012-06-20 23:01:14 UTC (rev 120878)
+++ trunk/Source/WebKit2/Platform/CoreIPC/Connection.h 2012-06-20 23:05:04 UTC (rev 120879)
@@ -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: trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp (120878 => 120879)
--- trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp 2012-06-20 23:01:14 UTC (rev 120878)
+++ trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp 2012-06-20 23:05:04 UTC (rev 120879)
@@ -3622,6 +3622,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);
}
