Move data reduction proxy logic out of chrome and android webview network delegate

net/base/wrapping_network_delegate.h

-// 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.
+// TODO: Insert description here. (generated by megjablon)

[mmenke, 2014/11/26 15:23:52]

Need copyright header.

Also, descriptions generally go above class definitions, not at the top of a
file (Where they'll often be ignored, along with the copyright notice)

[megjablon, 2014/12/01 19:26:56]

Done.

 
-#ifndef NET_BASE_NETWORK_DELEGATE_H_
-#define NET_BASE_NETWORK_DELEGATE_H_
+#ifndef NET_BASE_WRAPPING_NETWORK_DELEGATE_H_
+#define NET_BASE_WRAPPING_NETWORK_DELEGATE_H_
 
-#include <string>
-
-#include "base/callback.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/strings/string16.h"
-#include "base/threading/non_thread_safe.h"
-#include "net/base/auth.h"
 #include "net/base/completion_callback.h"
+#include "net/base/network_delegate.h"
 #include "net/cookies/canonical_cookie.h"
 
 class GURL;
 
 namespace base {
 class FilePath;
 }
 
 namespace net {
 
-// NOTE: Layering violations!
-// We decided to accept these violations (depending
-// on other net/ submodules from net/base/), because otherwise NetworkDelegate
-// would have to be broken up into too many smaller interfaces targeted to each
-// submodule. Also, since the lower levels in net/ may callback into higher
-// levels, we may encounter dangerous casting issues.
-//
-// NOTE: It is not okay to add any compile-time dependencies on symbols outside
-// of net/base here, because we have a net_base library. Forward declarations
-// are ok.
 class CookieOptions;
 class HttpRequestHeaders;
 class HttpResponseHeaders;
 class ProxyInfo;
 class ProxyServer;
 class ProxyService;
 class URLRequest;
 
-class NET_EXPORT NetworkDelegate : public base::NonThreadSafe {
+class NET_EXPORT WrappingNetworkDelegate : public NetworkDelegate {

[mmenke, 2014/11/26 15:23:53]

Maybe call this the LayeredNetworkDelegate?

[megjablon, 2014/12/01 19:26:56]

Done.

  public:
-  // AuthRequiredResponse indicates how a NetworkDelegate handles an
-  // OnAuthRequired call. It's placed in this file to prevent url_request.h
-  // from having to include network_delegate.h.
-  enum AuthRequiredResponse {
-    AUTH_REQUIRED_RESPONSE_NO_ACTION,
-    AUTH_REQUIRED_RESPONSE_SET_AUTH,
-    AUTH_REQUIRED_RESPONSE_CANCEL_AUTH,
-    AUTH_REQUIRED_RESPONSE_IO_PENDING,
-  };
-  typedef base::Callback<void(AuthRequiredResponse)> AuthCallback;
+  // WrappingNetworkDelegate takes a |network_delegate| and extends it. When
+  // On*() is called, the On*Internal() method of this is first called and then
+  // the On*() of |network_delegate| is called.

[mmenke, 2014/11/26 15:23:52]

Should also mention that the internal methods have no return values, and cannot
prevent calling into the nested network delegate.

[mmenke, 2014/11/26 15:23:52]

This should be just above the class declaration.

[megjablon, 2014/12/01 19:26:56]

Done.

+  WrappingNetworkDelegate(scoped_ptr<NetworkDelegate> network_delegate);

[mmenke, 2014/11/26 15:23:52]

Suggest network_delegate be renamed to nested_network_delegate, here and the
private variable as well.

[mmenke, 2014/11/26 15:23:52]

Single argument methods should be explicit.

[megjablon, 2014/12/01 19:26:56]

Done.

+  virtual ~WrappingNetworkDelegate();

[mmenke, 2014/11/26 15:23:52]

nit:  -virtual + override

[megjablon, 2014/12/01 19:26:56]

Done.

 
-  virtual ~NetworkDelegate() {}
-
-  // Notification interface called by the network stack. Note that these
-  // functions mostly forward to the private virtuals. They also add some sanity
-  // checking on parameters. See the corresponding virtuals for explanations of
-  // the methods and their arguments.
-  int NotifyBeforeURLRequest(URLRequest* request,
-                             const CompletionCallback& callback,
-                             GURL* new_url);
-  void NotifyResolveProxy(const GURL& url,
-                          int load_flags,
-                          const ProxyService& proxy_service,
-                          ProxyInfo* result);
-  void NotifyProxyFallback(const ProxyServer& bad_proxy,
-                           int net_error);
-  int NotifyBeforeSendHeaders(URLRequest* request,
-                              const CompletionCallback& callback,
-                              HttpRequestHeaders* headers);
-  void NotifyBeforeSendProxyHeaders(URLRequest* request,
-                                    const ProxyInfo& proxy_info,
-                                    HttpRequestHeaders* headers);
-  void NotifySendHeaders(URLRequest* request,
-                         const HttpRequestHeaders& headers);
-  int NotifyHeadersReceived(
-      URLRequest* request,
-      const CompletionCallback& callback,
-      const HttpResponseHeaders* original_response_headers,
-      scoped_refptr<HttpResponseHeaders>* override_response_headers,
-      GURL* allowed_unsafe_redirect_url);
-  void NotifyBeforeRedirect(URLRequest* request,
-                            const GURL& new_location);
-  void NotifyResponseStarted(URLRequest* request);
-  void NotifyRawBytesRead(const URLRequest& request, int bytes_read);
-  void NotifyCompleted(URLRequest* request, bool started);
-  void NotifyURLRequestDestroyed(URLRequest* request);
-  void NotifyPACScriptError(int line_number, const base::string16& error);
-  AuthRequiredResponse NotifyAuthRequired(URLRequest* request,
-                                          const AuthChallengeInfo& auth_info,
-                                          const AuthCallback& callback,
-                                          AuthCredentials* credentials);
-  bool CanGetCookies(const URLRequest& request,
-                     const CookieList& cookie_list);
-  bool CanSetCookie(const URLRequest& request,
-                    const std::string& cookie_line,
-                    CookieOptions* options);
-  bool CanAccessFile(const URLRequest& request,
-                     const base::FilePath& path) const;
-  bool CanThrottleRequest(const URLRequest& request) const;
-  bool CanEnablePrivacyMode(const GURL& url,
-                            const GURL& first_party_for_cookies) const;
-
-  bool CancelURLRequestWithPolicyViolatingReferrerHeader(
-      const URLRequest& request,
-      const GURL& target_url,
-      const GURL& referrer_url) const;
-
- private:
-  // This is the interface for subclasses of NetworkDelegate to implement. These
-  // member functions will be called by the respective public notification
-  // member function, which will perform basic sanity checking.
+ // Protected for testing purposes.

[mmenke, 2014/11/26 15:23:52]

+1 indent...Also, not really sure these need to be protected.

[megjablon, 2014/12/01 19:26:56]

Done.

+ protected:
 
   // Called before a request is sent. Allows the delegate to rewrite the URL
   // being fetched by modifying |new_url|. If set, the URL must be valid. The
   // reference fragment from the original URL is not automatically appended to
   // |new_url|; callers are responsible for copying the reference fragment if
   // desired.
   // |callback| and |new_url| are valid only until OnURLRequestDestroyed is
   // called for this request. Returns a net status code, generally either OK to
   // continue with the request or ERR_IO_PENDING if the result is not ready yet.
   // A status code other than OK and ERR_IO_PENDING will cancel the request and
   // report the status code as the reason.
   //
   // The default implementation returns OK (continue with request).

[mmenke, 2014/11/26 15:23:52]

Should not duplicate comments like this (They'll become out of date if only the
public verison is updated).  Think we probably should have / don't need any
method comments in this file, except maybe the constructor.

[megjablon, 2014/12/01 19:26:56]

Done.

   virtual int OnBeforeURLRequest(URLRequest* request,
                                  const CompletionCallback& callback,
-                                 GURL* new_url);
+                                 GURL* new_url) final;

[mmenke, 2014/11/26 15:23:52]

The overridden methods should be grouped together, with no lines between them,
and a comment above them along the liens of "NetworkDelegate implementation:"

[mmenke, 2014/11/26 15:23:53]

-virtual + override (And keep the final) for all of the overridden methods.

[megjablon, 2014/12/01 19:26:56]

Done.

+
+  virtual void OnBeforeURLRequestInternal(URLRequest* request,
+                                          const CompletionCallback& callback,
+                                          GURL* new_url);
 
   // Called as the proxy is being resolved for |url|. Allows the delegate to
   // override the proxy resolution decision made by ProxyService. The delegate
   // may override the decision by modifying the ProxyInfo |result|.
   virtual void OnResolveProxy(const GURL& url,
                               int load_flags,
                               const ProxyService& proxy_service,
-                              ProxyInfo* result);
+                              ProxyInfo* result) final;
+
+  virtual void OnResolveProxyInternal(const GURL& url,
+                                      int load_flags,
+                                      const ProxyService& proxy_service,
+                                      ProxyInfo* result);
 
   // Called when use of |bad_proxy| fails due to |net_error|. |net_error| is
   // the network error encountered, if any, and OK if the fallback was
   // for a reason other than a network error (e.g. the proxy service was
   // explicitly directed to skip a proxy).
   virtual void OnProxyFallback(const ProxyServer& bad_proxy,
-                               int net_error);
+                               int net_error) final;
+
+  virtual void OnProxyFallbackInternal(const ProxyServer& bad_proxy,
+                                       int net_error);
 
   // Called right before the HTTP headers are sent. Allows the delegate to
   // read/write |headers| before they get sent out. |callback| and |headers| are
   // valid only until OnCompleted or OnURLRequestDestroyed is called for this
   // request.
   // See OnBeforeURLRequest for return value description. Returns OK by default.
   virtual int OnBeforeSendHeaders(URLRequest* request,
                                   const CompletionCallback& callback,
-                                  HttpRequestHeaders* headers);
+                                  HttpRequestHeaders* headers) final;
+
+  virtual void OnBeforeSendHeadersInternal(URLRequest* request,
+                                           const CompletionCallback& callback,
+                                           HttpRequestHeaders* headers);
 
   // Called after a proxy connection. Allows the delegate to read/write
   // |headers| before they get sent out. |headers| is valid only until
   // OnCompleted or OnURLRequestDestroyed is called for this request.
   virtual void OnBeforeSendProxyHeaders(URLRequest* request,
                                         const ProxyInfo& proxy_info,
-                                        HttpRequestHeaders* headers);
+                                        HttpRequestHeaders* headers) final;
+
+  virtual void OnBeforeSendProxyHeadersInternal(URLRequest* request,
+                                                const ProxyInfo& proxy_info,
+                                                HttpRequestHeaders* headers);
 
   // Called right before the HTTP request(s) are being sent to the network.
   // |headers| is only valid until OnCompleted or OnURLRequestDestroyed is
   // called for this request.
   virtual void OnSendHeaders(URLRequest* request,
-                             const HttpRequestHeaders& headers);
+                             const HttpRequestHeaders& headers) final;
+
+  virtual void OnSendHeadersInternal(URLRequest* request,
+                                     const HttpRequestHeaders& headers);
 
   // Called for HTTP requests when the headers have been received.
   // |original_response_headers| contains the headers as received over the
   // network, these must not be modified. |override_response_headers| can be set
   // to new values, that should be considered as overriding
   // |original_response_headers|.
   // If the response is a redirect, and the Location response header value is
   // identical to |allowed_unsafe_redirect_url|, then the redirect is never
   // blocked and the reference fragment is not copied from the original URL
   // to the redirection target.
   //
   // |callback|, |original_response_headers|, and |override_response_headers|
   // are only valid until OnURLRequestDestroyed is called for this request.
   // See OnBeforeURLRequest for return value description. Returns OK by default.
   virtual int OnHeadersReceived(
       URLRequest* request,
       const CompletionCallback& callback,
       const HttpResponseHeaders* original_response_headers,
       scoped_refptr<HttpResponseHeaders>* override_response_headers,
+      GURL* allowed_unsafe_redirect_url) final;
+
+  virtual void OnHeadersReceivedInternal(
+      URLRequest* request,
+      const CompletionCallback& callback,
+      const HttpResponseHeaders* original_response_headers,
+      scoped_refptr<HttpResponseHeaders>* override_response_headers,
       GURL* allowed_unsafe_redirect_url);
 
   // Called right after a redirect response code was received.
   // |new_location| is only valid until OnURLRequestDestroyed is called for this
   // request.
   virtual void OnBeforeRedirect(URLRequest* request,
-                                const GURL& new_location);
+                                const GURL& new_location) final;
+
+  virtual void OnBeforeRedirectInternal(URLRequest* request,
+                                        const GURL& new_location);
 
   // This corresponds to URLRequestDelegate::OnResponseStarted.
-  virtual void OnResponseStarted(URLRequest* request);
+  virtual void OnResponseStarted(URLRequest* request) final;
+
+  virtual void OnResponseStartedInternal(URLRequest* request);
 
   // Called every time we read raw bytes.
-  virtual void OnRawBytesRead(const URLRequest& request, int bytes_read);
+  virtual void OnRawBytesRead(const URLRequest& request, int bytes_read) final;
+
+  virtual void OnRawBytesReadInternal(const URLRequest& request,
+                                      int bytes_read);
 
   // Indicates that the URL request has been completed or failed.
   // |started| indicates whether the request has been started. If false,
   // some information like the socket address is not available.
-  virtual void OnCompleted(URLRequest* request, bool started);
+  virtual void OnCompleted(URLRequest* request, bool started) final;
+
+  virtual void OnCompletedInternal(URLRequest* request, bool started);
 
   // Called when an URLRequest is being destroyed. Note that the request is
   // being deleted, so it's not safe to call any methods that may result in
   // a virtual method call.
-  virtual void OnURLRequestDestroyed(URLRequest* request);
+  virtual void OnURLRequestDestroyed(URLRequest* request) final;
+
+  virtual void OnURLRequestDestroyedInternal(URLRequest* request);
 
   // Corresponds to ProxyResolverJSBindings::OnError.
   virtual void OnPACScriptError(int line_number,
-                                const base::string16& error);
+                                const base::string16& error) final;
+
+  virtual void OnPACScriptErrorInternal(int line_number,
+                                        const base::string16& error);
 
   // Called when a request receives an authentication challenge
   // specified by |auth_info|, and is unable to respond using cached
   // credentials. |callback| and |credentials| must be non-NULL, and must
   // be valid until OnURLRequestDestroyed is called for |request|.
   //
   // The following return values are allowed:
   //  - AUTH_REQUIRED_RESPONSE_NO_ACTION: |auth_info| is observed, but
   //    no action is being taken on it.
   //  - AUTH_REQUIRED_RESPONSE_SET_AUTH: |credentials| is filled in with
   //    a username and password, which should be used in a response to
   //    |auth_info|.
   //  - AUTH_REQUIRED_RESPONSE_CANCEL_AUTH: The authentication challenge
   //    should not be attempted.
   //  - AUTH_REQUIRED_RESPONSE_IO_PENDING: The action will be decided
   //    asynchronously. |callback| will be invoked when the decision is made,
   //    and one of the other AuthRequiredResponse values will be passed in with
   //    the same semantics as described above.
   virtual AuthRequiredResponse OnAuthRequired(
       URLRequest* request,
       const AuthChallengeInfo& auth_info,
       const AuthCallback& callback,
+      AuthCredentials* credentials) final;
+
+  virtual void OnAuthRequiredInternal(
+      URLRequest* request,
+      const AuthChallengeInfo& auth_info,
+      const AuthCallback& callback,
       AuthCredentials* credentials);
 
   // Called when reading cookies to allow the network delegate to block access
   // to the cookie. This method will never be invoked when
   // LOAD_DO_NOT_SEND_COOKIES is specified.
   virtual bool OnCanGetCookies(const URLRequest& request,
-                               const CookieList& cookie_list);
+                               const CookieList& cookie_list) final;
+
+  virtual void OnCanGetCookiesInternal(const URLRequest& request,
+                                       const CookieList& cookie_list);
 
   // Called when a cookie is set to allow the network delegate to block access
   // to the cookie. This method will never be invoked when
   // LOAD_DO_NOT_SAVE_COOKIES is specified.
   virtual bool OnCanSetCookie(const URLRequest& request,
                               const std::string& cookie_line,
-                              CookieOptions* options);
+                              CookieOptions* options) final;
+
+  virtual void OnCanSetCookieInternal(const URLRequest& request,
+                                      const std::string& cookie_line,
+                                      CookieOptions* options);
 
   // Called when a file access is attempted to allow the network delegate to
   // allow or block access to the given file path.  Returns true if access is
   // allowed.
   virtual bool OnCanAccessFile(const URLRequest& request,
-                               const base::FilePath& path) const;
+                               const base::FilePath& path) const final;
+
+  virtual void OnCanAccessFileInternal(const URLRequest& request,
+                                       const base::FilePath& path) const;
 
   // Returns true if the given request may be rejected when the
   // URLRequestThrottlerManager believes the server servicing the
   // request is overloaded or down.
-  virtual bool OnCanThrottleRequest(const URLRequest& request) const;
+  virtual bool OnCanThrottleRequest(const URLRequest& request) const final;
+
+  virtual void OnCanThrottleRequestInternal(const URLRequest& request) const;
 
   // Returns true if the given |url| has to be requested over connection that
   // is not tracked by the server. Usually is false, unless user privacy
   // settings block cookies from being get or set.
   virtual bool OnCanEnablePrivacyMode(
       const GURL& url,
+      const GURL& first_party_for_cookies) const final;
+
+  virtual void OnCanEnablePrivacyModeInternal(
+      const GURL& url,
       const GURL& first_party_for_cookies) const;
 
   // Called when the |referrer_url| for requesting |target_url| during handling
   // of the |request| is does not comply with the referrer policy (e.g. a
   // secure referrer for an insecure initial target).
   // Returns true if the request should be cancelled. Otherwise, the referrer
   // header is stripped from the request.
   virtual bool OnCancelURLRequestWithPolicyViolatingReferrerHeader(
       const URLRequest& request,
       const GURL& target_url,
+      const GURL& referrer_url) const final;
+
+  virtual void OnCancelURLRequestWithPolicyViolatingReferrerHeaderInternal(
+      const URLRequest& request,
+      const GURL& target_url,
       const GURL& referrer_url) const;
+
+ private:
+  // WrappingNetworkDelegate takes ownership of this.

[mmenke, 2014/11/26 15:23:52]

Not needed (It's implied by the fact it's a scoped_ptr)

[megjablon, 2014/12/01 19:26:56]

Done.

+  scoped_ptr<NetworkDelegate> wrapped_network_delegate_;
 };
 
 }  // namespace net
 
-#endif  // NET_BASE_NETWORK_DELEGATE_H_
+#endif  // NET_BASE_WRAPPING_NETWORK_DELEGATE_H_