Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(211)

Unified Diff: content/browser/ssl/ssl_error_handler.h

Issue 2213193005: Make SSLErrorHandler UI-thread-only and not-refcounted (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: remove UI-thread dereference for DCHECK Created 4 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | content/browser/ssl/ssl_error_handler.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 b981e7ea911bf47c9ce5927d878cdd48b4e0aea7..d00a43ba86948f051441172d258e3f492cce873c 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,65 +84,31 @@ 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;
-
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.
+ // The URL for the request that generated the error.
const GURL request_url_;
// What kind of resource is associated with the request that generated
// the error.
const ResourceType resource_type_;
- // The SSLInfo associated with the request that generated the error.
+ // The net::SSLInfo associated with the request that generated the error.
const net::SSLInfo ssl_info_;
- // The net error code that occurred on the request.
+ // A net error code describing the error that occurred.
const int cert_error_;
// True if the error is from a host requiring certificate errors to be fatal.
const bool fatal_;
+ // The WebContents associated with the request that generated the error.
+ WebContents* web_contents_;
+
DISALLOW_COPY_AND_ASSIGN(SSLErrorHandler);
};
« no previous file with comments | « no previous file | content/browser/ssl/ssl_error_handler.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698