Skip to content
Permalink
Browse files
Change FrameLoadDelegate to support any number of delegates with dela…
…yed 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.

Canonical link: https://commits.webkit.org/69493@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@79573 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
aroben committed Feb 24, 2011
1 parent 70d5fc5 commit 5cd8ca8b111110c8562b5d7773bf761839805d28
Showing with 47 additions and 8 deletions.
  1. +25 −0 Tools/ChangeLog
  2. +22 −8 Tools/DumpRenderTree/win/FrameLoadDelegate.cpp
@@ -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.
@@ -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;
}

0 comments on commit 5cd8ca8

Please sign in to comment.