Combine SSLCertErrorHandler and SSLErrorHandler

content/browser/ssl/ssl_error_handler.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.
 
 #ifndef CONTENT_BROWSER_SSL_SSL_ERROR_HANDLER_H_
 #define CONTENT_BROWSER_SSL_SSL_ERROR_HANDLER_H_
 
 #include <string>
 
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "content/common/content_export.h"
 #include "content/public/browser/global_request_id.h"
 #include "content/public/common/resource_type.h"
+#include "net/ssl/ssl_info.h"
 #include "url/gurl.h"
 
 namespace net {
-class SSLInfo;
 class URLRequest;
 }  // namespace net
 
 namespace content {
 
 class ResourceDispatcherHostImpl;
-class SSLCertErrorHandler;
 class SSLManager;
 class WebContents;
 
 // An SSLErrorHandler carries information from the IO thread to the UI thread
 // and is dispatched to the appropriate SSLManager when it arrives on the
 // UI thread.  Subclasses should override the OnDispatched/OnDispatchFailed
 // methods to implement the actions that should be taken on the UI thread.
 // These methods can call the different convenience methods ContinueRequest/
 // CancelRequest to perform any required action on the net::URLRequest the
 // ErrorHandler was created with.
@@ skipping 16 matching lines @@
     virtual void CancelSSLRequest(int error, const net::SSLInfo* ssl_info) = 0;
 
     // Called when SSLErrorHandler decides to continue the request despite the
     // SSL error.
     virtual void ContinueSSLRequest() = 0;
 
    protected:
     virtual ~Delegate() {}
   };
 
-  virtual SSLCertErrorHandler* AsSSLCertErrorHandler();
+  // Construct on the IO thread.
+  SSLErrorHandler(const base::WeakPtr<Delegate>& delegate,
+                  ResourceType resource_type,
+                  const GURL& url,
+                  const net::SSLInfo& ssl_info,
+                  bool fatal);
 
   // Find the appropriate SSLManager for the net::URLRequest and begin handling
   // this error.
   //
   // Call on UI thread.
   void Dispatch(const base::Callback<WebContents*(void)>& web_contents_getter);
 
-  // Available on either thread.
+  // These accessors are available on either thread
+  const net::SSLInfo& ssl_info() const { return ssl_info_; }
+  int cert_error() const { return cert_error_; }
+  bool fatal() const { return fatal_; }
   const GURL& request_url() const { return request_url_; }
-
-  // Available on either thread.
   ResourceType resource_type() const { return resource_type_; }
 
   // Cancels the associated net::URLRequest.
-  // This method can be called from OnDispatchFailed and OnDispatched.
   CONTENT_EXPORT void CancelRequest();
 
   // Continue the net::URLRequest ignoring any previous errors.  Note that some
   // errors cannot be ignored, in which case this will result in the request
   // being canceled.
-  // This method can be called from OnDispatchFailed and OnDispatched.
   void ContinueRequest();
 
   // Cancels the associated net::URLRequest and mark it as denied.  The renderer
   // processes such request in a special manner, optionally replacing them
   // with alternate content (typically frames content is replaced with a
   // warning message).
-  // This method can be called from OnDispatchFailed and OnDispatched.
   void DenyRequest();
 
   // Does nothing on the net::URLRequest but ensures the current instance ref
-  // count is decremented appropriately.  Subclasses that do not want to
-  // take any specific actions in their OnDispatched/OnDispatchFailed should
-  // call this.
+  // count is decremented appropriately.
   void TakeNoAction();
 
   // Returns the manager associated with this SSLErrorHandler.
   // Should only be accessed on the UI thread.
   SSLManager* GetManager() const;
 
  protected:

[nasko, 2016/08/08 18:47:12]

Why not remove "protected:"?

[estark, 2016/08/08 20:13:00]

Done.

+ private:
   friend class base::RefCountedThreadSafe<SSLErrorHandler>;
 
-  // Construct on the IO thread.
-  SSLErrorHandler(const base::WeakPtr<Delegate>& delegate,
-                  ResourceType resource_type,
-                  const GURL& url);
-
   virtual ~SSLErrorHandler();
 
-  // The following 2 methods are the methods subclasses should implement.
   virtual void OnDispatchFailed();
 
   // Can use the manager_ member.
   virtual void OnDispatched();
 
   // Should only be accessed on the UI thread.
   SSLManager* manager_;  // Our manager.
 
   // The delegate we are associated with.
   base::WeakPtr<Delegate> delegate_;
 
- private:
   // Completes the CancelRequest operation on the IO thread.
   // Call on the IO thread.
   void CompleteCancelRequest(int error);
 
   // Completes the ContinueRequest operation on the IO thread.
   //
   // Call on the IO thread.
   void CompleteContinueRequest();
 
   // Derefs this instance.
   // Call on the IO thread.
   void CompleteTakeNoAction();
 
-  // The URL that we requested.
-  // This read-only member can be accessed on any thread.
-  const GURL request_url_;
-
-  // What kind of resource is associated with the requested that generated
-  // that error.
-  // This read-only member can be accessed on any thread.
-  const ResourceType resource_type_;
-
   // A flag to make sure we notify the net::URLRequest exactly once.
   // Should only be accessed on the IO thread
   bool request_has_been_notified_;
 
+  // The below read-only members may be accessed on any thread.
+
+  // The URL that we requested.
+  const GURL request_url_;
+
+  // What kind of resource is associated with the requested that generated
+  // that error.
+  const ResourceType resource_type_;
+

[nasko, 2016/08/08 18:47:12]

nit: No need for empty lines between the members, especially when there are no
comments.

[estark, 2016/08/08 20:13:01]

I ended up adding comments to each of them for consistency, so I left the empty
lines before each comment to break it up a bit (and consistent with your comment
below).

+  const net::SSLInfo ssl_info_;
+
+  const int cert_error_;  // The error we represent.

[nasko, 2016/08/08 18:47:12]

I know this is a code move, but let's clean things up a bit. Usually comments on
member variables go right above them with an empty line before the comment.

[estark, 2016/08/08 20:13:01]

Done.

+
+  const bool fatal_;  // True if the error is from a host requiring
+                      // certificate errors to be fatal.
+
   DISALLOW_COPY_AND_ASSIGN(SSLErrorHandler);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_SSL_SSL_ERROR_HANDLER_H_