Change FrameLoadDelegate to support any number of delegates with delayed work to...
authoraroben@apple.com <aroben@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 24 Feb 2011 16:56:56 +0000 (16:56 +0000)
committeraroben@apple.com <aroben@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 24 Feb 2011 16:56:56 +0000 (16:56 +0000)
This makes our behavior match Mac more closely, and allows us to remove an incorrect
assertion that was firing during some tests. (The assertion was claiming that there was
never more than one delegate with delayed work to process, but that was not the case.)

Fixes <http://webkit.org/b/55146> Assertion failure in FrameLoadDelegate::locationChangeDone
when running http/tests/navigation/back-twice-without-commit.html

Reviewed by Eric Carlson.

* DumpRenderTree/win/FrameLoadDelegate.cpp:
(delegatesWithDelayedWork): Added. Returns all FrameLoadDelegates that have delayed work to
process. A single delegate may appear in this Vector more than once (just as, on Mac, a
single delegate may have multiple performSelector requests).
(processWorkTimer): Pass the HWND to ::KillTimer, for pedantic brownie points. Added an
assertion that the timer firing is the shared process work timer. Instead of using the
single, global "delegate waiting for timer" delegate, give all delegates that have delayed
work to process a chance to process their work.
(FrameLoadDelegate::locationChangeDone): If we don't already have an active timer for
processing delayed work, create one. Then add ourselves to the delegatesWithDelayedWork
Vector so our processWork function will be called when the timer fires.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@79573 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog
Tools/DumpRenderTree/win/FrameLoadDelegate.cpp

index cbfed51..5e38ceb 100644 (file)
@@ -1,3 +1,28 @@
+2011-02-24  Adam Roben  <aroben@apple.com>
+
+        Change FrameLoadDelegate to support any number of delegates with delayed work to process
+
+        This makes our behavior match Mac more closely, and allows us to remove an incorrect
+        assertion that was firing during some tests. (The assertion was claiming that there was
+        never more than one delegate with delayed work to process, but that was not the case.)
+
+        Fixes <http://webkit.org/b/55146> Assertion failure in FrameLoadDelegate::locationChangeDone
+        when running http/tests/navigation/back-twice-without-commit.html
+
+        Reviewed by Eric Carlson.
+
+        * DumpRenderTree/win/FrameLoadDelegate.cpp:
+        (delegatesWithDelayedWork): Added. Returns all FrameLoadDelegates that have delayed work to
+        process. A single delegate may appear in this Vector more than once (just as, on Mac, a
+        single delegate may have multiple performSelector requests).
+        (processWorkTimer): Pass the HWND to ::KillTimer, for pedantic brownie points. Added an
+        assertion that the timer firing is the shared process work timer. Instead of using the
+        single, global "delegate waiting for timer" delegate, give all delegates that have delayed
+        work to process a chance to process their work.
+        (FrameLoadDelegate::locationChangeDone): If we don't already have an active timer for
+        processing delayed work, create one. Then add ourselves to the delegatesWithDelayedWork
+        Vector so our processWork function will be called when the timer fires.
+
 2011-02-24  Vsevolod Vlasov  <vsevik@chromium.org>
 
         Reviewed by Alexey Proskuryakov.
index a84e0f3..9f0de91 100644 (file)
@@ -206,12 +206,26 @@ void FrameLoadDelegate::resetToConsistentState()
     m_accessibilityController->resetToConsistentState();
 }
 
-static void CALLBACK processWorkTimer(HWND, UINT, UINT_PTR id, DWORD)
+typedef Vector<COMPtr<FrameLoadDelegate> > DelegateVector;
+static DelegateVector& delegatesWithDelayedWork()
 {
-    ::KillTimer(0, id);
-    FrameLoadDelegate* d = g_delegateWaitingOnTimer;
-    g_delegateWaitingOnTimer = 0;
-    d->processWork();
+    DEFINE_STATIC_LOCAL(DelegateVector, delegates, ());
+    return delegates;
+}
+
+static UINT_PTR processWorkTimerID;
+
+static void CALLBACK processWorkTimer(HWND hwnd, UINT, UINT_PTR id, DWORD)
+{
+    ASSERT_ARG(id, id == processWorkTimerID);
+    ::KillTimer(hwnd, id);
+    processWorkTimerID = 0;
+
+    DelegateVector delegates;
+    delegates.swap(delegatesWithDelayedWork());
+
+    for (size_t i = 0; i < delegates.size(); ++i)
+        delegates[i]->processWork();
 }
 
 void FrameLoadDelegate::locationChangeDone(IWebError*, IWebFrame* frame)
@@ -226,9 +240,9 @@ void FrameLoadDelegate::locationChangeDone(IWebError*, IWebFrame* frame)
         return;
 
     if (WorkQueue::shared()->count()) {
-        ASSERT(!g_delegateWaitingOnTimer);
-        g_delegateWaitingOnTimer = this;
-        ::SetTimer(0, 0, 0, processWorkTimer);
+        if (!processWorkTimerID)
+            processWorkTimerID = ::SetTimer(0, 0, 0, processWorkTimer);
+        delegatesWithDelayedWork().append(this);
         return;
     }