Remove thread hopping from SSLManager and SSLErrorHandler

no longer necessary with network service.

Bug: none
Change-Id: Ia96f8cb19c8c738d4cb310fbf161a655d6bdb307
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1888383Reviewed-by: Emily Stark <estark@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713591}

content/browser/ssl/ssl_error_handler.cc

@@ -4,8 +4,6 @@
 
 #include "content/browser/ssl/ssl_error_handler.h"
 
-#include "base/bind.h"
-#include "base/task/post_task.h"
 #include "content/browser/frame_host/navigation_controller_impl.h"
 #include "content/browser/frame_host/render_frame_host_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
@@ -19,37 +17,14 @@ using net::SSLInfo;
 
 namespace content {
 
-namespace {
-
-void CompleteCancelRequest(
-    const base::WeakPtr<SSLErrorHandler::Delegate>& delegate,
-    const net::SSLInfo& ssl_info,
-    int error) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  if (delegate.get())
-    delegate->CancelSSLRequest(error, &ssl_info);
-}
-
-void CompleteContinueRequest(
-    const base::WeakPtr<SSLErrorHandler::Delegate>& delegate) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  if (delegate.get()) {
-    delegate->ContinueSSLRequest();
-  }
-}
-
-}  // namespace
-
 SSLErrorHandler::SSLErrorHandler(WebContents* web_contents,
                                  const base::WeakPtr<Delegate>& delegate,
-                                 BrowserThread::ID delegate_thread,
                                  bool is_main_frame_request,
                                  const GURL& url,
                                  int net_error,
                                  const net::SSLInfo& ssl_info,
                                  bool fatal)
     : delegate_(delegate),
-      delegate_thread_(delegate_thread),
       request_url_(url),
       is_main_frame_request_(is_main_frame_request),
       ssl_info_(ssl_info),
@@ -57,45 +32,26 @@ SSLErrorHandler::SSLErrorHandler(WebContents* web_contents,
       fatal_(fatal),
       web_contents_(web_contents) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  DCHECK(delegate_thread == BrowserThread::UI ||
-         delegate_thread == BrowserThread::IO);
 }
 
 SSLErrorHandler::~SSLErrorHandler() {}
 
 void SSLErrorHandler::CancelRequest() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  if (delegate_thread_ == BrowserThread::UI) {
-    if (delegate_)
-      delegate_->CancelSSLRequest(net::ERR_ABORTED, &ssl_info());
-    return;
-  }
-  base::PostTask(FROM_HERE, {BrowserThread::IO},
-                 base::BindOnce(&CompleteCancelRequest, delegate_, ssl_info(),
-                                net::ERR_ABORTED));
+  if (delegate_)
+    delegate_->CancelSSLRequest(net::ERR_ABORTED, &ssl_info());
 }
 
 void SSLErrorHandler::DenyRequest() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  if (delegate_thread_ == BrowserThread::UI) {
-    if (delegate_)
-      delegate_->CancelSSLRequest(cert_error_, &ssl_info());
-    return;
-  }
-  base::PostTask(FROM_HERE, {BrowserThread::IO},
-                 base::BindOnce(&CompleteCancelRequest, delegate_, ssl_info(),
-                                cert_error_));
+  if (delegate_)
+    delegate_->CancelSSLRequest(cert_error_, &ssl_info());
 }
 
 void SSLErrorHandler::ContinueRequest() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  if (delegate_thread_ == BrowserThread::UI) {
-    if (delegate_)
-      delegate_->ContinueSSLRequest();
-    return;
-  }
-  base::PostTask(FROM_HERE, {BrowserThread::IO},
-                 base::BindOnce(&CompleteContinueRequest, delegate_));
+  if (delegate_)
+    delegate_->ContinueSSLRequest();
 }
 
 }  // namespace content

content/browser/ssl/ssl_error_handler.h

@@ -31,8 +31,6 @@ class WebContents;
 // call exactly one of those methods exactly once.
 class SSLErrorHandler {
  public:
-  // SSLErrorHandler's delegate lives on the UI or IO thread based on the passed
-  // in |delegate_thread|. The methods will be called on that thread.
   class CONTENT_EXPORT Delegate {
    public:
     // Called when SSLErrorHandler decides to cancel the request because of
@@ -49,7 +47,6 @@ class SSLErrorHandler {
 
   SSLErrorHandler(WebContents* web_contents,
                   const base::WeakPtr<Delegate>& delegate,
-                  BrowserThread::ID delegate_thread,
                   bool is_main_frame_request,
                   const GURL& url,
                   int net_error,
@@ -85,12 +82,8 @@ class SSLErrorHandler {
   void DenyRequest();
 
  private:
-  // This is called on |delegate_thread_|.
   base::WeakPtr<Delegate> delegate_;
 
-  // The thread that the delegate is called on.
-  BrowserThread::ID delegate_thread_;
-
   // The URL for the request that generated the error.
   const GURL request_url_;
 

content/browser/ssl/ssl_manager.cc

@@ -103,36 +103,6 @@ class SSLManagerSet : public base::SupportsUserData::Data {
   DISALLOW_COPY_AND_ASSIGN(SSLManagerSet);
 };
 
-void HandleSSLErrorOnUI(
-    const base::Callback<WebContents*(void)>& web_contents_getter,
-    const base::WeakPtr<SSLErrorHandler::Delegate>& delegate,
-    BrowserThread::ID delegate_thread,
-    bool is_main_frame_request,
-    const GURL& url,
-    int net_error,
-    const net::SSLInfo& ssl_info,
-    bool fatal) {
-  content::WebContents* web_contents = web_contents_getter.Run();
-  std::unique_ptr<SSLErrorHandler> handler(new SSLErrorHandler(
-      web_contents, delegate, delegate_thread, is_main_frame_request, url,
-      net_error, ssl_info, fatal));
-
-  if (!web_contents) {
-    // Requests can fail to dispatch because they don't have a WebContents. See
-    // https://crbug.com/86537. In this case we have to make a decision in this
-    // function.
-    handler->CancelRequest();
-    return;
-  }
-
-  NavigationControllerImpl* controller =
-      static_cast<NavigationControllerImpl*>(&web_contents->GetController());
-  controller->SetPendingNavigationSSLError(true);
-
-  SSLManager* manager = controller->ssl_manager();
-  manager->OnCertError(std::move(handler));
-}
-
 void LogMixedContentMetrics(MixedContentType type,
                             ukm::SourceId source_id,
                             ukm::UkmRecorder* recorder) {
@@ -157,20 +127,27 @@ void SSLManager::OnSSLCertificateError(
   DVLOG(1) << "OnSSLCertificateError() cert_error: " << net_error
            << " url: " << url.spec() << " cert_status: " << std::hex
            << ssl_info.cert_status;
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  content::WebContents* web_contents = web_contents_getter.Run();
+  std::unique_ptr<SSLErrorHandler> handler(
+      new SSLErrorHandler(web_contents, delegate, is_main_frame_request, url,
+                          net_error, ssl_info, fatal));
 
-  if (BrowserThread::CurrentlyOn(BrowserThread::UI)) {
-    HandleSSLErrorOnUI(web_contents_getter, delegate, BrowserThread::UI,
-                       is_main_frame_request, url, net_error, ssl_info, fatal);
+  if (!web_contents) {
+    // Requests can fail to dispatch because they don't have a WebContents. See
+    // https://crbug.com/86537. In this case we have to make a decision in this
+    // function.
+    handler->CancelRequest();
     return;
   }
 
-  // TODO(jam): remove the logic to call this from IO thread once the
-  // network service code path is the only one.
-  base::PostTask(
-      FROM_HERE, {BrowserThread::UI},
-      base::BindOnce(&HandleSSLErrorOnUI, web_contents_getter, delegate,
-                     BrowserThread::IO, is_main_frame_request, url, net_error,
-                     ssl_info, fatal));
+  NavigationControllerImpl* controller =
+      static_cast<NavigationControllerImpl*>(&web_contents->GetController());
+  controller->SetPendingNavigationSSLError(true);
+
+  SSLManager* manager = controller->ssl_manager();
+  manager->OnCertError(std::move(handler));
 }
 
 // static