Bug 105044 – REGRESSION (r137607): Cannot download files, stuck in “Preparing to download”

• Component: Page Loading
• Status: RESOLVED FIXED

[Alexey Proskuryakov]

Downloads no longer work in Safari, and probably in other WebKit based browsers too.

Steps to reproduce: go to nightlies.webkit.org, click on a link to download a nightly.

<rdar://problem/12239483>

[Alexey Proskuryakov]

What happens here is that we are getting a ResourceHandle::cancel() call now.

Anders is going to address this symptom by making ResourceHandle::cancel() a no-op once download has been started, but the change in ResourceLoader behavior is unexpected, and possibly wrong.

[Anders Carlsson]

*** Bug 105050 has been marked as a duplicate of this bug. ***

[Anders Carlsson]

Created attachment 179515 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 179515 [details]
Patch

Please keep the bug open for Nate to investigate.

[Alexey Proskuryakov]

*** Bug 105033 has been marked as a duplicate of this bug. ***

[Anders Carlsson]

Committed r137763: <http://trac.webkit.org/changeset/137763>

[Thiago Marcos P. Santos]

(In reply to comment #6)
> Committed r137763: <http://trac.webkit.org/changeset/137763>

Do you have a webkit-wise fix for this? I'm having the same issue on EFL since r137607.

[Nate Chapin]

Created attachment 179748 [details]
Fix the regression

It appears that in r137607 we call DocumentLoader::mainReceivedError(), which now calls MainResourceLoader::cancel(), which cancels the CachedResource load, which cancels the ResourceLoader, which cancels the ResourceHandle.

Calling directly into ResourceLoader::didFail() instead is a bit of a layering violation, but it emulates the old behavior quite well.  Strictly speaking, it isn't necessary to make downloads work, but if we don't, the loader objects (ResourceLoader, CachedResource, MainResourceLoader) will be left alive but useless until the associated DocumentLoader is detached.

[Sergio Villar Senin]

(In reply to comment #8)
> Created an attachment (id=179748) [details]
> Fix the regression
> 
> It appears that in r137607 we call DocumentLoader::mainReceivedError(), which now calls MainResourceLoader::cancel(), which cancels the CachedResource load, which cancels the ResourceLoader, which cancels the ResourceHandle.
> 
> Calling directly into ResourceLoader::didFail() instead is a bit of a layering violation, but it emulates the old behavior quite well.  Strictly speaking, it isn't necessary to make downloads work, but if we don't, the loader objects (ResourceLoader, CachedResource, MainResourceLoader) will be left alive but useless until the associated DocumentLoader is detached.

LGTM, BTW doesn't this regression mean that we lack some test?

[WebKit Review Bot]

Comment on attachment 179748 [details]
Fix the regression

Clearing flags on attachment: 179748

Committed r138012: <http://trac.webkit.org/changeset/138012>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.