Ensuring interception of stream get determined by plugin path before checking mime type.

content/browser/loader/buffered_resource_handler.h

Index: content/browser/loader/buffered_resource_handler.h
diff --git a/content/browser/loader/buffered_resource_handler.h b/content/browser/loader/buffered_resource_handler.h
index 4e80ec53bd58e9c731037c14fc10ac3af05b7099..8a41f2ab162ccf153cea274dc608f9c34f6ca0b6 100644
--- a/content/browser/loader/buffered_resource_handler.h
+++ b/content/browser/loader/buffered_resource_handler.h
@@ -56,6 +56,7 @@ class CONTENT_EXPORT BufferedResourceHandler
 
   bool ShouldSniffContent();
   bool DetermineMimeType();
+  bool IsHandledByPlugin(bool* defer, bool* result);

[mmenke, 2015/03/24 14:44:32]

This should be documented.  We return a boolean, and yet we also take a pointer
to a "result".  Unclear what either means.  Also not clear how we call back in
the case "defer" is set to true.

[raymes, 2015/03/25 00:26:32]

It was never clear how we call back in the case defer is set to true - the
documentation in this class isn't great :) I think that's orthogonal to this
change though.

|result| is the return value of SelectNextHandler and again there is no
documentation here, so it's unclear. But tracing the code through it looks like
it is whether the request should be cancelled.

So we can rename |result| to |cancel_request|

// Returns true if a plugin will handle the current request. |defer| 
// is set to true if plugin data is stale and needs to be refreshed 
// before the request can be handled. |cancel_request| is true if 
// the request should be cancelled and false otherwise.

[Deepak, 2015/03/25 05:43:34]

Done.

   bool SelectNextHandler(bool* defer);
   bool UseAlternateNextHandler(scoped_ptr<ResourceHandler> handler,
                                const std::string& payload_for_old_handler);
@@ -64,7 +65,7 @@ class CONTENT_EXPORT BufferedResourceHandler
   void CallReplayReadCompleted();
 
   bool MustDownload();
-  bool HasSupportingPlugin(bool* is_stale);
+  bool GetSupportingPlugin(WebPluginInfo* plugin, bool* stale);

[mmenke, 2015/03/24 14:44:32]

Mind documenting this?  Unclear what the return value means, or stale, for that
matter.

[raymes, 2015/03/25 00:26:32]

As you mentioned in your comment, this function can (and probably should) just
be inlined into IsHandledByPlugin. But for the sake of explanation:

// Returns true if there is a plugin that can handle the current 
// request. If there is such a plugin, it is returned in |plugin|. If 
// the plugin data is out of date and needs to be refreshed from disk,
// |stale| will be set to true and the result will not be valid.

[Deepak, 2015/03/25 05:43:34]

Have made this function inline as it is not getting used anywhere else except in
IsHandledByPlugin().

 
   // Copies data from |read_buffer_| to |next_handler_|.
   bool CopyReadBufferToNextHandler();