Move MimeTypeResourceHandler before ThrottlingResourceHandler

content/public/browser/resource_throttle.h

Index: content/public/browser/resource_throttle.h
diff --git a/content/public/browser/resource_throttle.h b/content/public/browser/resource_throttle.h
index 7a980f2a21691357e111eeebdf469ffc317c6c63..4c52d7e9437e9526c84cfb2462e82cd20e589416 100644
--- a/content/public/browser/resource_throttle.h
+++ b/content/public/browser/resource_throttle.h
@@ -5,6 +5,8 @@
 #ifndef CONTENT_PUBLIC_BROWSER_RESOURCE_THROTTLE_H_
 #define CONTENT_PUBLIC_BROWSER_RESOURCE_THROTTLE_H_
 
+#include "content/common/content_export.h"
+
 namespace net {
 struct RedirectInfo;
 }
@@ -20,7 +22,7 @@ class ThrottlingResourceHandler;
 // resource load.  The ResourceController interface may be used to resume a
 // deferred resource load, or it may be used to cancel a resource load at any
 // time.
-class ResourceThrottle {
+class CONTENT_EXPORT ResourceThrottle {
  public:
   virtual ~ResourceThrottle() {}
 
@@ -42,6 +44,12 @@ class ResourceThrottle {
   // returned string.
   virtual const char* GetNameForLogging() const = 0;
 
+  // Whether this ResourceThrottle needs to execute WillProcessResponse before
+  // any part of the response body is read. Normally this is false. This should
+  // be set to true if the ResourceThrottle wants to ensure that no part of the
+  // response will be cached if the request is canceled in WillProcessResponse.
+  virtual bool MustProcessResponseBeforeReadingBody();

[mmenke, 2016/08/19 16:33:02]

"no part of the response" -> "no part of the response body".  I suspect we won't
cache the headers either, but I'm not sure of that (And I don't think this code
should implicitly make promises about caching stuff once we've pulled it from
net)

[mmenke, 2016/08/19 16:33:02]

Think we need test coverage of this method returning true - Can either cancel in
WillProcessResponse and make sure we never read the body, or make a cacheable
response, cancel there and make sure a second request doesn't use the cache.

[mmenke, 2016/08/22 20:39:05]

+const

[mmenke, 2016/08/22 20:39:05]

Looks through all other throttles that implement WillProcessresponse, the only
other change with this approach instead of moving all throttles but the
navigation one before the mime sniffer looks to be that
PowerSaveBlockResourceThrottle will block entering power save mode for longer
(Normally blocks from upload start to headers received, now will wait until
after mime type is sniffed).  That doesn't exactly fill me with dread, though
I'm still concerned we could be missing something.

[clamy, 2016/09/01 23:19:41]

See https://www.chromium.org/developers/content-module/content-api: "don't add
the const identifier to interfaces. For interfaces implemented by the embedder,
we can't make assumptions about what the embedder needs to implement it. For
interfaces implemented by content, the implementation details doesn't have to be
exposed.". I know we're doing it above, but we shouldn't.

[clamy, 2016/09/01 23:19:42]

I have added a unit tests that checks that if a ResourceThrottle has
MustProcessResponseBeforeReadingBody and cancels the request in
WillProcessResponse, we will never call ReadData on the URLRequestJob.

[clamy, 2016/09/01 23:19:42]

Done.

+
   void set_controller_for_testing(ResourceController* c) {
     controller_ = c;
   }