Add OnUserCertRemoved to CertDatabase::Observer

Change-Id: Ide0e496c6919282ee707847c4d11db8a182eeac2

BUG=24403
TEST=Add a certificate. Ensure that it shows up when adding Wifi or VPN networks. Remove the certificate. Make sure it is no longer available when adding Wifi or VPN networks. Also, see issue.


Review URL: http://codereview.chromium.org/9280014

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@119536 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 27 insertions, 6 deletions

diff --git a/net/base/cert_database.cc b/net/base/cert_database.cc
index b8c9aa3..6f9c609 100644
--- a/net/base/cert_database.cc
+++ b/net/base/cert_database.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -51,6 +51,12 @@ void CertDatabase::NotifyObserversOfUserCertAdded(const X509Certificate* cert) {
       &CertDatabase::Observer::OnUserCertAdded, make_scoped_refptr(cert));
 }
 
+void CertDatabase::NotifyObserversOfUserCertRemoved(
+    const X509Certificate* cert) {
+  CertDatabaseNotifier::GetInstance()->observer_list_->Notify(
+      &CertDatabase::Observer::OnUserCertRemoved, make_scoped_refptr(cert));
+}
+
 void CertDatabase::NotifyObserversOfCertTrustChanged(
     const X509Certificate* cert) {
   CertDatabaseNotifier::GetInstance()->observer_list_->Notify(
diff --git a/net/base/cert_database.h b/net/base/cert_database.h
index cfd670c..9761bd1 100644
--- a/net/base/cert_database.h
+++ b/net/base/cert_database.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -43,11 +43,16 @@ class NET_EXPORT CertDatabase {
     virtual ~Observer() {}
 
     // Will be called when a new user certificate is added.
-    // Note that |cert| could be NULL when called.
+    // Called with |cert| == NULL after importing a list of certificates
+    // in ImportFromPKCS12().
     virtual void OnUserCertAdded(const X509Certificate* cert) {}
 
+    // Will be called when a user certificate is removed.
+    virtual void OnUserCertRemoved(const X509Certificate* cert) {}
+
     // Will be called when a certificate's trust is changed.
-    // Note that |cert| could be NULL when called.
+    // Called with |cert| == NULL after importing a list of certificates
+    // in ImportCACerts().
     virtual void OnCertTrustChanged(const X509Certificate* cert) {}
 
    protected:
@@ -196,6 +201,7 @@ class NET_EXPORT CertDatabase {
  private:
   // Broadcasts notifications to all registered observers.
   static void NotifyObserversOfUserCertAdded(const X509Certificate* cert);
+  static void NotifyObserversOfUserCertRemoved(const X509Certificate* cert);
   static void NotifyObserversOfCertTrustChanged(const X509Certificate* cert);
 
   DISALLOW_COPY_AND_ASSIGN(CertDatabase);
diff --git a/net/base/cert_database_nss.cc b/net/base/cert_database_nss.cc
index a8ee5dd..0364d11 100644
--- a/net/base/cert_database_nss.cc
+++ b/net/base/cert_database_nss.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -290,7 +290,6 @@ bool CertDatabase::SetCertTrust(const X509Certificate* cert,
   return success;
 }
 
-// TODO(xiyuan): Add an Observer method for this event.
 bool CertDatabase::DeleteCertAndKey(const X509Certificate* cert) {
   // For some reason, PK11_DeleteTokenCertAndKey only calls
   // SEC_DeletePermCertificate if the private key is found.  So, we check
@@ -310,6 +309,9 @@ bool CertDatabase::DeleteCertAndKey(const X509Certificate* cert) {
       return false;
     }
   }
+
+  CertDatabase::NotifyObserversOfUserCertRemoved(cert);
+
   return true;
 }
 
diff --git a/net/base/cert_database_nss_unittest.cc b/net/base/cert_database_nss_unittest.cc
index 4d94946..8119dae 100644
--- a/net/base/cert_database_nss_unittest.cc
+++ b/net/base/cert_database_nss_unittest.cc
@@ -10,6 +10,7 @@
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/lazy_instance.h"
+#include "base/message_loop.h"
 #include "base/path_service.h"
 #include "base/string16.h"
 #include "base/string_util.h"
@@ -56,6 +57,12 @@ class CertDatabaseNSSTest : public testing::Test {
     ASSERT_TRUE(slot_->os_module_handle());
 
     EXPECT_TRUE(CleanupSlotContents(slot_->os_module_handle()));
+
+    // Run the message loop to process any observer callbacks (e.g. for the
+    // ClientSocketFactory singleton) so that the scoped ref ptrs created in
+    // CertDatabase::NotifyObservers* get released.
+    MessageLoop::current()->RunAllPending();
+
     EXPECT_EQ(0U, ListCertsInSlot(slot_->os_module_handle()).size());
   }