Make the logic for stream interception by MimeHandlerViews consistent with starting the plugin

content/browser/loader/buffered_resource_handler.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/loader/buffered_resource_handler.h"
 
 #include <vector>
 
 #include "base/bind.h"
 #include "base/location.h"
@@ skipping 276 matching lines @@
                          type_hint, &new_type);
 
   // SniffMimeType() returns false if there is not enough data to determine
   // the mime type. However, even if it returns false, it returns a new type
   // that is probably better than the current one.
   response_->head.mime_type.assign(new_type);
 
   return made_final_decision;
 }
 
+bool BufferedResourceHandler::IsHandledByPlugin(bool* defer,
+                                                bool* request_handled) {
+#if defined(ENABLE_PLUGINS)
+  bool stale;
+  WebPluginInfo plugin;
+  bool allow_wildcard = false;
+  ResourceRequestInfoImpl* info = GetRequestInfo();
+  bool has_plugin = plugin_service_->GetPluginInfo(
+      info->GetChildID(), info->GetRenderFrameID(), info->GetContext(),
+      request()->url(), GURL(), response_->head.mime_type, allow_wildcard,
+      &stale, &plugin, NULL);
+
+  if (stale) {
+    // Refresh the plugins asynchronously.
+    plugin_service_->GetPlugins(
+        base::Bind(&BufferedResourceHandler::OnPluginsLoaded,
+                   weak_ptr_factory_.GetWeakPtr()));
+    request()->LogBlockedBy("BufferedResourceHandler");
+    *defer = true;
+    return true;
+  }
+
+  if (has_plugin) {
+    if (plugin.type == WebPluginInfo::PLUGIN_TYPE_BROWSER_PLUGIN) {
+      // If it is a MimeHandlerView plugin, intercept the stream.
+      std::string payload;
+      scoped_ptr<ResourceHandler> handler(host_->MaybeInterceptAsStream(
+          plugin.path, request(), response_.get(), &payload));

[mmenke, 2015/06/11 19:38:04]

This exactly matches the fallthrough logic.  Maybe this should be:

if (has_plugin && plugin.type != WebPluginInfo::PLUGIN_TYPE_BROWSER_PLUGIN) {
  // Some comment here
  return true;
}

// If it is a MimeHandlerView plugin or using the legacy streamsPrivate
extensions
// API, attempt to intercept the stream.
// TODO...
...
if (handler) {
  ...
  return true;
}

DCHECK(!has_plugin);
// Could have special case logic here if has_plugin is true, if you want.
return false;


It does require a slightly bigger change when the streamsPrivate API case is
removed, but think it's clearer that both cases follow the same logic.

[raymes, 2015/07/01 05:13:18]

That's much simpler, thanks!

+      if (handler) {
+        *request_handled = UseAlternateNextHandler(handler.Pass(), payload);
+        return true;
+      }
+      // If the plugin is a MimeHandlerView plugin it should always be
+      // intercepted as a stream so this point should never be reached.

[mmenke, 2015/06/11 19:38:04]

This is content/, not chrome/.  I assume MimeHandlerView is a chrome/ concept?

[raymes, 2015/07/01 05:13:18]

Done.

+      NOTREACHED();
+      return false;
+    } else {
+      return true;
+    }
+  }
+
+  // If execution reaches here then try intercepting the stream for the old
+  // streamsPrivate extensions API. This API is deprecated and should go away.

[mmenke, 2015/06/11 19:38:04]

The streamsPrivate API is a chrome feature.  We're in content.  Can we document
this in some more content-friendly manner.

[raymes, 2015/07/01 05:13:18]

Done.

+  std::string payload;
+  scoped_ptr<ResourceHandler> handler(host_->MaybeInterceptAsStream(
+      base::FilePath(), request(), response_.get(), &payload));
+  if (handler) {
+    *request_handled = UseAlternateNextHandler(handler.Pass(), payload);
+    return true;
+  }
+#endif
+  return false;
+}
+
 bool BufferedResourceHandler::SelectNextHandler(bool* defer) {
   DCHECK(!response_->head.mime_type.empty());
 
   ResourceRequestInfoImpl* info = GetRequestInfo();
   const std::string& mime_type = response_->head.mime_type;
 
   if (mime_util::IsSupportedCertificateMimeType(mime_type)) {
     // Install certificate file.
     info->set_is_download(true);
     scoped_ptr<ResourceHandler> handler(
         new CertificateResourceHandler(request()));
     return UseAlternateNextHandler(handler.Pass(), std::string());
   }
 
   // Allow requests for object/embed tags to be intercepted as streams.
   if (info->GetResourceType() == content::RESOURCE_TYPE_OBJECT) {
     DCHECK(!info->allow_download());
-    std::string payload;
-    scoped_ptr<ResourceHandler> handler(
-        host_->MaybeInterceptAsStream(request(), response_.get(), &payload));
-    if (handler) {
-      DCHECK(!mime_util::IsSupportedMimeType(mime_type));
-      return UseAlternateNextHandler(handler.Pass(), payload);
-    }
+
+    bool request_handled = true;
+    bool handled_by_plugin = IsHandledByPlugin(defer, &request_handled);
+    if (!request_handled)
+      return false;
+    if (handled_by_plugin)

[mmenke, 2015/06/11 19:38:04]

If |defer| is set to true, the method also must return true, which seems a
little unexpected.  Suggest "if (handled_by_plugin || defer)", here and below,
and documenting what is returned in the defer case in the header.

[raymes, 2015/07/01 05:13:18]

Done.

+      return true;
   }
 
   if (!info->allow_download())
     return true;
 
   // info->allow_download() == true implies
   // info->GetResourceType() == RESOURCE_TYPE_MAIN_FRAME or
   // info->GetResourceType() == RESOURCE_TYPE_SUB_FRAME.
   DCHECK(info->GetResourceType() == RESOURCE_TYPE_MAIN_FRAME ||
          info->GetResourceType() == RESOURCE_TYPE_SUB_FRAME);
 
   bool must_download = MustDownload();
   if (!must_download) {
     if (mime_util::IsSupportedMimeType(mime_type))
       return true;
 
-    std::string payload;
-    scoped_ptr<ResourceHandler> handler(
-        host_->MaybeInterceptAsStream(request(), response_.get(), &payload));
-    if (handler) {
-      return UseAlternateNextHandler(handler.Pass(), payload);
-    }
-
-#if defined(ENABLE_PLUGINS)
-    bool stale;
-    bool has_plugin = HasSupportingPlugin(&stale);
-    if (stale) {
-      // Refresh the plugins asynchronously.
-      plugin_service_->GetPlugins(
-          base::Bind(&BufferedResourceHandler::OnPluginsLoaded,
-                     weak_ptr_factory_.GetWeakPtr()));
-      request()->LogBlockedBy("BufferedResourceHandler");
-      *defer = true;
+    bool request_handled = true;
+    bool handled_by_plugin = IsHandledByPlugin(defer, &request_handled);
+    if (!request_handled)
+      return false;
+    if (handled_by_plugin)
       return true;
-    }
-    if (has_plugin)
-      return true;
-#endif
   }
 
   // Install download handler
   info->set_is_download(true);
   scoped_ptr<ResourceHandler> handler(
       host_->CreateResourceHandlerForDownload(
           request(),
           true,  // is_content_initiated
           must_download,
           DownloadItem::kInvalidId,
@@ skipping 96 matching lines @@
              host_->delegate()->ShouldForceDownloadResource(
                  request()->url(), response_->head.mime_type)) {
     must_download_ = true;
   } else {
     must_download_ = false;
   }
 
   return must_download_;
 }
 
-bool BufferedResourceHandler::HasSupportingPlugin(bool* stale) {
-#if defined(ENABLE_PLUGINS)
-  ResourceRequestInfoImpl* info = GetRequestInfo();
-
-  bool allow_wildcard = false;
-  WebPluginInfo plugin;
-  return plugin_service_->GetPluginInfo(
-      info->GetChildID(), info->GetRenderFrameID(), info->GetContext(),
-      request()->url(), GURL(), response_->head.mime_type, allow_wildcard,
-      stale, &plugin, NULL);
-#else
-  if (stale)
-    *stale = false;
-  return false;
-#endif
-}
-
 bool BufferedResourceHandler::CopyReadBufferToNextHandler() {
   if (!read_buffer_.get())
     return true;
 
   scoped_refptr<net::IOBuffer> buf;
   int buf_len = 0;
   if (!next_handler_->OnWillRead(&buf, &buf_len, bytes_read_))
     return false;
 
   CHECK((buf_len >= bytes_read_) && (bytes_read_ >= 0));
   memcpy(buf->data(), read_buffer_->data(), bytes_read_);
   return true;
 }
 
 void BufferedResourceHandler::OnPluginsLoaded(
     const std::vector<WebPluginInfo>& plugins) {
   request()->LogUnblocked();
   bool defer = false;
   if (!ProcessResponse(&defer)) {
     controller()->Cancel();
   } else if (!defer) {
     controller()->Resume();
   }
 }
 
 }  // namespace content