Fix random crash on app quit.

Move AtomCTDelegate to brightray as RequireCTDelegate and transfer ownership to
brightray::URLRequestContextGetter. This fixes the wrong lifetime assumptions
that result in AtomCTDelegate being used after free in some scenarios.

Close #10051

atom/browser/atom_browser_context.cc

@@ -13,7 +13,6 @@
 #include "atom/browser/net/about_protocol_handler.h"
 #include "atom/browser/net/asar/asar_protocol_handler.h"
 #include "atom/browser/net/atom_cert_verifier.h"
-#include "atom/browser/net/atom_ct_delegate.h"
 #include "atom/browser/net/atom_network_delegate.h"
 #include "atom/browser/net/atom_url_request_job_factory.h"
 #include "atom/browser/net/http_protocol_handler.h"
@@ -72,7 +71,6 @@ AtomBrowserContext::AtomBrowserContext(const std::string& partition,
                                        bool in_memory,
                                        const base::DictionaryValue& options)
     : brightray::BrowserContext(partition, in_memory),
-      ct_delegate_(new AtomCTDelegate),
       network_delegate_(new AtomNetworkDelegate),
       cookie_delegate_(new AtomCookieDelegate) {
   // Construct user agent string.
@@ -192,8 +190,9 @@ content::PermissionManager* AtomBrowserContext::GetPermissionManager() {
   return permission_manager_.get();
 }
 
-std::unique_ptr<net::CertVerifier> AtomBrowserContext::CreateCertVerifier() {
-  return base::WrapUnique(new AtomCertVerifier(ct_delegate_.get()));
+std::unique_ptr<net::CertVerifier> AtomBrowserContext::CreateCertVerifier(
+    brightray::RequireCTDelegate* ct_delegate) {
+  return base::WrapUnique(new AtomCertVerifier(ct_delegate));
 }
 
 std::vector<std::string> AtomBrowserContext::GetCookieableSchemes() {
@@ -204,11 +203,6 @@ std::vector<std::string> AtomBrowserContext::GetCookieableSchemes() {
   return default_schemes;
 }
 
-net::TransportSecurityState::RequireCTDelegate*
-AtomBrowserContext::GetRequireCTDelegate() {
-  return ct_delegate_.get();
-}
-
 void AtomBrowserContext::RegisterPrefs(PrefRegistrySimple* pref_registry) {
   pref_registry->RegisterFilePathPref(prefs::kSelectFileLastDirectory,
                                       base::FilePath());

atom/browser/atom_browser_context.h

@@ -15,7 +15,6 @@
 namespace atom {
 
 class AtomBlobReader;
-class AtomCTDelegate;
 class AtomDownloadManagerDelegate;
 class AtomNetworkDelegate;
 class AtomPermissionManager;
@@ -40,10 +39,9 @@ class AtomBrowserContext : public brightray::BrowserContext {
       content::ProtocolHandlerMap* protocol_handlers) override;
   net::HttpCache::BackendFactory* CreateHttpCacheBackendFactory(
       const base::FilePath& base_path) override;
-  std::unique_ptr<net::CertVerifier> CreateCertVerifier() override;
+  std::unique_ptr<net::CertVerifier> CreateCertVerifier(
+      brightray::RequireCTDelegate* ct_delegate) override;
   std::vector<std::string> GetCookieableSchemes() override;
-  net::TransportSecurityState::RequireCTDelegate* GetRequireCTDelegate()
-      override;
 
   // content::BrowserContext:
   content::DownloadManagerDelegate* GetDownloadManagerDelegate() override;
@@ -69,7 +67,6 @@ class AtomBrowserContext : public brightray::BrowserContext {
   std::unique_ptr<WebViewManager> guest_manager_;
   std::unique_ptr<AtomPermissionManager> permission_manager_;
   std::unique_ptr<AtomBlobReader> blob_reader_;
-  std::unique_ptr<AtomCTDelegate> ct_delegate_;
   std::string user_agent_;
   bool use_cache_;
 

atom/browser/net/atom_cert_verifier.cc

@@ -5,11 +5,11 @@
 #include "atom/browser/net/atom_cert_verifier.h"
 
 #include "atom/browser/browser.h"
-#include "atom/browser/net/atom_ct_delegate.h"
 #include "atom/common/native_mate_converters/net_converter.h"
 #include "base/containers/linked_list.h"
 #include "base/memory/ptr_util.h"
 #include "base/memory/weak_ptr.h"
+#include "brightray/browser/net/require_ct_delegate.h"
 #include "content/public/browser/browser_thread.h"
 #include "net/base/net_errors.h"
 #include "net/cert/cert_verify_result.h"
@@ -147,7 +147,7 @@ class CertVerifierRequest : public AtomCertVerifier::Request {
   base::WeakPtrFactory<CertVerifierRequest> weak_ptr_factory_;
 };
 
-AtomCertVerifier::AtomCertVerifier(AtomCTDelegate* ct_delegate)
+AtomCertVerifier::AtomCertVerifier(brightray::RequireCTDelegate* ct_delegate)
     : default_cert_verifier_(net::CertVerifier::CreateDefault()),
       ct_delegate_(ct_delegate) {}
 

atom/browser/net/atom_cert_verifier.h

@@ -11,9 +11,14 @@
 
 #include "net/cert/cert_verifier.h"
 
+namespace brightray {
+
+class RequireCTDelegate;
+
+}  // namespace brightray
+
 namespace atom {
 
-class AtomCTDelegate;
 class CertVerifierRequest;
 
 struct VerifyRequestParams {
@@ -25,7 +30,7 @@ struct VerifyRequestParams {
 
 class AtomCertVerifier : public net::CertVerifier {
  public:
-  explicit AtomCertVerifier(AtomCTDelegate* ct_delegate);
+  explicit AtomCertVerifier(brightray::RequireCTDelegate* ct_delegate);
   virtual ~AtomCertVerifier();
 
   using VerifyProc = base::Callback<void(const VerifyRequestParams& request,
@@ -34,7 +39,7 @@ class AtomCertVerifier : public net::CertVerifier {
   void SetVerifyProc(const VerifyProc& proc);
 
   const VerifyProc verify_proc() const { return verify_proc_; }
-  AtomCTDelegate* ct_delegate() const { return ct_delegate_; }
+  brightray::RequireCTDelegate* ct_delegate() const { return ct_delegate_; }
   net::CertVerifier* default_verifier() const {
     return default_cert_verifier_.get();
   }
@@ -58,7 +63,7 @@ class AtomCertVerifier : public net::CertVerifier {
   std::map<RequestParams, CertVerifierRequest*> inflight_requests_;
   VerifyProc verify_proc_;
   std::unique_ptr<net::CertVerifier> default_cert_verifier_;
-  AtomCTDelegate* ct_delegate_;
+  brightray::RequireCTDelegate* ct_delegate_;
 
   DISALLOW_COPY_AND_ASSIGN(AtomCertVerifier);
 };

atom/browser/net/atom_ct_delegate.cc → brightray/browser/net/require_ct_delegate.cc

@@ -1,28 +1,28 @@
-// Copyright (c) 2016 GitHub, Inc.
+// Copyright (c) 2017 GitHub, Inc.
 // Use of this source code is governed by the MIT license that can be
 // found in the LICENSE file.
 
-#include "atom/browser/net/atom_ct_delegate.h"
+#include "brightray/browser/net/require_ct_delegate.h"
 
 #include "content/public/browser/browser_thread.h"
 
-namespace atom {
+namespace brightray {
 
-AtomCTDelegate::AtomCTDelegate() {}
+RequireCTDelegate::RequireCTDelegate() {}
 
-AtomCTDelegate::~AtomCTDelegate() {}
+RequireCTDelegate::~RequireCTDelegate() {}
 
-void AtomCTDelegate::AddCTExcludedHost(const std::string& host) {
+void RequireCTDelegate::AddCTExcludedHost(const std::string& host) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   ct_excluded_hosts_.insert(host);
 }
 
-void AtomCTDelegate::ClearCTExcludedHostsList() {
+void RequireCTDelegate::ClearCTExcludedHostsList() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   ct_excluded_hosts_.clear();
 }
 
-AtomCTDelegate::CTRequirementLevel AtomCTDelegate::IsCTRequiredForHost(
+RequireCTDelegate::CTRequirementLevel RequireCTDelegate::IsCTRequiredForHost(
     const std::string& host) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   if (!ct_excluded_hosts_.empty() &&
@@ -31,4 +31,4 @@ AtomCTDelegate::CTRequirementLevel AtomCTDelegate::IsCTRequiredForHost(
   return CTRequirementLevel::DEFAULT;
 }
 
-}  // namespace atom
+}  // namespace brightray

atom/browser/net/atom_ct_delegate.h → brightray/browser/net/require_ct_delegate.h

@@ -1,21 +1,22 @@
-// Copyright (c) 2016 GitHub, Inc.
+// Copyright (c) 2017 GitHub, Inc.
 // Use of this source code is governed by the MIT license that can be
 // found in the LICENSE file.
 
-#ifndef ATOM_BROWSER_NET_ATOM_CT_DELEGATE_H_
-#define ATOM_BROWSER_NET_ATOM_CT_DELEGATE_H_
+#ifndef BRIGHTRAY_BROWSER_NET_REQUIRE_CT_DELEGATE_H_
+#define BRIGHTRAY_BROWSER_NET_REQUIRE_CT_DELEGATE_H_
 
 #include <set>
 #include <string>
 
 #include "net/http/transport_security_state.h"
 
-namespace atom {
+namespace brightray {
 
-class AtomCTDelegate : public net::TransportSecurityState::RequireCTDelegate {
+class RequireCTDelegate
+    : public net::TransportSecurityState::RequireCTDelegate {
  public:
-  AtomCTDelegate();
-  ~AtomCTDelegate() override;
+  RequireCTDelegate();
+  ~RequireCTDelegate() override;
 
   void AddCTExcludedHost(const std::string& host);
   void ClearCTExcludedHostsList();
@@ -25,9 +26,9 @@ class AtomCTDelegate : public net::TransportSecurityState::RequireCTDelegate {
 
  private:
   std::set<std::string> ct_excluded_hosts_;
-  DISALLOW_COPY_AND_ASSIGN(AtomCTDelegate);
+  DISALLOW_COPY_AND_ASSIGN(RequireCTDelegate);
 };
 
-}  // namespace atom
+}  // namespace brightray
 
-#endif  // ATOM_BROWSER_NET_ATOM_CT_DELEGATE_H_
+#endif  // BRIGHTRAY_BROWSER_NET_REQUIRE_CT_DELEGATE_H_

brightray/browser/url_request_context_getter.cc

@@ -14,6 +14,7 @@
 #include "base/threading/worker_pool.h"
 #include "brightray/browser/net/devtools_network_controller_handle.h"
 #include "brightray/browser/net/devtools_network_transaction_factory.h"
+#include "brightray/browser/net/require_ct_delegate.h"
 #include "brightray/browser/net_log.h"
 #include "brightray/browser/network_delegate.h"
 #include "brightray/common/switches.h"
@@ -107,7 +108,8 @@ URLRequestContextGetter::Delegate::CreateHttpCacheBackendFactory(
 }
 
 std::unique_ptr<net::CertVerifier>
-URLRequestContextGetter::Delegate::CreateCertVerifier() {
+URLRequestContextGetter::Delegate::CreateCertVerifier(
+    RequireCTDelegate* ct_delegate) {
   return net::CertVerifier::CreateDefault();
 }
 
@@ -170,6 +172,7 @@ net::URLRequestContext* URLRequestContextGetter::GetURLRequestContext() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
   if (!url_request_context_.get()) {
+    ct_delegate_.reset(new RequireCTDelegate);
     auto& command_line = *base::CommandLine::ForCurrentProcess();
     url_request_context_.reset(new net::URLRequestContext);
 
@@ -280,10 +283,10 @@ net::URLRequestContext* URLRequestContextGetter::GetURLRequestContext() {
 
     std::unique_ptr<net::TransportSecurityState> transport_security_state =
         base::WrapUnique(new net::TransportSecurityState);
-    transport_security_state->SetRequireCTDelegate(
-        delegate_->GetRequireCTDelegate());
+    transport_security_state->SetRequireCTDelegate(ct_delegate_.get());
     storage_->set_transport_security_state(std::move(transport_security_state));
-    storage_->set_cert_verifier(delegate_->CreateCertVerifier());
+    storage_->set_cert_verifier(
+        delegate_->CreateCertVerifier(ct_delegate_.get()));
     storage_->set_ssl_config_service(delegate_->CreateSSLConfigService());
     storage_->set_http_auth_handler_factory(std::move(auth_handler_factory));
     std::unique_ptr<net::HttpServerProperties> server_properties(

brightray/browser/url_request_context_getter.h

@@ -33,6 +33,7 @@ class URLRequestJobFactory;
 
 namespace brightray {
 
+class RequireCTDelegate;
 class DevToolsNetworkControllerHandle;
 class MediaDeviceIDSalt;
 class NetLog;
@@ -53,13 +54,10 @@ class URLRequestContextGetter : public net::URLRequestContextGetter {
     CreateURLRequestJobFactory(content::ProtocolHandlerMap* protocol_handlers);
     virtual net::HttpCache::BackendFactory* CreateHttpCacheBackendFactory(
         const base::FilePath& base_path);
-    virtual std::unique_ptr<net::CertVerifier> CreateCertVerifier();
+    virtual std::unique_ptr<net::CertVerifier> CreateCertVerifier(
+        RequireCTDelegate* ct_delegate);
     virtual net::SSLConfigService* CreateSSLConfigService();
     virtual std::vector<std::string> GetCookieableSchemes();
-    virtual net::TransportSecurityState::RequireCTDelegate*
-    GetRequireCTDelegate() {
-      return nullptr;
-    }
     virtual MediaDeviceIDSalt* GetMediaDeviceIDSalt() { return nullptr; }
   };
 
@@ -98,6 +96,7 @@ class URLRequestContextGetter : public net::URLRequestContextGetter {
 
   std::string user_agent_;
 
+  std::unique_ptr<RequireCTDelegate> ct_delegate_;
   std::unique_ptr<net::ProxyConfigService> proxy_config_service_;
   std::unique_ptr<net::NetworkDelegate> network_delegate_;
   std::unique_ptr<net::URLRequestContextStorage> storage_;

brightray/filenames.gypi

@@ -61,6 +61,8 @@
       'browser/net/devtools_network_transaction.h',
       'browser/net/devtools_network_upload_data_stream.cc',
       'browser/net/devtools_network_upload_data_stream.h',
+      'browser/net/require_ct_delegate.cc',
+      'browser/net/require_ct_delegate.h',
       'browser/net_log.cc',
       'browser/net_log.h',
       'browser/network_delegate.cc',

filenames.gypi

@@ -250,8 +250,6 @@
       'atom/browser/net/asar/url_request_asar_job.h',
       'atom/browser/net/atom_cert_verifier.cc',
       'atom/browser/net/atom_cert_verifier.h',
-      'atom/browser/net/atom_ct_delegate.cc',
-      'atom/browser/net/atom_ct_delegate.h',
       'atom/browser/net/atom_cookie_delegate.cc',
       'atom/browser/net/atom_cookie_delegate.h',
       'atom/browser/net/atom_network_delegate.cc',