Have fewer CertDatabase files.

CertDatabase is just a giant singleton observer list at this point,
except for the Mac code which internally registers some notifier.

Since URLRequestContext no longer assumes the platform verifier at this
point, the Right Way to do this would probably be for CertVerifier to
have an observer and, separately, for there to be an external way to
tell a URLRequestContext to drop all client certificate identities.
But, to start with, trim a bunch of unnecessarily duplicated code.

Change-Id: I11b15b10a0eb4da0f528581da1c57dad9ebbc30d
Reviewed-on: https://chromium-review.googlesource.com/619929
Commit-Queue: David Benjamin <davidben@chromium.org>
Reviewed-by: Matt Mueller <mattm@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557624}

android_webview/browser/aw_contents_statics.cc

@@ -41,7 +41,7 @@ void ClientCertificatesCleared(const JavaRef<jobject>& callback) {
 
 void NotifyClientCertificatesChanged() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  net::CertDatabase::GetInstance()->OnAndroidKeyStoreChanged();
+  net::CertDatabase::GetInstance()->NotifyObserversCertDBChanged();
 }
 
 void SafeBrowsingWhitelistAssigned(const JavaRef<jobject>& callback,

chrome/browser/ui/android/ssl_client_certificate_request.cc

@@ -160,7 +160,7 @@ static void JNI_SSLClientCertificateRequest_OnSystemRequestCompletion(
 }
 
 static void NotifyClientCertificatesChanged() {
-  net::CertDatabase::GetInstance()->OnAndroidKeyStoreChanged();
+  net::CertDatabase::GetInstance()->NotifyObserversCertDBChanged();
 }
 
 static void

net/BUILD.gn

@@ -157,7 +157,6 @@ component("net") {
     "cert/asn1_util.h",
     "cert/cert_database.cc",
     "cert/cert_database.h",
-    "cert/cert_database_stub.cc",
     "cert/cert_status_flags.cc",
     "cert/cert_status_flags.h",
     "cert/cert_status_flags_list.h",
@@ -513,11 +512,7 @@ component("net") {
       "base/winsock_util.h",
       "cert/caching_cert_verifier.cc",
       "cert/caching_cert_verifier.h",
-      "cert/cert_database_android.cc",
-      "cert/cert_database_ios.cc",
       "cert/cert_database_mac.cc",
-      "cert/cert_database_nss.cc",
-      "cert/cert_database_win.cc",
       "cert/cert_net_fetcher.cc",
       "cert/cert_net_fetcher.h",
       "cert/cert_verify_proc.cc",
@@ -1902,10 +1897,7 @@ component("net") {
     }
 
     if (!is_nacl) {
-      sources -= [
-        "base/network_interfaces_nacl.cc",
-        "cert/cert_database_stub.cc",
-      ]
+      sources -= [ "base/network_interfaces_nacl.cc" ]
     }
 
     # Use getifaddrs() on POSIX platforms, except Linux and Android.
@@ -1922,7 +1914,6 @@ component("net") {
 
     if (!use_nss_certs) {
       sources -= [
-        "cert/cert_database_nss.cc",
         "cert/internal/trust_store_nss.cc",
         "cert/internal/trust_store_nss.h",
         "cert/known_roots_nss.cc",
@@ -2045,7 +2036,6 @@ component("net") {
       sources += [
         "base/network_interfaces_fuchsia.cc",
         "base/platform_mime_util_fuchsia.cc",
-        "cert/cert_database_fuchsia.cc",
         "cert/test_root_certs_fuchsia.cc",
       ]
       deps += [ "//third_party/fuchsia-sdk:netstack" ]

net/cert/cert_database.cc

@@ -29,4 +29,13 @@ void CertDatabase::NotifyObserversCertDBChanged() {
   observer_list_->Notify(FROM_HERE, &Observer::OnCertDBChanged);
 }
 
+CertDatabase::CertDatabase()
+    : observer_list_(new base::ObserverListThreadSafe<Observer>) {}
+
+CertDatabase::~CertDatabase() {
+#if defined(OS_MACOSX) && !defined(OS_IOS)
+  ReleaseNotifier();
+#endif
+}
+
 }  // namespace net

net/cert/cert_database.h

@@ -9,8 +9,8 @@
 
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
+#include "build/build_config.h"
 #include "net/base/net_export.h"
-#include "net/cert/x509_certificate.h"
 
 namespace base {
 template <typename T> struct DefaultSingletonTraits;
@@ -21,12 +21,14 @@ class ObserverListThreadSafe;
 
 namespace net {
 
-// This class provides cross-platform functions to verify and add user
-// certificates, and to observe changes to the underlying certificate stores.
-
-// TODO(gauravsh): This class could be augmented with methods
-// for all operations that manipulate the underlying system
-// certificate store.
+// This class allows callers to observe changes to the underlying certificate
+// stores.
+//
+// TODO(davidben): This class is really just a giant global ObserverList. It
+// does not do anything with the platform certificate and, in principle, //net's
+// dependency on the platform is abstracted behind the CertVerifier and
+// ClientCertStore interfaces. Ideally these signals would originate out of
+// those interfaces' platform implementations.
 
 class NET_EXPORT CertDatabase {
  public:
@@ -71,21 +73,9 @@ class NET_EXPORT CertDatabase {
   void SetMessageLoopForKeychainEvents();
 #endif
 
-#if defined(OS_ANDROID)
-  // On Android, the system key store may be replaced with a device-specific
-  // KeyStore used for storing client certificates. When the Java side replaces
-  // the KeyStore used for client certificates, notifies the observers as if a
-  // new client certificate was added.
-  void OnAndroidKeyStoreChanged();
-
-  // On Android, the system database is used. When the system notifies the
-  // application that the certificates changed, the observers must be notified.
-  void OnAndroidKeyChainChanged();
-#endif
-
   // Synthetically injects notifications to all observers. In general, this
   // should only be called by the creator of the CertDatabase. Used to inject
-  // notifcations from other DB interfaces.
+  // notifications from other DB interfaces.
   void NotifyObserversCertDBChanged();
 
  private:
@@ -97,9 +87,11 @@ class NET_EXPORT CertDatabase {
   const scoped_refptr<base::ObserverListThreadSafe<Observer>> observer_list_;
 
 #if defined(OS_MACOSX) && !defined(OS_IOS)
+  void ReleaseNotifier();
+
   class Notifier;
   friend class Notifier;
-  std::unique_ptr<Notifier> notifier_;
+  Notifier* notifier_ = nullptr;
 #endif
 
   DISALLOW_COPY_AND_ASSIGN(CertDatabase);

net/cert/cert_database_android.cc

-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "net/cert/cert_database.h"
-
-#include "base/logging.h"
-#include "base/observer_list_threadsafe.h"
-#include "net/base/net_errors.h"
-
-namespace net {
-
-CertDatabase::CertDatabase()
-    : observer_list_(new base::ObserverListThreadSafe<Observer>) {
-}
-
-CertDatabase::~CertDatabase() {}
-
-void CertDatabase::OnAndroidKeyStoreChanged() {
-  NotifyObserversCertDBChanged();
-}
-
-void CertDatabase::OnAndroidKeyChainChanged() {
-  observer_list_->Notify(FROM_HERE, &Observer::OnCertDBChanged);
-}
-
-}  // namespace net

net/cert/cert_database_fuchsia.cc

-// Copyright 2017 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "net/cert/cert_database.h"
-
-#include "base/observer_list_threadsafe.h"
-
-namespace net {
-
-CertDatabase::CertDatabase()
-    : observer_list_(new base::ObserverListThreadSafe<Observer>) {}
-
-CertDatabase::~CertDatabase() {}
-
-}  // namespace net

net/cert/cert_database_ios.cc

-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "net/cert/cert_database.h"
-
-#include "base/logging.h"
-#include "base/observer_list_threadsafe.h"
-#include "net/base/net_errors.h"
-
-namespace net {
-
-CertDatabase::CertDatabase()
-    : observer_list_(new base::ObserverListThreadSafe<Observer>) {
-}
-
-CertDatabase::~CertDatabase() {}
-
-}  // namespace net

net/cert/cert_database_mac.cc

@@ -116,21 +116,16 @@ OSStatus CertDatabase::Notifier::KeychainCallback(
 }
 
 void CertDatabase::SetMessageLoopForKeychainEvents() {
-  // Shutdown will take care to delete the notifier on the right thread.
-  if (notifier_.get())
-    notifier_.release()->Shutdown();
-
-  notifier_.reset(new Notifier(this, base::MessageLoopCurrentForUI::Get()));
+  ReleaseNotifier();
+  notifier_ = new Notifier(this, base::MessageLoopCurrentForUI::Get());
 }
 
-CertDatabase::CertDatabase()
-    : observer_list_(new base::ObserverListThreadSafe<Observer>) {
-}
-
-CertDatabase::~CertDatabase() {
+void CertDatabase::ReleaseNotifier() {
   // Shutdown will take care to delete the notifier on the right thread.
-  if (notifier_.get())
-    notifier_.release()->Shutdown();
+  if (notifier_) {
+    notifier_->Shutdown();
+    notifier_ = nullptr;
+  }
 }
 
 }  // namespace net

net/cert/cert_database_nss.cc

-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "net/cert/cert_database.h"
-
-#include "base/observer_list_threadsafe.h"
-
-namespace net {
-
-CertDatabase::CertDatabase()
-    : observer_list_(new base::ObserverListThreadSafe<Observer>) {
-}
-
-CertDatabase::~CertDatabase() = default;
-
-}  // namespace net

net/cert/cert_database_stub.cc

-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "net/cert/cert_database.h"
-
-#include "base/observer_list_threadsafe.h"
-
-namespace net {
-
-CertDatabase::CertDatabase()
-    : observer_list_(new base::ObserverListThreadSafe<Observer>) {}
-
-CertDatabase::~CertDatabase() {}
-
-}  // namespace net

net/cert/cert_database_win.cc

-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "net/cert/cert_database.h"
-
-#include "base/observer_list_threadsafe.h"
-
-namespace net {
-
-CertDatabase::CertDatabase()
-    : observer_list_(new base::ObserverListThreadSafe<Observer>) {
-}
-
-CertDatabase::~CertDatabase() {}
-
-}  // namespace net

net/cert/x509_util_android.cc

@@ -11,7 +11,7 @@ namespace net {
 
 void JNI_X509Util_NotifyKeyChainChanged(JNIEnv* env,
                                         const JavaParamRef<jclass>& clazz) {
-  CertDatabase::GetInstance()->OnAndroidKeyChainChanged();
+  CertDatabase::GetInstance()->NotifyObserversCertDBChanged();
 }
 
 }  // namespace net