Do not trigger CrossSiteResourceHandler for streams or user certs.

content/browser/loader/cross_site_resource_handler.cc

Index: content/browser/loader/cross_site_resource_handler.cc
diff --git a/content/browser/loader/cross_site_resource_handler.cc b/content/browser/loader/cross_site_resource_handler.cc
index 5bed76777c540a5e31b4017247d83f2f6d651e72..486f48ecb06a8bf20e6f95bb5c793e4eb4915f09 100644
--- a/content/browser/loader/cross_site_resource_handler.cc
+++ b/content/browser/loader/cross_site_resource_handler.cc
@@ -161,7 +161,7 @@ bool CrossSiteResourceHandler::OnResponseStarted(
   // navigation) will stick around until the next cross-site navigation, since
   // we are unable to tell when to destroy it.
   // See RenderFrameHostManager::RendererAbortedProvisionalLoad.
-  if (!swap_needed || info->IsDownload() ||
+  if (!swap_needed || info->IsDownload() || info->is_stream() ||

[davidben, 2014/02/12 19:57:56]

It's possible we should just set the IsDownload bit on streams too. I went with
this just because ResourceLoader::CancelRequestInternal also checks both.

[Zachary Kuznia, 2014/02/12 22:04:48]

IsDownload and is_stream should be mutually exclusive, so this should be the
right approach.

[davidben, 2014/02/12 22:18:56]

Right, but the question is whether consumers of the ResourceRequestInfo should
care about the distinction. Maybe we should remove is_stream() and always set
IsDownload() when the BRH detaches the handler. The current approach is
certainly more error-prone, as this bug demonstrates.

Checking on codesearch, everything that checks for is_stream() also checks for
IsDownload(). Everything that checks for IsDownload() also checks for
is_stream() except this code (a bug) and AppUrlRedirector which... couldn't
check is_stream() if it wanted to because it's not exported. But it doesn't care
either way because it only queries when a request starts. Haven't looked closely
at what it does and whether it should care.

[Charlie Reis, 2014/02/13 01:45:07]

I agree that the current approach is error prone.  It might be nice to have a
method that covers these similar cases if there's a concept that be used to
describe all of them (or their inverse, something like "IsDocument" or
"WillCommit").

In this case, whether the network response will commit as a new navigation is
the distinction the code is looking for.  Do the other callers of IsDownload()
and is_stream() need to care about 204 responses, since they lead to a similar
no-new-commit outcome?

[davidben, 2014/02/13 03:48:28]

The other callers seem to be more interested in the detaching aspect. There's
logic to allow them to outlive the route, and logic to ignore the renderer's
cancellations. And there's AppUrlRedirector which I still need to look at.

Calling a stream a "download" seems fairly sound to me. I'm already lumping
x-x509-user-cert here under "download" on grounds that it's a download with a
funny handler. Could we claim a stream is one too? Maybe Download isn't the best
word... "detached"? Detached is already used for DetachableResourceHandler, but
it's pretty similar actually. Maybe we could unifying those codepaths too, I
dunno. "Redirected" is the other word that comes to mind, but that also already
means something else. :-)

[Charlie Reis, 2014/02/13 19:33:05]

I'll defer to networking folks on that decision, since I don't have a great
sense for the behavior for streams, certificates, or what detaching means.  I'm
ok with lumping it all as downloads unless there's a better name for it. 
Certainly makes sense to do that as a followup CL.

       (response->head.headers.get() &&
        response->head.headers->response_code() == 204)) {
     return next_handler_->OnResponseStarted(request_id, response, defer);