Bug 52264 – REGRESSION (r71562): servePendingRequests() no longer called when resources are done loading.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Andy Estes]

REGRESSION (r71562): servePendingRequests() no longer called when resources are done loading.

[Andy Estes]

<rdar://problem/8767429>

[Andy Estes]

The removal of servePendingRequests() is what caused the Adobe Creative Suite 4 installer to break on Mac OS X.

[Andy Estes]

Prior to r71562, Loader::servePendingRequests() would be called as part of marking a load as finished. servePendingRequests() kicks off loads for pending requests if there are available loader slots due to the completed load. r71562 removed this call (accidentally? intentionally? sadly the ChangeLog provides no clue) as part of a refactoring, instead relying on a request timer to call servePendingRequests().

However, if a modal dialog is displayed while the request timer is active, and that dialog attempts to use resources that were previously placed in the pending queue, the load will stall since the request timer can't be started reentrantly and therefore the resources will have no way to move from the pending queue to the loader.

Restoring the call to servePendingRequests() at various points (didFinishLoading(), didFail(), didReceiveResponse()) fixes this issue.

It's not entirely clear to me how to write a test using web content that captures the nature of this bug. The key to the bug is that a timer fires which calls runJavaScriptAlert(), and Adobe has an alert delegate which then re-enters WebCore. Since a timer is firing, sharedTimerFiredInternal's reentrancy guard causes no new timers (such as m_requestTimer in ResourceLoadScheduler) to fire in the nested context.

I've tried to approximate this using showModalDialog(), but that calls TimerBase::fireTimersInNestedEventLoop(), which resets the reentrancy guard and allows the timers to fire. I'm not sure what else to try short of adding a mechanism to DRT where a test can provide a custom UI delegate for runJavaScriptAlertPanelWithMessage:

[Andy Estes]

Created attachment 78631 [details]
Patch

[Darin Adler]

Comment on attachment 78631 [details]
Patch

Still worth trying to figure out if we can do a regression test for this, but that should not block checking the fix in.

[Andy Estes]

Committed r75580: <http://trac.webkit.org/changeset/75580>