CrossSiteResourceHandler: cancel request if the RFH is gone

CrossSiteResourceHandler: cancel request if there's no RFH to do
a policy check against.

Also included is a new test which demonstrates the issue. This was based
on CrossSiteTransferTest, which I did some cleanup on.

BUG=538784, 268640
TEST=content_browsertests, with and without --enable-browser-side-navigation

Committed: https://crrev.com/dc67237b8beb43a3389c38861c9c153cbb374a91
Cr-Commit-Position: refs/heads/master@{#352413}

[ncarter (slow), 2015-10-02 22:20:38]

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

[ncarter (slow), 2015-10-02 22:20:58]

nick@chromium.org changed reviewers:
+ nasko@chromium.org

[ncarter (slow), 2015-10-02 22:20:59]

Nasko, please review.

[commit-bot: I haz the power, 2015-10-02 22:21:09]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1384113002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1384113002/60001

[commit-bot: I haz the power, 2015-10-02 22:37:47]

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

[commit-bot: I haz the power, 2015-10-02 22:37:47]

Dry run: Try jobs failed on following builders:
  cast_shell_android on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_andr...)
  cast_shell_linux on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[nasko, 2015-10-05 18:01:55]

Looks good, just some nits.

https://codereview.chromium.org/1384113002/diff/60001/content/browser/loader/...
File content/browser/loader/cross_site_resource_handler_browsertest.cc (right):

https://codereview.chromium.org/1384113002/diff/60001/content/browser/loader/...
content/browser/loader/cross_site_resource_handler_browsertest.cc:49: // Expect
only a single request for the tracked url.
nit: Empty line between code and new comment line.

https://codereview.chromium.org/1384113002/diff/60001/content/browser/loader/...
content/browser/loader/cross_site_resource_handler_browsertest.cc:50:
ASSERT_FALSE(throttle_created_);
I don't see how throttle_created_ can actually become true. It is initialized to
false in the constructor and set to false in SetTrackedURLOnIOThread.

https://codereview.chromium.org/1384113002/diff/60001/content/browser/loader/...
content/browser/loader/cross_site_resource_handler_browsertest.cc:91: class
DeferStartForTestCallback : public ResourceThrottle {
Shouldn't this class bear some mention of Throttle on its name?

https://codereview.chromium.org/1384113002/diff/60001/content/browser/loader/...
content/browser/loader/cross_site_resource_handler_browsertest.cc:99:
weak_factory_(this) {}
It seems that this constructor should be setting throttle_created_ to true.

https://codereview.chromium.org/1384113002/diff/60001/content/browser/loader/...
content/browser/loader/cross_site_resource_handler_browsertest.cc:209:
old_delegate_ = NULL;
nullptr

[ncarter (slow), 2015-10-05 18:31:37]

https://codereview.chromium.org/1384113002/diff/60001/content/browser/loader/...
File content/browser/loader/cross_site_resource_handler_browsertest.cc (right):

https://codereview.chromium.org/1384113002/diff/60001/content/browser/loader/...
content/browser/loader/cross_site_resource_handler_browsertest.cc:49: // Expect
only a single request for the tracked url.
On 2015/10/05 18:01:54, nasko (slow to review) wrote:
> nit: Empty line between code and new comment line.

Done.

https://codereview.chromium.org/1384113002/diff/60001/content/browser/loader/...
content/browser/loader/cross_site_resource_handler_browsertest.cc:50:
ASSERT_FALSE(throttle_created_);
On 2015/10/05 18:01:54, nasko (slow to review) wrote:
> I don't see how throttle_created_ can actually become true. It is initialized
to
> false in the constructor and set to false in SetTrackedURLOnIOThread.

Good catch; I think I lose my copy+paste privileges for a week. Fixed.

https://codereview.chromium.org/1384113002/diff/60001/content/browser/loader/...
content/browser/loader/cross_site_resource_handler_browsertest.cc:91: class
DeferStartForTestCallback : public ResourceThrottle {
On 2015/10/05 18:01:54, nasko (slow to review) wrote:
> Shouldn't this class bear some mention of Throttle on its name?

Done.

https://codereview.chromium.org/1384113002/diff/60001/content/browser/loader/...
content/browser/loader/cross_site_resource_handler_browsertest.cc:99:
weak_factory_(this) {}
On 2015/10/05 18:01:54, nasko (slow to review) wrote:
> It seems that this constructor should be setting throttle_created_ to true.

Done.

https://codereview.chromium.org/1384113002/diff/60001/content/browser/loader/...
content/browser/loader/cross_site_resource_handler_browsertest.cc:209:
old_delegate_ = NULL;
On 2015/10/05 18:01:54, nasko (slow to review) wrote:
> nullptr

Done, everywhere.

[nasko, 2015-10-05 18:38:33]

LGTM with one last comment.

https://codereview.chromium.org/1384113002/diff/100001/content/browser/loader...
File content/browser/loader/cross_site_resource_handler_browsertest.cc (right):

https://codereview.chromium.org/1384113002/diff/100001/content/browser/loader...
content/browser/loader/cross_site_resource_handler_browsertest.cc:246:
NavigateToURL(shell(), attacker_page);
Oh, missed to notice that this needs to be wrapped in EXPECT_TRUE.

[ncarter (slow), 2015-10-05 18:59:34]

https://codereview.chromium.org/1384113002/diff/100001/content/browser/loader...
File content/browser/loader/cross_site_resource_handler_browsertest.cc (right):

https://codereview.chromium.org/1384113002/diff/100001/content/browser/loader...
content/browser/loader/cross_site_resource_handler_browsertest.cc:246:
NavigateToURL(shell(), attacker_page);
On 2015/10/05 18:38:33, nasko (slow to review) wrote:
> Oh, missed to notice that this needs to be wrapped in EXPECT_TRUE.

Done here, and everywhere in the other browsertest file too (since this line was
copied from there)

[ncarter (slow), 2015-10-05 18:59:37]

The CQ bit was checked by nick@chromium.org

[ncarter (slow), 2015-10-05 18:59:38]

The patchset sent to the CQ was uploaded after l-g-t-m from nasko@chromium.org
Link to the patchset: https://codereview.chromium.org/1384113002/#ps120001
(title: "Add EXPECT_TRUE's")

[commit-bot: I haz the power, 2015-10-05 19:00:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1384113002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1384113002/120001

[commit-bot: I haz the power, 2015-10-05 20:38:54]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-10-05 20:40:20]

Patchset 7 (id:??) landed as
https://crrev.com/dc67237b8beb43a3389c38861c9c153cbb374a91
Cr-Commit-Position: refs/heads/master@{#352413}