Some minor simplifications can be made to some of the referrer/origin code in FrameLoader -- particularly in the urlSelected() and loadFrameRequest() calls.
I have a question. In this code-- http://trac.webkit.org/browser/trunk/WebCore/loader/FrameLoader.cpp?rev=59829#L343 Does the call to addHTTPOriginIfNeeded() in line 376 seem superfluous to you? > 372 if (referrerPolicy == NoReferrer) > 373 m_suppressOpenerInNewFrame = true; > 374 else if (frameRequest.resourceRequest().httpReferrer().isEmpty()) > 375 frameRequest.resourceRequest().setHTTPReferrer(m_outgoingReferrer); > 376 addHTTPOriginIfNeeded(frameRequest.resourceRequest(), outgoingOrigin()); The reason is that the private urlSelected(changeLocation(const KURL& url,const ResourceRequest& request, ...) overload only gets called by changeLocation(const KURL& url, ...) and urlSelected(const KURL& url, ...) above it. You can see in both of those methods that the ResourceRequest is getting constructed for the first time. Since ResourceRequest initially has httpMethod "GET" when it's constructed, the call to addHTTPOriginIfNeeded() won't do anything (it only does something if the method is not "GET" or "HEAD"). If this analysis is right, it seems odd that the outgoingOrigin() expression was in there in the first place. FYI, this recent simple refactoring is what made this much easier to see: http://trac.webkit.org/changeset/59829 (tightening the FrameLoader API by making urlSelected() publicly accept only KURL instead of ResourceRequest)
(In reply to comment #1) > The reason is that the private urlSelected(changeLocation(const KURL& url,const ResourceRequest& request, ...) overload only gets called by changeLocation(const KURL& url, ...) and urlSelected(const KURL& url, ...) above it. Paste-miss. That should have been-- The reason is that the private urlSelected(const ResourceRequest& request, ...) overload only gets called by changeLocation(const KURL& url, ...) and urlSelected(const KURL& url, ...) above it.
It's the job of addHTTPOriginIfNeeded to know that Origin doesn't get attached to GET requests. In principle, the callers of that method aren't supposed to know that.
Created attachment 56722 [details] Proposed patch
Comment on attachment 56722 [details] Proposed patch Mostly questions. Thanks for digging into this class. WebCore/loader/FrameLoader.cpp:1781 + void FrameLoader::loadFrameRequest(const ResourceRequest& request, const String& frameName, bool lockHistory, bool lockBackForwardList, PassRefPtr<Event> event, PassRefPtr<FormState> formState, ReferrerPolicy referrerPolicy, String origin) Strange that we use the name |request| for both FrameLoadRequest and ResourceRequest. Is that what we do elsewhere? WebCore/loader/FrameLoader.cpp:1778 + loadFrameRequest(request.resourceRequest(), request.frameName(), lockHistory, lockBackForwardList, event, formState, referrerPolicy, String()); The last arg here is somewhat unfortunate. WebCore/loader/FrameLoader.cpp:3211 + // The inRequest's httpReferrer() is not examined by this method. The newReferrer Crazy WebCore/loader/FrameLoader.h:385 + void addExtraFieldsToRequest(ResourceRequest&, FrameLoadType loadType, bool isMainResource, bool cookiePolicyURLFromRequest, String httpOrigin); Why httpOrigin instead of origin?
Comment on attachment 56722 [details] Proposed patch Sadly cjerdonek appears to no longer be contributing to the project. Hopefully he'll return and finish this patch.