Make DocumentThreadableLoader's cross origin logic clearer in terms of layering

Make DocumentThreadableLoader's cross origin logic clearer in terms of layering

- Call removeCredentials() on ResourceRequest before invoking the
  CrossOriginPreflightResultCache's canSkipPreflight to determine the
  final request to send after possible preflight. But generate the
  referrer and origin header right before loadRequest(). It looks this
  would fix a possible bug that canSkipPreflight() always returns false
  due to setHTTPReferrer() call when m_didRedirect is true.
- Rename m_didRedirect to m_overrideReferrer as it's set only when the
  redirect was cross origin. Not always.
- Eliminate the unnecessary loadActualRequest() invocation on the path
  where we don't issue a preflight.
- Let createAccessControlPreflightRequest() just DCHECK() on the passed
  URL instead of calling removeCredentials() separately. It's better to
  just depend on the input |request| as the role of the preflight is to
  check that the |request| can be accepted by the server.

R=mkwst@chromium.org
BUG=427429, 641620, 557621
Committed: https://crrev.com/5eba3e9043b40c4a0b051123a364f2c97dfdf088
Cr-Commit-Position: refs/heads/master@{#430543}

[tyoshino (SeeGerritForStatus), 2016-10-13 14:21:40]

The CQ bit was checked by tyoshino@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-13 14:22:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tyoshino (SeeGerritForStatus), 2016-10-13 14:36:02]

Description was changed from

==========
aaaa

BUG=427429,641620,557621
==========

to

==========
Make DocumentThreadableLoader's cross origin logic clearer in terms of layering

- Call removeCredentials() on ResourceRequest before invoking the
  CrossOriginPreflightResultCache's canSkipPreflight to determine the
  final request to send after possible preflight. But generate the
  referrer and origin header right before loadRequest(). It looks this
  would fix a possible bug that canSkipPreflight() always returns false
  due to setHTTPReferrer() call when m_didRedirect is true.
- Rename m_didRedirect to m_overrideReferrer as it's set only when the
  redirect was cross origin. Not always.
- Eliminate the unnecessary loadActualRequest() invocation on the path
  where we don't issue a preflight.
- Let createAccessControlPreflightRequest() just DCHECK() on the passed
  URL instead of calling removeCredentials() separately. It's better to
  just depend on the input |request| as the role of the preflight is to
  check that the |request| can be accepted by the server.


BUG=427429,641620,557621
==========

[tyoshino (SeeGerritForStatus), 2016-10-13 14:36:13]

Description was changed from

==========
Make DocumentThreadableLoader's cross origin logic clearer in terms of layering

- Call removeCredentials() on ResourceRequest before invoking the
  CrossOriginPreflightResultCache's canSkipPreflight to determine the
  final request to send after possible preflight. But generate the
  referrer and origin header right before loadRequest(). It looks this
  would fix a possible bug that canSkipPreflight() always returns false
  due to setHTTPReferrer() call when m_didRedirect is true.
- Rename m_didRedirect to m_overrideReferrer as it's set only when the
  redirect was cross origin. Not always.
- Eliminate the unnecessary loadActualRequest() invocation on the path
  where we don't issue a preflight.
- Let createAccessControlPreflightRequest() just DCHECK() on the passed
  URL instead of calling removeCredentials() separately. It's better to
  just depend on the input |request| as the role of the preflight is to
  check that the |request| can be accepted by the server.


BUG=427429,641620,557621
==========

to

==========
Make DocumentThreadableLoader's cross origin logic clearer in terms of layering

- Call removeCredentials() on ResourceRequest before invoking the
  CrossOriginPreflightResultCache's canSkipPreflight to determine the
  final request to send after possible preflight. But generate the
  referrer and origin header right before loadRequest(). It looks this
  would fix a possible bug that canSkipPreflight() always returns false
  due to setHTTPReferrer() call when m_didRedirect is true.
- Rename m_didRedirect to m_overrideReferrer as it's set only when the
  redirect was cross origin. Not always.
- Eliminate the unnecessary loadActualRequest() invocation on the path
  where we don't issue a preflight.
- Let createAccessControlPreflightRequest() just DCHECK() on the passed
  URL instead of calling removeCredentials() separately. It's better to
  just depend on the input |request| as the role of the preflight is to
  check that the |request| can be accepted by the server.


BUG=427429,641620,557621
==========

[tyoshino (SeeGerritForStatus), 2016-10-13 15:36:09]

tyoshino@chromium.org changed reviewers:
+ mkwst@chromium.org

[commit-bot: I haz the power, 2016-10-13 15:37:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-13 15:37:31]

Dry run: This issue passed the CQ dry run.

[Mike West, 2016-10-26 16:26:57]

This looks clearer to me. Thanks, and apologies for the delayed review. I missed
this entirely. :(

LGTM!

[tyoshino (SeeGerritForStatus), 2016-11-08 06:02:08]

On 2016/10/26 16:26:57, Mike West wrote:
> This looks clearer to me. Thanks, and apologies for the delayed review. I
missed
> this entirely. :(
> 
> LGTM!

Thank you! No problem. I also haven't got time to land this for 1.5w :)

[tyoshino (SeeGerritForStatus), 2016-11-08 06:02:36]

Description was changed from

==========
Make DocumentThreadableLoader's cross origin logic clearer in terms of layering

- Call removeCredentials() on ResourceRequest before invoking the
  CrossOriginPreflightResultCache's canSkipPreflight to determine the
  final request to send after possible preflight. But generate the
  referrer and origin header right before loadRequest(). It looks this
  would fix a possible bug that canSkipPreflight() always returns false
  due to setHTTPReferrer() call when m_didRedirect is true.
- Rename m_didRedirect to m_overrideReferrer as it's set only when the
  redirect was cross origin. Not always.
- Eliminate the unnecessary loadActualRequest() invocation on the path
  where we don't issue a preflight.
- Let createAccessControlPreflightRequest() just DCHECK() on the passed
  URL instead of calling removeCredentials() separately. It's better to
  just depend on the input |request| as the role of the preflight is to
  check that the |request| can be accepted by the server.


BUG=427429,641620,557621
==========

to

==========
Make DocumentThreadableLoader's cross origin logic clearer in terms of layering

- Call removeCredentials() on ResourceRequest before invoking the
  CrossOriginPreflightResultCache's canSkipPreflight to determine the
  final request to send after possible preflight. But generate the
  referrer and origin header right before loadRequest(). It looks this
  would fix a possible bug that canSkipPreflight() always returns false
  due to setHTTPReferrer() call when m_didRedirect is true.
- Rename m_didRedirect to m_overrideReferrer as it's set only when the
  redirect was cross origin. Not always.
- Eliminate the unnecessary loadActualRequest() invocation on the path
  where we don't issue a preflight.
- Let createAccessControlPreflightRequest() just DCHECK() on the passed
  URL instead of calling removeCredentials() separately. It's better to
  just depend on the input |request| as the role of the preflight is to
  check that the |request| can be accepted by the server.

R=mkwst@chromium.org
BUG=427429,641620,557621
==========

[tyoshino (SeeGerritForStatus), 2016-11-08 06:02:49]

The CQ bit was checked by tyoshino@chromium.org

[tyoshino (SeeGerritForStatus), 2016-11-08 06:02:50]

The patchset sent to the CQ was uploaded after l-g-t-m from mkwst@chromium.org
Link to the patchset: https://codereview.chromium.org/2420603003/#ps20001
(title: "Rebase")

[commit-bot: I haz the power, 2016-11-08 06:03:01]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-08 07:56:32]

Description was changed from

==========
Make DocumentThreadableLoader's cross origin logic clearer in terms of layering

- Call removeCredentials() on ResourceRequest before invoking the
  CrossOriginPreflightResultCache's canSkipPreflight to determine the
  final request to send after possible preflight. But generate the
  referrer and origin header right before loadRequest(). It looks this
  would fix a possible bug that canSkipPreflight() always returns false
  due to setHTTPReferrer() call when m_didRedirect is true.
- Rename m_didRedirect to m_overrideReferrer as it's set only when the
  redirect was cross origin. Not always.
- Eliminate the unnecessary loadActualRequest() invocation on the path
  where we don't issue a preflight.
- Let createAccessControlPreflightRequest() just DCHECK() on the passed
  URL instead of calling removeCredentials() separately. It's better to
  just depend on the input |request| as the role of the preflight is to
  check that the |request| can be accepted by the server.

R=mkwst@chromium.org
BUG=427429,641620,557621
==========

to

==========
Make DocumentThreadableLoader's cross origin logic clearer in terms of layering

- Call removeCredentials() on ResourceRequest before invoking the
  CrossOriginPreflightResultCache's canSkipPreflight to determine the
  final request to send after possible preflight. But generate the
  referrer and origin header right before loadRequest(). It looks this
  would fix a possible bug that canSkipPreflight() always returns false
  due to setHTTPReferrer() call when m_didRedirect is true.
- Rename m_didRedirect to m_overrideReferrer as it's set only when the
  redirect was cross origin. Not always.
- Eliminate the unnecessary loadActualRequest() invocation on the path
  where we don't issue a preflight.
- Let createAccessControlPreflightRequest() just DCHECK() on the passed
  URL instead of calling removeCredentials() separately. It's better to
  just depend on the input |request| as the role of the preflight is to
  check that the |request| can be accepted by the server.

R=mkwst@chromium.org
BUG=427429,641620,557621
==========

[commit-bot: I haz the power, 2016-11-08 07:56:33]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-11-08 07:59:24]

Description was changed from

==========
Make DocumentThreadableLoader's cross origin logic clearer in terms of layering

- Call removeCredentials() on ResourceRequest before invoking the
  CrossOriginPreflightResultCache's canSkipPreflight to determine the
  final request to send after possible preflight. But generate the
  referrer and origin header right before loadRequest(). It looks this
  would fix a possible bug that canSkipPreflight() always returns false
  due to setHTTPReferrer() call when m_didRedirect is true.
- Rename m_didRedirect to m_overrideReferrer as it's set only when the
  redirect was cross origin. Not always.
- Eliminate the unnecessary loadActualRequest() invocation on the path
  where we don't issue a preflight.
- Let createAccessControlPreflightRequest() just DCHECK() on the passed
  URL instead of calling removeCredentials() separately. It's better to
  just depend on the input |request| as the role of the preflight is to
  check that the |request| can be accepted by the server.

R=mkwst@chromium.org
BUG=427429,641620,557621
==========

to

==========
Make DocumentThreadableLoader's cross origin logic clearer in terms of layering

- Call removeCredentials() on ResourceRequest before invoking the
  CrossOriginPreflightResultCache's canSkipPreflight to determine the
  final request to send after possible preflight. But generate the
  referrer and origin header right before loadRequest(). It looks this
  would fix a possible bug that canSkipPreflight() always returns false
  due to setHTTPReferrer() call when m_didRedirect is true.
- Rename m_didRedirect to m_overrideReferrer as it's set only when the
  redirect was cross origin. Not always.
- Eliminate the unnecessary loadActualRequest() invocation on the path
  where we don't issue a preflight.
- Let createAccessControlPreflightRequest() just DCHECK() on the passed
  URL instead of calling removeCredentials() separately. It's better to
  just depend on the input |request| as the role of the preflight is to
  check that the |request| can be accepted by the server.

R=mkwst@chromium.org
BUG=427429,641620,557621

Committed: https://crrev.com/5eba3e9043b40c4a0b051123a364f2c97dfdf088
Cr-Commit-Position: refs/heads/master@{#430543}
==========

[commit-bot: I haz the power, 2016-11-08 07:59:25]

Patchset 2 (id:??) landed as
https://crrev.com/5eba3e9043b40c4a0b051123a364f2c97dfdf088
Cr-Commit-Position: refs/heads/master@{#430543}