use delegate to notify verification requests

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

Hide file tree

Showing 9 changed files with 34 additions and 45 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
@@ -55,7 +55,6 @@ void AtomCertVerifier::CertVerifyRequest::RunResult(int result) {
   for (auto& callback : callbacks_)
     callback.Run(result);
   cert_verifier_->RemoveRequest(this);
-  Release();
 }
 
 void AtomCertVerifier::CertVerifyRequest::DelegateToDefaultVerifier() {
@@ -77,7 +76,6 @@ void AtomCertVerifier::CertVerifyRequest::DelegateToDefaultVerifier() {
     for (auto& callback : callbacks_)
       callback.Run(rv);
     cert_verifier_->RemoveRequest(this);
-    Release();
   }
 }
 
@@ -122,7 +120,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(),
@@ -139,14 +137,13 @@ int AtomCertVerifier::Verify(
                                     cert,
                                     crl_set,
                                     verify_result,
-                                    out_req,
                                     net_log);
     requests_.insert(make_scoped_refptr(request));
 
     BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-                            base::Bind(&Browser::RequestCertVerification,
-                                       base::Unretained(Browser::Get()),
-                                       make_scoped_refptr(request)));
+                            base::Bind(&Delegate::RequestCertVerification,
+                                      base::Unretained(delegate_),
+                                      make_scoped_refptr(request)));
   }
 
   request->AddCompletionCallback(callback);

diff --git a/atom/browser/atom_cert_verifier.h b/atom/browser/atom_cert_verifier.h
@@ -38,34 +38,26 @@ class AtomCertVerifier : public net::CertVerifier {
   };
 
   class CertVerifyRequest
-      : public net::CertVerifier::Request,
-        public base::RefCountedThreadSafe<CertVerifyRequest> {
+      : public base::RefCountedThreadSafe<CertVerifyRequest> {
    public:
     CertVerifyRequest(
         AtomCertVerifier* cert_verifier,
         const RequestParams& key,
         scoped_refptr<net::X509Certificate> cert,
         scoped_refptr<net::CRLSet> crl_set,
         net::CertVerifyResult* verify_result,
-        scoped_ptr<Request>* out_req,
         const net::BoundNetLog& net_log)
         : cert_verifier_(cert_verifier),
           key_(key),
           certificate_(cert),
           crl_set_(crl_set),
           verify_result_(verify_result),
-          out_req_(out_req),
           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);
     void DelegateToDefaultVerifier();
     void ContinueWithResult(int result);
@@ -84,14 +76,14 @@ class AtomCertVerifier : public net::CertVerifier {
 
    private:
     friend class base::RefCountedThreadSafe<CertVerifyRequest>;
+    ~CertVerifyRequest() {}
 
     AtomCertVerifier* cert_verifier_;
     const RequestParams key_;
 
     scoped_refptr<net::X509Certificate> certificate_;
     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_;
 
@@ -103,8 +95,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 +152,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
@@ -157,13 +157,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
@@ -136,10 +136,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"
 
@@ -64,10 +63,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() {}
 };