Bug 73358 – Download page URL should be set by WebCore

• Status: RESOLVED
• Resolution: FIXED
• Component: Page Loading

[Alexey Proskuryakov]

When a file is downloaded, we need to tell NSURLDownload where it came from to have correct quarantine metadata.

Currently, this is done in WebKit, which has several downsides:
- code duplication;
- need to use an SPI;
- it's difficult to pass this data over to WebKit2 UI process.

It would be much easier to just set request's main document URL - CFNetwork uses that for quarantine.

[Alexey Proskuryakov]

Created attachment 117036 [details]
proposed patch

[Adam Barth]

Comment on attachment 117036 [details]
proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=117036&action=review

> Source/WebCore/ChangeLog:8
> +        No change in behavior, just refactoring.

Even on non-Mac platforms?

[Alexey Proskuryakov]

Yes. Do you think that it changes behavior on other platforms?

[Adam Barth]

(In reply to comment #3)
> Yes. Do you think that it changes behavior on other platforms?

I haven't looked at it closely.  I just noticed that the code only existed previously on Mac.

The code itself looks pretty dodgy.  For example:

hostOnlyURLString = makeString(originalURL.protocol(), "://", originalURL.host(), ":", String::number(port));

is making a lot of assumptions about the format of the URL.

[Alexey Proskuryakov]

> I just noticed that the code only existed previously on Mac.

Chromium will likely want to use it too if it wants to set file quarantine data on Mac. It has no use for other platforms AFAICT, but also negligible cost.

> The code itself looks pretty dodgy.

Agreed - that's why I added many FIXME comments when moving it from WebKit(s) to WebCore. In this particular case, we shouldn't be constructing a host only URL string in the first place, but it's better than nothing.

[Adam Barth]

Thanks for explaining!

[WebKit Review Bot]

Comment on attachment 117036 [details]
proposed patch

Clearing flags on attachment: 117036

Committed r101486: <http://trac.webkit.org/changeset/101486>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.