Bug 62806 - Set referrer sooner on a ResourceRequest from a worker
Summary: Set referrer sooner on a ResourceRequest from a worker
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nate Chapin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-16 11:31 PDT by Nate Chapin
Modified: 2011-06-16 16:45 PDT (History)
4 users (show)

See Also:


Attachments
patch (12.25 KB, patch)
2011-06-16 11:34 PDT, Nate Chapin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nate Chapin 2011-06-16 11:31:39 PDT
Currently, we have an optionalOutgoingReferrer parameter on several functions to allow a WorkerThreadableLoader to override the referrer of its requests.  

I think we should be able to call setHTTPReferrer() on the ResourceRequest as soon as we are in the main thread and pass the overriding referrer that way, meaning one less optional parameter in a few places.

Patch coming shortly.
Comment 1 Nate Chapin 2011-06-16 11:34:30 PDT
Created attachment 97471 [details]
patch
Comment 2 Alexey Proskuryakov 2011-06-16 13:22:10 PDT
Comment on attachment 97471 [details]
patch

I can't follow how Referer setting logic works with this patch. For example, will we be getting https workers' URLs sent to the server in this field now? Or was it a problem even before this patch?
Comment 3 Nate Chapin 2011-06-16 13:26:24 PDT
(In reply to comment #2)
> (From update of attachment 97471 [details])
> I can't follow how Referer setting logic works with this patch. For example, will we be getting https workers' URLs sent to the server in this field now? Or was it a problem even before this patch?

The diff cut off at a lousy place.  The next line in SubresourceLoader.cpp should enforce referrer blocking regardless of whether we're using the document's outgoingReferrer of the worker's override: http://trac.webkit.org/browser/trunk/Source/WebCore/loader/SubresourceLoader.cpp#L94
Comment 4 Alexey Proskuryakov 2011-06-16 13:34:41 PDT
Looks good to me. David, do you remember if there was a reason why we didn't implement it this way originally?
Comment 5 David Levin 2011-06-16 15:27:13 PDT
(In reply to comment #4)
> Looks good to me. David, do you remember if there was a reason why we didn't implement it this way originally?

In short, it was a bad oversight on my part.

As best I can remember, my mental rough outline was to add it into ResourceRequest but was discouraged from adding more there.  Unfortunately (for no good reason), I must not have checked to see that it was already there. 

This is much better.
Comment 6 WebKit Review Bot 2011-06-16 16:45:28 PDT
Comment on attachment 97471 [details]
patch

Clearing flags on attachment: 97471

Committed r89086: <http://trac.webkit.org/changeset/89086>
Comment 7 WebKit Review Bot 2011-06-16 16:45:33 PDT
All reviewed patches have been landed.  Closing bug.