Bug 52264 - REGRESSION (r71562): servePendingRequests() no longer called when resources are done loading.
Summary: REGRESSION (r71562): servePendingRequests() no longer called when resources a...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Andy Estes
URL:
Keywords: InRadar, Regression
Depends on: 27165
Blocks:
  Show dependency treegraph
 
Reported: 2011-01-11 17:23 PST by Andy Estes
Modified: 2011-01-11 17:57 PST (History)
0 users

See Also:


Attachments
Patch (1.80 KB, patch)
2011-01-11 17:43 PST, Andy Estes
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andy Estes 2011-01-11 17:23:04 PST
REGRESSION (r71562): servePendingRequests() no longer called when resources are done loading.
Comment 1 Andy Estes 2011-01-11 17:38:45 PST
<rdar://problem/8767429>
Comment 2 Andy Estes 2011-01-11 17:40:06 PST
The removal of servePendingRequests() is what caused the Adobe Creative Suite 4 installer to break on Mac OS X.
Comment 3 Andy Estes 2011-01-11 17:41:02 PST
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:
Comment 4 Andy Estes 2011-01-11 17:43:42 PST
Created attachment 78631 [details]
Patch
Comment 5 Darin Adler 2011-01-11 17:50:30 PST
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.
Comment 6 Andy Estes 2011-01-11 17:57:58 PST
Committed r75580: <http://trac.webkit.org/changeset/75580>