use delegate to notify verification requests

electron · Nov 16, 2015 · 92c3ee8 · 92c3ee8
1 parent 37e6e6f
commit 92c3ee8
Show file tree

Hide file tree

Showing 9 changed files with 38 additions and 53 deletions.
diff --git a/atom/browser/api/atom_api_session.cc b/atom/browser/api/atom_api_session.cc
@@ -13,7 +13,6 @@
 #include "atom/browser/api/save_page_handler.h"
 #include "atom/browser/atom_browser_context.h"
 #include "atom/browser/atom_browser_main_parts.h"
-#include "atom/browser/browser.h"
 #include "atom/common/native_mate_converters/callback.h"
 #include "atom/common/native_mate_converters/gurl_converter.h"
 #include "atom/common/native_mate_converters/file_path_converter.h"
@@ -253,9 +252,7 @@ void PassVerificationResult(
 Session::Session(AtomBrowserContext* browser_context)
     : browser_context_(browser_context) {
   AttachAsUserData(browser_context);
-
-  // Observe Browser to get certificate verification notification.
-  Browser::Get()->AddObserver(this);
+  browser_context->cert_verifier()->SetDelegate(this);
 
   // Observe DownloadManger to get download notifications.
   content::BrowserContext::GetDownloadManager(browser_context)->
@@ -265,11 +262,10 @@ Session::Session(AtomBrowserContext* browser_context)
 Session::~Session() {
   content::BrowserContext::GetDownloadManager(browser_context())->
       RemoveObserver(this);
-  Browser::Get()->RemoveObserver(this);
   Destroy();
 }
 
-void Session::OnCertVerification(
+void Session::RequestCertVerification(
     const scoped_refptr<AtomCertVerifier::CertVerifyRequest>& request) {
   bool prevent_default = Emit(
       "verify-certificate",

diff --git a/atom/browser/api/atom_api_session.h b/atom/browser/api/atom_api_session.h
@@ -9,7 +9,6 @@
 
 #include "atom/browser/api/trackable_object.h"
 #include "atom/browser/atom_cert_verifier.h"
-#include "atom/browser/browser_observer.h"
 #include "content/public/browser/download_manager.h"
 #include "native_mate/handle.h"
 #include "net/base/completion_callback.h"
@@ -36,7 +35,7 @@ class AtomBrowserContext;
 namespace api {
 
 class Session: public mate::TrackableObject<Session>,
-               public BrowserObserver,
+               public AtomCertVerifier::Delegate,
                public content::DownloadManager::Observer {
  public:
   using ResolveProxyCallback = base::Callback<void(std::string)>;
@@ -55,8 +54,8 @@ class Session: public mate::TrackableObject<Session>,
   explicit Session(AtomBrowserContext* browser_context);
   ~Session();
 
-  // BrowserObserver:
-  void OnCertVerification(
+  // AtomCertVerifier::Delegate:
+  void RequestCertVerification(
       const scoped_refptr<AtomCertVerifier::CertVerifyRequest>&) override;
 
   // content::DownloadManager::Observer:

diff --git a/atom/browser/atom_browser_context.cc b/atom/browser/atom_browser_context.cc
@@ -61,6 +61,7 @@ std::string RemoveWhitespace(const std::string& str) {
 AtomBrowserContext::AtomBrowserContext(const std::string& partition,
                                        bool in_memory)
     : brightray::BrowserContext(partition, in_memory),
+      cert_verifier_(new AtomCertVerifier),
       job_factory_(new AtomURLRequestJobFactory),
       allow_ntlm_everywhere_(false) {
 }
@@ -160,7 +161,7 @@ content::BrowserPluginGuestManager* AtomBrowserContext::GetGuestManager() {
 }
 
 net::CertVerifier* AtomBrowserContext::CreateCertVerifier() {
-  return new AtomCertVerifier;
+  return cert_verifier_;
 }
 
 net::SSLConfigService* AtomBrowserContext::CreateSSLConfigService() {

diff --git a/atom/browser/atom_browser_context.h b/atom/browser/atom_browser_context.h
@@ -12,6 +12,7 @@
 namespace atom {
 
 class AtomDownloadManagerDelegate;
+class AtomCertVerifier;
 class AtomURLRequestJobFactory;
 class WebViewManager;
 
@@ -40,13 +41,16 @@ class AtomBrowserContext : public brightray::BrowserContext {
 
   void AllowNTLMCredentialsForAllDomains(bool should_allow);
 
+  AtomCertVerifier* cert_verifier() const { return cert_verifier_; }
+
   AtomURLRequestJobFactory* job_factory() const { return job_factory_; }
 
  private:
   scoped_ptr<AtomDownloadManagerDelegate> download_manager_delegate_;
   scoped_ptr<WebViewManager> guest_manager_;
 
   // Managed by brightray::BrowserContext.
+  AtomCertVerifier* cert_verifier_;
   AtomURLRequestJobFactory* job_factory_;
 
   bool allow_ntlm_everywhere_;

diff --git a/atom/browser/atom_cert_verifier.cc b/atom/browser/atom_cert_verifier.cc
@@ -6,7 +6,6 @@
 
 #include "atom/browser/browser.h"
 #include "atom/common/native_mate_converters/net_converter.h"
-#include "base/callback_helpers.h"
 #include "base/sha1.h"
 #include "base/stl_util.h"
 #include "content/public/browser/browser_thread.h"
@@ -55,7 +54,6 @@ void AtomCertVerifier::CertVerifyRequest::RunResult(int result) {
   for (auto& callback : callbacks_)
     callback.Run(result);
   cert_verifier_->RemoveRequest(this);
-  Release();
 }
 
 void AtomCertVerifier::CertVerifyRequest::DelegateToDefaultVerifier() {
@@ -70,15 +68,11 @@ void AtomCertVerifier::CertVerifyRequest::DelegateToDefaultVerifier() {
       verify_result_,
       base::Bind(&CertVerifyRequest::RunResult,
                  weak_ptr_factory_.GetWeakPtr()),
-      &new_out_req_,
+      out_req_,
       net_log_);
 
-  if (rv != net::ERR_IO_PENDING && !callbacks_.empty()) {
-    for (auto& callback : callbacks_)
-      callback.Run(rv);
-    cert_verifier_->RemoveRequest(this);
-    Release();
-  }
+  if (rv != net::ERR_IO_PENDING)
+    RunResult(rv);
 }
 
 void AtomCertVerifier::CertVerifyRequest::ContinueWithResult(int result) {
@@ -103,7 +97,8 @@ void AtomCertVerifier::CertVerifyRequest::ContinueWithResult(int result) {
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
-AtomCertVerifier::AtomCertVerifier() {
+AtomCertVerifier::AtomCertVerifier()
+    : delegate_(nullptr) {
   default_cert_verifier_.reset(net::CertVerifier::CreateDefault());
 }
 
@@ -122,7 +117,7 @@ int AtomCertVerifier::Verify(
     const net::BoundNetLog& net_log) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
-  if (callback.is_null() || !verify_result || hostname.empty())
+  if (callback.is_null() || !verify_result || hostname.empty() || !delegate_)
     return net::ERR_INVALID_ARGUMENT;
 
   const RequestParams key(cert->fingerprint(),
@@ -144,8 +139,8 @@ int AtomCertVerifier::Verify(
     requests_.insert(make_scoped_refptr(request));
 
     BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-                            base::Bind(&Browser::RequestCertVerification,
-                                       base::Unretained(Browser::Get()),
+                            base::Bind(&Delegate::RequestCertVerification,
+                                       base::Unretained(delegate_),
                                        make_scoped_refptr(request)));
   }
 

diff --git a/atom/browser/atom_cert_verifier.h b/atom/browser/atom_cert_verifier.h
@@ -38,8 +38,7 @@ class AtomCertVerifier : public net::CertVerifier {
   };
 
   class CertVerifyRequest
-      : public net::CertVerifier::Request,
-        public base::RefCountedThreadSafe<CertVerifyRequest> {
+      : public base::RefCountedThreadSafe<CertVerifyRequest> {
    public:
     CertVerifyRequest(
         AtomCertVerifier* cert_verifier,
@@ -58,12 +57,6 @@ class AtomCertVerifier : public net::CertVerifier {
           net_log_(net_log),
           handled_(false),
           weak_ptr_factory_(this) {
-      out_req_->reset(this);
-      new_out_req_.reset(new net::CertVerifier::Request());
-    }
-
-    ~CertVerifyRequest() {
-      out_req_->reset();
     }
 
     void RunResult(int result);
@@ -84,6 +77,7 @@ class AtomCertVerifier : public net::CertVerifier {
 
    private:
     friend class base::RefCountedThreadSafe<CertVerifyRequest>;
+    ~CertVerifyRequest() {}
 
     AtomCertVerifier* cert_verifier_;
     const RequestParams key_;
@@ -92,7 +86,6 @@ class AtomCertVerifier : public net::CertVerifier {
     scoped_refptr<net::CRLSet> crl_set_;
     net::CertVerifyResult* verify_result_;
     scoped_ptr<Request>* out_req_;
-    scoped_ptr<Request> new_out_req_;
     const net::BoundNetLog net_log_;
 
     std::vector<net::CompletionCallback> callbacks_;
@@ -103,8 +96,22 @@ class AtomCertVerifier : public net::CertVerifier {
     DISALLOW_COPY_AND_ASSIGN(CertVerifyRequest);
   };
 
+  class Delegate {
+   public:
+    Delegate() {}
+    virtual ~Delegate() {}
+
+    // Called on UI thread.
+    virtual void RequestCertVerification(
+        const scoped_refptr<CertVerifyRequest>& request) {}
+  };
+
   AtomCertVerifier();
-  ~AtomCertVerifier() override;
+  virtual ~AtomCertVerifier();
+
+  void SetDelegate(Delegate* delegate) {
+    delegate_ = delegate;
+  }
 
  protected:
   // net::CertVerifier:
@@ -146,6 +153,8 @@ class AtomCertVerifier : public net::CertVerifier {
                CertVerifyRequestComparator>;
   ActiveRequestSet requests_;
 
+  Delegate* delegate_;
+
   scoped_ptr<net::CertVerifier> default_cert_verifier_;
 
   DISALLOW_COPY_AND_ASSIGN(AtomCertVerifier);

diff --git a/atom/browser/browser.cc b/atom/browser/browser.cc
@@ -7,7 +7,6 @@
 #include <string>
 
 #include "atom/browser/atom_browser_main_parts.h"
-#include "atom/browser/atom_cert_verifier.h"
 #include "atom/browser/native_window.h"
 #include "atom/browser/window_list.h"
 #include "base/message_loop/message_loop.h"
@@ -157,13 +156,6 @@ void Browser::RequestLogin(LoginHandler* login_handler) {
   FOR_EACH_OBSERVER(BrowserObserver, observers_, OnLogin(login_handler));
 }
 
-void Browser::RequestCertVerification(
-    const scoped_refptr<AtomCertVerifier::CertVerifyRequest>& request) {
-  FOR_EACH_OBSERVER(BrowserObserver,
-                    observers_,
-                    OnCertVerification(request));
-}
-
 void Browser::NotifyAndShutdown() {
   if (is_shutdown_)
     return;

diff --git a/atom/browser/browser.h b/atom/browser/browser.h
@@ -29,7 +29,6 @@ class MenuModel;
 
 namespace atom {
 
-class AtomCertVerifier;
 class LoginHandler;
 
 // This class is used for control application-wide operations.
@@ -136,10 +135,6 @@ class Browser : public WindowListObserver {
   // Request basic auth login.
   void RequestLogin(LoginHandler* login_handler);
 
-  // Request Server Certificate Verification.
-  void RequestCertVerification(
-      const scoped_refptr<AtomCertVerifier::CertVerifyRequest>& request);
-
   void AddObserver(BrowserObserver* obs) {
     observers_.AddObserver(obs);
   }

diff --git a/atom/browser/browser_observer.h b/atom/browser/browser_observer.h
@@ -7,7 +7,6 @@
 
 #include <string>
 
-#include "atom/browser/atom_cert_verifier.h"
 #include "base/memory/scoped_ptr.h"
 #include "content/public/browser/client_certificate_delegate.h"
 
@@ -17,7 +16,6 @@ class WebContents;
 
 namespace net {
 class SSLCertRequestInfo;
-class X509Certificate;
 }
 
 namespace atom {
@@ -64,10 +62,6 @@ class BrowserObserver {
   // The browser requests HTTP login.
   virtual void OnLogin(LoginHandler* login_handler) {}
 
-  // The browser requests Server Certificate Verification.
-  virtual void OnCertVerification(
-      const scoped_refptr<AtomCertVerifier::CertVerifyRequest>& request) {}
-
  protected:
   virtual ~BrowserObserver() {}
 };