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..731d8b8efaa531e6130c1b6895ac4699cb2a458d 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
+  // mime sniffing happens. 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 MustProcessResponseBeforeMimeSniffing();

[mmenke, 2016/07/27 19:34:01]

Could name this be more general?  MustProcessResponseBeforeReadingBody?

I guess you're not just moving all embedded-supplied throttles before the the
mime sniffer handler to maintain order as closely as possible?  Maybe we should
just move all throttles before the mime sniffer, and then add the
x-frame-options one after it, when it's added?  That does an even better job of
maintaining throttle order.

[clamy, 2016/08/17 12:47:35]

Done.

To do what you suggest we have two options:
- Move the NavigationResourceThrottle after the mime sniffer -> but this means
that NavigationThrottles which execute first now execute last (ex:
InterceptNavigationThrottle on Android).
- Have two NavigationResourceThrottles: this adds one more thread hop, and also
requires a lot of changes since the UI thread classes don't expect to be called
twice.

All in all, just moving the SafeBrowsingResourceThrottle up has the less
disruption of the throttle order. Note that I think this is a somewhat temporary
solution. since it would be better enventually to ensure that a request
cancelled for security reasons can be removed form cache. This way we don't have
to worry about parts of it having been cached.

[mmenke, 2016/08/17 19:53:08]

I don't think this is going to happen, reliably - if a request is cancelled
after it's been cached, but before it reaches the SafeBrowsingResourceThrottle,
the bad response will still be cached.  I consider safe browsing checks after
any part of a response reaches the cache best-effort only.  We also don't ensure
the URLRequest isn't deleted in the middle of a safe browsing check.  Nor do we
ensure Chrome doesn't shut down in the meantime, etc.  Just nothing about
deleting a file in the cache in the safe browsing case can be considered
reliable.

[clamy, 2016/08/18 12:01:51]

Acknowledged.

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