Prevents sending of 'orgin' in the "Access-Control-Request-Headers" when preflight requests are mad…

Prevents sending of 'orgin' in the "Access-Control-Request-Headers" when preflight requests are made.

As per the W3C CORS Spec(http://www.w3.org/TR/cors/#cross-origin-request-with-preflight-0), 
the "Access-Control-Request-Headers" should contain only author request headers. Origin header was
set(by UA) for the actual request before the preflight request was done. 

Avoid setting of Origin header for the actual request in case of preflight. Origin header is anyways
set for the actual request in preflightSuccess(). This makes the behavior same as Mozilla Firefox browser.

R=abarth@chromium.org, kbr@chromium.org, bbudge@chromium.org
BUG=258828
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=154346

[ancilgeorge, 2013-07-12 11:38:47]

Please review.

[bbudge, 2013-07-12 19:10:11]

https://codereview.chromium.org/18595008/diff/1/Source/core/loader/CrossOrigi...
File Source/core/loader/CrossOriginAccessControl.cpp (right):

https://codereview.chromium.org/18595008/diff/1/Source/core/loader/CrossOrigi...
Source/core/loader/CrossOriginAccessControl.cpp:105: 
Why not allow a NULL securityOrigin parameter in the existing function?

[ancilgeorge, 2013-07-15 06:26:28]

On 2013/07/12 19:10:11, bbudge1 wrote:
>
https://codereview.chromium.org/18595008/diff/1/Source/core/loader/CrossOrigi...
> File Source/core/loader/CrossOriginAccessControl.cpp (right):
> 
>
https://codereview.chromium.org/18595008/diff/1/Source/core/loader/CrossOrigi...
> Source/core/loader/CrossOriginAccessControl.cpp:105: 
> Why not allow a NULL securityOrigin parameter in the existing function?

Thanks for the quick review.

Initially I had though of using function with name
updateRequestForAccessControlForPreflight() for better readability.
Then change to current implementation. My understanding was that it was not
recommended to pass NULL parameter.
I will change as per your suggestion and re-submit the patch.

[ancilgeorge, 2013-07-15 15:32:46]

https://codereview.chromium.org/18595008/diff/1/Source/core/loader/CrossOrigi...
File Source/core/loader/CrossOriginAccessControl.cpp (right):

https://codereview.chromium.org/18595008/diff/1/Source/core/loader/CrossOrigi...
Source/core/loader/CrossOriginAccessControl.cpp:105: 
On 2013/07/12 19:10:11, bbudge1 wrote:
> Why not allow a NULL securityOrigin parameter in the existing function?

Done. Could you please review the changes.

[bbudge, 2013-07-15 17:00:25]

Just FYI, I have worked *some* in this area but am not an OWNER.

https://codereview.chromium.org/18595008/diff/6001/Source/core/loader/CrossOr...
File Source/core/loader/CrossOriginAccessControl.h (right):

https://codereview.chromium.org/18595008/diff/6001/Source/core/loader/CrossOr...
Source/core/loader/CrossOriginAccessControl.h:48: void
updateRequestForAccessControl(ResourceRequest&, StoredCredentials,
SecurityOrigin* = 0);
If you don't supply a default parameter value, you can leave the ordering
unchanged. I think I prefer the original parameter ordering too.

Then you will have to explicitly pass a NULL parameter at the call site where
you want to prevent sending the origin. I think that will help make the code
clearer too.

[ancilgeorge, 2013-07-16 06:26:13]

On 2013/07/15 17:00:25, bbudge1 wrote:
> Just FYI, I have worked *some* in this area but am not an OWNER.
> 
Thanks for the review. Would request abarth to review. Your inputs were helpful.

abarth, Could you please review this.

>
https://codereview.chromium.org/18595008/diff/6001/Source/core/loader/CrossOr...
> File Source/core/loader/CrossOriginAccessControl.h (right):
> 
>
https://codereview.chromium.org/18595008/diff/6001/Source/core/loader/CrossOr...
> Source/core/loader/CrossOriginAccessControl.h:48: void
> updateRequestForAccessControl(ResourceRequest&, StoredCredentials,
> SecurityOrigin* = 0);
> If you don't supply a default parameter value, you can leave the ordering
> unchanged. I think I prefer the original parameter ordering too.
> 
> Then you will have to explicitly pass a NULL parameter at the call site where
> you want to prevent sending the origin. I think that will help make the code
> clearer too.

[ancilgeorge, 2013-07-16 06:26:27]

https://codereview.chromium.org/18595008/diff/6001/Source/core/loader/CrossOr...
File Source/core/loader/CrossOriginAccessControl.h (right):

https://codereview.chromium.org/18595008/diff/6001/Source/core/loader/CrossOr...
Source/core/loader/CrossOriginAccessControl.h:48: void
updateRequestForAccessControl(ResourceRequest&, StoredCredentials,
SecurityOrigin* = 0);
On 2013/07/15 17:00:25, bbudge1 wrote:
> If you don't supply a default parameter value, you can leave the ordering
> unchanged. I think I prefer the original parameter ordering too.
> 
> Then you will have to explicitly pass a NULL parameter at the call site where
> you want to prevent sending the origin. I think that will help make the code
> clearer too.

Done.

[abarth-chromium, 2013-07-16 06:28:20]

https://codereview.chromium.org/18595008/diff/12001/Source/core/loader/Docume...
File Source/core/loader/DocumentThreadableLoader.cpp (right):

https://codereview.chromium.org/18595008/diff/12001/Source/core/loader/Docume...
Source/core/loader/DocumentThreadableLoader.cpp:109:
updateRequestForAccessControl(*crossOriginRequest,
static_cast<SecurityOrigin*>(0), m_options.allowCredentials);
You shouldn't need the static_cast here.

[abarth-chromium, 2013-07-16 06:29:33]

I had trouble reviewing this CL because the description didn't link to the part
of the spec that defines this behavior.  It would have also been helpful if the
description said whether our new behavior matches other browsers.

[ancilgeorge, 2013-07-16 07:05:01]

On 2013/07/16 06:29:33, abarth wrote:
> I had trouble reviewing this CL because the description didn't link to the
part
> of the spec that defines this behavior.  It would have also been helpful if
the
> description said whether our new behavior matches other browsers.

Done. Please let me know if any changes are required.

https://codereview.chromium.org/18595008/diff/12001/Source/core/loader/Docume...
File Source/core/loader/DocumentThreadableLoader.cpp (right):

https://codereview.chromium.org/18595008/diff/12001/Source/core/loader/Docume...
Source/core/loader/DocumentThreadableLoader.cpp:109:
updateRequestForAccessControl(*crossOriginRequest,
static_cast<SecurityOrigin*>(0), m_options.allowCredentials);
On 2013/07/16 06:28:20, abarth wrote:
> You shouldn't need the static_cast here.

Done.

[abarth-chromium, 2013-07-16 22:05:06]

lgtm

Thanks for updating the description.  The change itself looks good.

[commit-bot: I haz the power, 2013-07-16 22:05:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ancilgeorge@samsung.com/18595008/19001

[commit-bot: I haz the power, 2013-07-17 01:41:44]

Change committed as 154346