Forward MIME types to BrowserPlugin when a viewer is specified.

content/browser/loader/buffered_resource_handler.cc

Index: content/browser/loader/buffered_resource_handler.cc
diff --git a/content/browser/loader/buffered_resource_handler.cc b/content/browser/loader/buffered_resource_handler.cc
index d5cabe35d4b0105f2c7002353dfa14da96d124bc..48061396a7a18634d5cdab08140632a08a091c0c 100644
--- a/content/browser/loader/buffered_resource_handler.cc
+++ b/content/browser/loader/buffered_resource_handler.cc
@@ -15,6 +15,7 @@
 #include "content/browser/loader/certificate_resource_handler.h"
 #include "content/browser/loader/resource_dispatcher_host_impl.h"
 #include "content/browser/loader/resource_request_info_impl.h"
+#include "content/browser/loader/stream_resource_handler.h"
 #include "content/browser/plugin_service_impl.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/download_item.h"
@@ -321,10 +322,22 @@ bool BufferedResourceHandler::SelectNextHandler(bool* defer) {
     if (net::IsSupportedMimeType(mime_type))
       return true;
 
-    scoped_ptr<ResourceHandler> handler(
+    scoped_ptr<StreamResourceHandler> handler(
         host_->MaybeInterceptAsStream(request(), response_.get()));
-    if (handler)
-      return UseAlternateNextHandler(handler.Pass());
+    if (handler) {
+      if (handler->HasPayload()) {

[mmenke, 2014/05/12 17:12:13]

I'm really not following any of this code.

+        handler->SetNextHandler(next_handler_.Pass());
+
+        next_handler_ = handler.Pass();
+        next_handler_->SetController(this);
+
+        int request_id =
+            ResourceRequestInfo::ForRequest(request())->GetRequestID();
+        return CopyReadBufferToNextHandler(request_id);

[mmenke, 2014/05/13 15:14:32]

I think it's less than ideal to split the responsibility to handle the
particulars of this case between the BufferedResourceHandler and the
StreamResourceHandler like this - I don't think both classes should know about
the behaviors in this case.

Given that we want to use "UseAlternateNextHander" in the non-payload case, my
suggestion is to do this:

// Feel free to come up with a better name.
std::string payload_to_send_old_handler;

scoped_ptr<StreamResourceHandler> handler(request(), response_.get(),
&payload_to_send_old_handler);

if (handler) {
  UseAlternateNextHandler(handler.PassAs<ResourceHandler>(),
payload_to_send_old_handler);
}

Then just maintain the old behavior unless the payload is non-empty.

I'm also not sure about the non-200 response case, where we cancel the request
in UseAlternateNextHandler - do we want this behavior or not, in the new case?


An alternate approach would be to move all the logic into the
StreamResourceHandler, but I think you'd still need the check if you should call
UseAlternateNextHandler or not, so I prefer the other approach (Also, that means
only the BufferedResourceHandler is responsible for sending information to the
original next handler, in all cases, which I think is a little cleaner).

[Zachary Kuznia, 2014/05/20 22:24:39]

If I understand correctly, you're saying that I should move the code in
StreamResourceHandler() into UseAlternateNextHandler(), and use that codepath
instead of cancelling the request if a payload is passed in via
payload_to_send_old_handler?

[mmenke, 2014/05/21 15:17:49]

That's my suggestion.  I don't think the new behavior should be split between
the BufferedResourceHandler and the StreamResourceHandler.  I'm certainly open
to doing that in other ways, but that seems the simplest option to me.

+      } else {
+        UseAlternateNextHandler(handler.PassAs<ResourceHandler>());
+      }
+    }
 
 #if defined(ENABLE_PLUGINS)
     bool stale;