PlzNavigate: make StreamPrivate API work

PlzNavigate: make StreamPrivate API work

This CL enables the extensions Stream private API to work with PlzNavigate
enabled. This fixes the MaterialPDFExtensionTests, PDFExtensionTests and
StreamsPrivate API tests with browser-side-navigation enabled.

BUG=504347

[clamy, 2015-12-03 14:39:34]

Description was changed from

==========
PlzNavigate: make StreamPrivate API work

This CL enables the extensions Stream private API to work with PlzNavigate
enabled. This fixes the MaterialPDFExtensionTests, PDFExtensionTests and
StreamsPrivate API tests with browser-side-navigation enabled.

BUG=504347
==========

to

==========
PlzNavigate: make StreamPrivate API work

This CL enables the extensions Stream private API to work with PlzNavigate
enabled. This fixes the MaterialPDFExtensionTests, PDFExtensionTests and
StreamsPrivate API tests with browser-side-navigation enabled.

BUG=504347
==========

[clamy, 2015-12-03 14:39:35]

clamy@chromium.org changed reviewers:
+ davidben@chromium.org

[clamy, 2015-12-03 14:39:52]

@davidben: PTAL

[davidben, 2015-12-09 23:53:44]

Hrm. Does this quite work? This is all kind of horrific. As I understand,
there's two very different codepaths for this API:

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ren...

First is the streamsPrivate API. For those, we navigate to a URL and the MIME
type is intercepted by an extension. The navigation aborts like a download and
the extension gets an onExecuteMimeTypeHandler event with tabId so it can do
what it likes, be it open a new window or navigate the tab elsewhere. For that
use, this logic is what we want. NavigationResourceHandler will see:

OnResponseStarted
OnResponseCompleted(ERR_ABORTED)

I think the QuickOffice stuff uses this, but you might want to check with
raymes@ or zork@ to see if this is in use.


Then there's the mimeHandlerPrivate API. I'm less familiar with how this one
works, but it's what the PDF plugin uses and crashes because
MimeTypeResourceHandler now sends

OnResponseStarted
OnWillRead
OnReadCompleted
OnResponseCompleted(OK)
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/lo...

Instead of sending the actual contents, we send down some GUID, which is the
payload argument that
ChromeResourceDispatcherHostDelegate::ShouldInterceptResourceAsStream spits out.
This gets routed around via some crazy thing or another to a plugin and the GUID
is somehow related to tying it all together.


Unfortunately, we can't distinguish those cases. I think ideally what would
happen is that, for PlzNavigate, MimeTypeResourceHandler should *not* send
OnResponseStarted down to the child ResourceHandler. That logic in
NavigationResourceHandler was really just to work around
MimeTypeResourceHandler's weirdness. I think the only reason OnResponseStarted
gets signaled is for DevTools, which needs a PlzNavigate story anyway.

(There's also the other direction I'd eventually like which is that
MimeTypeResourceHandler ends up just doing sniffing and the dispatch portion
happens on the UI thread in the navigation code. I don't know how feasible
getting there is today.)


See also this CL was also related to this problem I think:
https://codereview.chromium.org/1323793002/

https://codereview.chromium.org/1485693002/diff/1/chrome/browser/renderer_hos...
File chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc
(right):

https://codereview.chromium.org/1485693002/diff/1/chrome/browser/renderer_hos...
chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc:195: //
switched to FrameTreeNode IDs, so that unclaimed streams are not leaked.
Probably worth a bug number?

[davidben, 2015-12-09 23:54:30]

On 2015/12/09 23:53:44, davidben (behind on reviews) wrote:
> Hrm. Does this quite work? This is all kind of horrific.

(Er, "this" being this existing machinery, not your CL specifically.)

[davidben, 2015-12-09 23:56:49]

Oh, yet another option is to get some bool stapled to the ResourceRequestInfo
like request_was_intercepted() which means that , although you see
OnResponseStarted, it's really just kind of a legacy thing due to DevTools
plumbing and the request is going to get aborted immediately. And it would only
get set in the download and streamsPrivate cases, but not the mimeHandlerPrivate
case.