Make SSLErrorHandler UI-thread-only and not-refcounted

content/browser/ssl/ssl_error_handler.h

Index: content/browser/ssl/ssl_error_handler.h
diff --git a/content/browser/ssl/ssl_error_handler.h b/content/browser/ssl/ssl_error_handler.h
index a713377f8c9d390d53bbf3a85c29f6f93556b4cc..438a0089ffd1656830486677c97cf2b9cb69cc4b 100644
--- a/content/browser/ssl/ssl_error_handler.h
+++ b/content/browser/ssl/ssl_error_handler.h
@@ -26,25 +26,15 @@ class ResourceDispatcherHostImpl;
 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.
-//
-// IMPORTANT NOTE:
-//
-//   If you are not doing anything in OnDispatched/OnDispatchFailed, make sure
-//   you call TakeNoAction().  This is necessary for ensuring the instance is
-//   not leaked.
-//
-class SSLErrorHandler : public base::RefCountedThreadSafe<SSLErrorHandler> {
+// SSLErrorHandler is the UI-thread class for handling SSL certificate
+// errors. Users of this class can call CancelRequest(),
+// ContinueRequest(), or DenyRequest() when a decision about how to
+// handle the error has been made. Users of this class must
+// call exactly one of those methods exactly once.
+class SSLErrorHandler {
  public:
-  // Delegate functions must be called from IO thread. Finally,
-  // CancelSSLRequest() or ContinueSSLRequest() will be called after
-  // SSLErrorHandler makes a decision on the SSL error.
+  // SSLErrorHandler's delegate lives on the IO thread, and thus these
+  // delegate methods must be called on the IO thread only.
   class CONTENT_EXPORT Delegate {
    public:
     // Called when SSLErrorHandler decides to cancel the request because of
@@ -59,26 +49,27 @@ class SSLErrorHandler : public base::RefCountedThreadSafe<SSLErrorHandler> {
     virtual ~Delegate() {}
   };
 
-  // Construct on the IO thread.
-  SSLErrorHandler(const base::WeakPtr<Delegate>& delegate,
+  SSLErrorHandler(WebContents* web_contents,
+                  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);
+  virtual ~SSLErrorHandler();
 
-  // 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_; }
+
   ResourceType resource_type() const { return resource_type_; }
 
+  WebContents* web_contents() const { return web_contents_; }
+
+  int cert_error() const { return cert_error_; }
+
+  bool fatal() const { return fatal_; }
+
   // Cancels the associated net::URLRequest.
   CONTENT_EXPORT void CancelRequest();
 
@@ -93,63 +84,22 @@ class SSLErrorHandler : public base::RefCountedThreadSafe<SSLErrorHandler> {
   // warning message).
   void DenyRequest();
 
-  // Does nothing on the net::URLRequest but ensures the current instance ref
-  // 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:
  private:
-  friend class base::RefCountedThreadSafe<SSLErrorHandler>;
-
-  virtual ~SSLErrorHandler();
-
-  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.
+  // This must not be dereferenced on the UI thread. SSLErrorHandler
+  // simply holds on to the reference to be passed back to the IO thread
+  // to enact a decision about the error once one has been made.
   base::WeakPtr<Delegate> delegate_;
 
-  // 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();
-
-  // 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_;
-
   const net::SSLInfo ssl_info_;
-
-  const int cert_error_;  // The error we represent.
-
-  const bool fatal_;  // True if the error is from a host requiring
-                      // certificate errors to be fatal.
+  // A net error code describing the error that occurred.

[nasko, 2016/08/08 20:57:06]

nit: Empty lines before comments.

[estark, 2016/08/09 05:35:01]

Done.

+  const int cert_error_;
+  // True if the error is from a host requiring certificate errors to be fatal.
+  const bool fatal_;
+  WebContents* web_contents_;
 
   DISALLOW_COPY_AND_ASSIGN(SSLErrorHandler);
 };