Make ResourceFetcher return Resources with LoadError instead of nullptrs.

Make ResourceFetcher return Resources with LoadError instead of nullptrs.

Before this change, when a request was immediately blocked by access checks,
ResourceFetcher::requestResource() would return a nullptr, while returning a
Resource instance with a LoadError set if the request was blocked only after
following one or more redirects. This resulted in various resource types
failing in inconsistent ways in the two cases.

This CL makes this behavior consistent by making requestResource() always
return a Resource instance with a LoadError. This allows for cleaner and more
consistent handling of errors in ResourceClients, as wells as more flexibility
should the client choose to distinguish the underlying causes of errors. Due
to the infrequency of requests failing access checks during normal use, the
performance impact should be negligible.

This is a reland of https://codereview.chromium.org/2231523002/, which got
reverted due to making the following layout test flaky:

  http/tests/security/contentSecurityPolicy/nonces/import-enforce-blocked.php

The test has been updated so there is no longer a race condition.

BUG=616234
Committed: https://crrev.com/2f545df32a684b115b8b4c5458778695881b1bc6
Cr-Commit-Position: refs/heads/master@{#423978}

[engedy, 2016-10-07 19:49:57]

Patchset #1 (id:1) has been deleted

[engedy, 2016-10-07 19:50:23]

Description was changed from

==========
Make ResourceFetcher return Resources with LoadError instead of nullptrs.

Before this change, when a request was immediately blocked by access checks,
ResourceFetcher::requestResource() would return a nullptr, while returning a
Resource instance with a LoadError set if the request was blocked only after
following one or more redirects. This resulted in various resource types
failing in inconsistent ways in the two cases.

This CL makes this behavior consistent by making requestResource() always
return a Resource instance with a LoadError. This allows for cleaner and more
consistent handling of errors in ResourceClients, as wells as more flexibility
should the client choose to distinguish the underlying causes of errors. Due
to the infrequency of requests failing access checks during normal use, the
performance impact should be negligible.

This is a reland of https://codereview.chromium.org/2231523002/, which got
reverted due to making the following layout test flaky:

  http/tests/security/contentSecurityPolicy/nonces/import-enforce-blocked.php

The test has been updated so there is no longer a race condition.

BUG=616234
==========

to

==========
Make ResourceFetcher return Resources with LoadError instead of nullptrs.

Before this change, when a request was immediately blocked by access checks,
ResourceFetcher::requestResource() would return a nullptr, while returning a
Resource instance with a LoadError set if the request was blocked only after
following one or more redirects. This resulted in various resource types
failing in inconsistent ways in the two cases.

This CL makes this behavior consistent by making requestResource() always
return a Resource instance with a LoadError. This allows for cleaner and more
consistent handling of errors in ResourceClients, as wells as more flexibility
should the client choose to distinguish the underlying causes of errors. Due
to the infrequency of requests failing access checks during normal use, the
performance impact should be negligible.

This is a reland of https://codereview.chromium.org/2231523002/, which got
reverted due to making the following layout test flaky:

  http/tests/security/contentSecurityPolicy/nonces/import-enforce-blocked.php

The test has been updated so there is no longer a race condition.

BUG=616234
==========

[engedy, 2016-10-07 19:50:23]

engedy@chromium.org changed reviewers:
+ japhet@chromium.org

[engedy, 2016-10-07 19:50:53]

Nate, please take a look at the diff between Patch Sets 1 and 2.

[Nate Chapin, 2016-10-07 19:53:12]

lgtm if the bots are happy!

[engedy, 2016-10-07 19:53:18]

Do you think it would be useful to add a comment to describe the motivation, or
is it clear enough?

[engedy, 2016-10-07 19:53:47]

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

[Nate Chapin, 2016-10-07 19:53:56]

On 2016/10/07 19:53:18, engedy wrote:
> Do you think it would be useful to add a comment to describe the motivation,
or
> is it clear enough?

Hmm, yeah, a comment wouldn't hurt.

[commit-bot: I haz the power, 2016-10-07 19:54:07]

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

[engedy, 2016-10-07 20:28:28]

Patchset #3 (id:60001) has been deleted

[engedy, 2016-10-07 20:28:47]

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

[commit-bot: I haz the power, 2016-10-07 20:29:12]

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

[engedy, 2016-10-07 20:29:20]

On 2016/10/07 19:53:56, Nate Chapin wrote:
> On 2016/10/07 19:53:18, engedy wrote:
> > Do you think it would be useful to add a comment to describe the motivation,
> or
> > is it clear enough?
> 
> Hmm, yeah, a comment wouldn't hurt.

Done.

[engedy, 2016-10-07 20:33:06]

The CQ bit was unchecked by engedy@chromium.org

[engedy, 2016-10-07 20:33:17]

The CQ bit was checked by engedy@chromium.org

[engedy, 2016-10-07 20:33:18]

The patchset sent to the CQ was uploaded after l-g-t-m from japhet@chromium.org
Link to the patchset: https://codereview.chromium.org/2408443003/#ps80001
(title: "Add comment.")

[commit-bot: I haz the power, 2016-10-07 20:33:53]

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-10-07 22:06:07]

Description was changed from

==========
Make ResourceFetcher return Resources with LoadError instead of nullptrs.

Before this change, when a request was immediately blocked by access checks,
ResourceFetcher::requestResource() would return a nullptr, while returning a
Resource instance with a LoadError set if the request was blocked only after
following one or more redirects. This resulted in various resource types
failing in inconsistent ways in the two cases.

This CL makes this behavior consistent by making requestResource() always
return a Resource instance with a LoadError. This allows for cleaner and more
consistent handling of errors in ResourceClients, as wells as more flexibility
should the client choose to distinguish the underlying causes of errors. Due
to the infrequency of requests failing access checks during normal use, the
performance impact should be negligible.

This is a reland of https://codereview.chromium.org/2231523002/, which got
reverted due to making the following layout test flaky:

  http/tests/security/contentSecurityPolicy/nonces/import-enforce-blocked.php

The test has been updated so there is no longer a race condition.

BUG=616234
==========

to

==========
Make ResourceFetcher return Resources with LoadError instead of nullptrs.

Before this change, when a request was immediately blocked by access checks,
ResourceFetcher::requestResource() would return a nullptr, while returning a
Resource instance with a LoadError set if the request was blocked only after
following one or more redirects. This resulted in various resource types
failing in inconsistent ways in the two cases.

This CL makes this behavior consistent by making requestResource() always
return a Resource instance with a LoadError. This allows for cleaner and more
consistent handling of errors in ResourceClients, as wells as more flexibility
should the client choose to distinguish the underlying causes of errors. Due
to the infrequency of requests failing access checks during normal use, the
performance impact should be negligible.

This is a reland of https://codereview.chromium.org/2231523002/, which got
reverted due to making the following layout test flaky:

  http/tests/security/contentSecurityPolicy/nonces/import-enforce-blocked.php

The test has been updated so there is no longer a race condition.

BUG=616234
==========

[commit-bot: I haz the power, 2016-10-07 22:06:09]

Committed patchset #3 (id:80001)

[commit-bot: I haz the power, 2016-10-07 22:08:11]

Description was changed from

==========
Make ResourceFetcher return Resources with LoadError instead of nullptrs.

Before this change, when a request was immediately blocked by access checks,
ResourceFetcher::requestResource() would return a nullptr, while returning a
Resource instance with a LoadError set if the request was blocked only after
following one or more redirects. This resulted in various resource types
failing in inconsistent ways in the two cases.

This CL makes this behavior consistent by making requestResource() always
return a Resource instance with a LoadError. This allows for cleaner and more
consistent handling of errors in ResourceClients, as wells as more flexibility
should the client choose to distinguish the underlying causes of errors. Due
to the infrequency of requests failing access checks during normal use, the
performance impact should be negligible.

This is a reland of https://codereview.chromium.org/2231523002/, which got
reverted due to making the following layout test flaky:

  http/tests/security/contentSecurityPolicy/nonces/import-enforce-blocked.php

The test has been updated so there is no longer a race condition.

BUG=616234
==========

to

==========
Make ResourceFetcher return Resources with LoadError instead of nullptrs.

Before this change, when a request was immediately blocked by access checks,
ResourceFetcher::requestResource() would return a nullptr, while returning a
Resource instance with a LoadError set if the request was blocked only after
following one or more redirects. This resulted in various resource types
failing in inconsistent ways in the two cases.

This CL makes this behavior consistent by making requestResource() always
return a Resource instance with a LoadError. This allows for cleaner and more
consistent handling of errors in ResourceClients, as wells as more flexibility
should the client choose to distinguish the underlying causes of errors. Due
to the infrequency of requests failing access checks during normal use, the
performance impact should be negligible.

This is a reland of https://codereview.chromium.org/2231523002/, which got
reverted due to making the following layout test flaky:

  http/tests/security/contentSecurityPolicy/nonces/import-enforce-blocked.php

The test has been updated so there is no longer a race condition.

BUG=616234

Committed: https://crrev.com/2f545df32a684b115b8b4c5458778695881b1bc6
Cr-Commit-Position: refs/heads/master@{#423978}
==========

[commit-bot: I haz the power, 2016-10-07 22:08:12]

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