ResourceLoaderOptions also must be updated by updateRequestForAccessControl()

ResourceLoaderOptions also must be updated by updateRequestForAccessControl()

updateRequestForAccessControl() updates ResourceRequest for access
control, but ResourceLoaderOptions passed to FetchRequest also contains
configuration items that must be updated for access control.

This CL removes cookie handling code and iframe from the following
layout tests since they're testing user:pass but not cookie.
- access-control-preflight-credential-async.html
- access-control-preflight-credential-sync.html

A new test is added to check that no cookie is set in a preflight
request. This new test uses third-party-cookie-relaxing-iframe.html
which was loaded by the two tests but was not actually used.

Missing testRunner.setAlwaysAcceptCookies() calls are added to
third-party-cookie-relaxing-iframe.html.

BUG=377541
R=japhet

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=175527

[Nate Chapin, 2014-06-03 16:24:55]

LGTM

https://codereview.chromium.org/312653002/diff/170001/Source/core/loader/Docu...
File Source/core/loader/DocumentThreadableLoader.cpp (right):

https://codereview.chromium.org/312653002/diff/170001/Source/core/loader/Docu...
Source/core/loader/DocumentThreadableLoader.cpp:139: ResourceLoaderOptions
preflightOptions = *m_actualOptions;
m_acutalOptions.get() is how we usually dereference an OwnPtr, but I guess this
is consistent with the rest of the file.

https://codereview.chromium.org/312653002/diff/170001/Source/core/loader/Docu...
Source/core/loader/DocumentThreadableLoader.cpp:414:
resourceLoaderOptions.allowCredentials = DoNotAllowStoredCredentials;
I take it there are cases where m_allowCredentials will allow credenetials, but
resourceLoaderOptions.allowCredentials will not? When exactly is this the case?

[tyoshino (SeeGerritForStatus), 2014-06-04 05:50:35]

PTAL

https://codereview.chromium.org/312653002/diff/170001/Source/core/loader/Docu...
File Source/core/loader/DocumentThreadableLoader.cpp (right):

https://codereview.chromium.org/312653002/diff/170001/Source/core/loader/Docu...
Source/core/loader/DocumentThreadableLoader.cpp:139: ResourceLoaderOptions
preflightOptions = *m_actualOptions;
On 2014/06/03 16:24:56, Nate Chapin wrote:
> m_acutalOptions.get() is how we usually dereference an OwnPtr, but I guess
this
> is consistent with the rest of the file.

Oh, ok. Fixed the rest too instead.

https://codereview.chromium.org/312653002/diff/170001/Source/core/loader/Docu...
Source/core/loader/DocumentThreadableLoader.cpp:414:
resourceLoaderOptions.allowCredentials = DoNotAllowStoredCredentials;
On 2014/06/03 16:24:56, Nate Chapin wrote:
> I take it there are cases where m_allowCredentials will allow credenetials,
but
> resourceLoaderOptions.allowCredentials will not? When exactly is this the
case?

This code need to be like this because for preflight request we set
resourceLoaderOptions to DoNot and we don't want it to be overridden by
m_allowCredentials here.

m_allowCredentials is only set to DoNotAllowStoredCredentials at L233 when
redirect happens. Otherwise, it's the same as the original allowCredentials
value set in m_resourceLoaderOptions.

I replaced m_allowCredentials with a boolean named
"forceDoNotAllowStoredCredentials" to make it clear.

[Nate Chapin, 2014-06-04 16:08:20]

LGTM with an apology

https://codereview.chromium.org/312653002/diff/190001/Source/core/loader/Docu...
File Source/core/loader/DocumentThreadableLoader.cpp (right):

https://codereview.chromium.org/312653002/diff/190001/Source/core/loader/Docu...
Source/core/loader/DocumentThreadableLoader.cpp:392:
loadRequest(*actualRequest.get(), *actualOptions.get());
I think I lead you astray :(

The cases were we use .get() with OwnPtrs are usually cases where we want a raw
pointer, not a reference. The *actualRequest method of getting a reference is
probably actually the right way.

The real weird thing is that we're OwnPtr'ing classes that usually aren't
handled as pointers. So maybe these should return to the old way.

[Nate Chapin, 2014-06-04 16:08:21]

LGTM with an apology

[tyoshino (SeeGerritForStatus), 2014-06-05 02:10:49]

https://codereview.chromium.org/312653002/diff/190001/Source/core/loader/Docu...
File Source/core/loader/DocumentThreadableLoader.cpp (right):

https://codereview.chromium.org/312653002/diff/190001/Source/core/loader/Docu...
Source/core/loader/DocumentThreadableLoader.cpp:392:
loadRequest(*actualRequest.get(), *actualOptions.get());
On 2014/06/04 16:08:21, Nate Chapin wrote:
> I think I lead you astray :(
> 
> The cases were we use .get() with OwnPtrs are usually cases where we want a
raw
> pointer, not a reference. The *actualRequest method of getting a reference is
> probably actually the right way.
> 

Ah, ok. Reverted.

> The real weird thing is that we're OwnPtr'ing classes that usually aren't
> handled as pointers. So maybe these should return to the old way.

Yeah. Looks over optimized :)

[tyoshino (SeeGerritForStatus), 2014-06-05 02:12:13]

The CQ bit was checked by tyoshino@chromium.org

[tyoshino (SeeGerritForStatus), 2014-06-05 02:12:41]

The CQ bit was unchecked by tyoshino@chromium.org

[tyoshino (SeeGerritForStatus), 2014-06-05 02:13:02]

The CQ bit was checked by tyoshino@chromium.org

[commit-bot: I haz the power, 2014-06-05 02:13:59]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tyoshino@chromium.org/312653002/210001

[commit-bot: I haz the power, 2014-06-05 03:17:41]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/10700)

[commit-bot: I haz the power, 2014-06-05 04:46:16]

Change committed as 175527