diff options
5 files changed, 38 insertions, 30 deletions
diff --git a/components/gcm_driver/crypto/gcm_encryption_provider.cc b/components/gcm_driver/crypto/gcm_encryption_provider.cc index 675507a..2b775e4 100644 --- a/components/gcm_driver/crypto/gcm_encryption_provider.cc +++ b/components/gcm_driver/crypto/gcm_encryption_provider.cc @@ -34,7 +34,8 @@ void GCMEncryptionProvider::Init( if (!store_path.empty()) encryption_store_path = store_path.Append(kEncryptionDirectoryName); - key_store_ = new GCMKeyStore(encryption_store_path, blocking_task_runner); + key_store_.reset( + new GCMKeyStore(encryption_store_path, blocking_task_runner)); } void GCMEncryptionProvider::GetPublicKey(const std::string& app_id, diff --git a/components/gcm_driver/crypto/gcm_encryption_provider.h b/components/gcm_driver/crypto/gcm_encryption_provider.h index fce5732..a395118 100644 --- a/components/gcm_driver/crypto/gcm_encryption_provider.h +++ b/components/gcm_driver/crypto/gcm_encryption_provider.h @@ -10,7 +10,6 @@ #include "base/callback_forward.h" #include "base/macros.h" -#include "base/memory/ref_counted.h" #include "base/memory/weak_ptr.h" namespace base { @@ -53,7 +52,7 @@ class GCMEncryptionProvider { void DidCreatePublicKey(const PublicKeyCallback& callback, const KeyPair& pair); - scoped_refptr<GCMKeyStore> key_store_; + scoped_ptr<GCMKeyStore> key_store_; base::WeakPtrFactory<GCMEncryptionProvider> weak_ptr_factory_; diff --git a/components/gcm_driver/crypto/gcm_key_store.cc b/components/gcm_driver/crypto/gcm_key_store.cc index 0086291f..a9776fb 100644 --- a/components/gcm_driver/crypto/gcm_key_store.cc +++ b/components/gcm_driver/crypto/gcm_key_store.cc @@ -25,7 +25,8 @@ GCMKeyStore::GCMKeyStore( const scoped_refptr<base::SequencedTaskRunner>& blocking_task_runner) : key_store_path_(key_store_path), blocking_task_runner_(blocking_task_runner), - state_(State::UNINITIALIZED) { + state_(State::UNINITIALIZED), + weak_factory_(this) { DCHECK(blocking_task_runner); } @@ -33,8 +34,8 @@ GCMKeyStore::~GCMKeyStore() {} void GCMKeyStore::GetKeys(const std::string& app_id, const KeysCallback& callback) { - LazyInitialize(base::Bind(&GCMKeyStore::GetKeysAfterInitialize, this, app_id, - callback)); + LazyInitialize(base::Bind(&GCMKeyStore::GetKeysAfterInitialize, + weak_factory_.GetWeakPtr(), app_id, callback)); } void GCMKeyStore::GetKeysAfterInitialize(const std::string& app_id, @@ -51,8 +52,8 @@ void GCMKeyStore::GetKeysAfterInitialize(const std::string& app_id, void GCMKeyStore::CreateKeys(const std::string& app_id, const KeysCallback& callback) { - LazyInitialize(base::Bind(&GCMKeyStore::CreateKeysAfterInitialize, this, - app_id, callback)); + LazyInitialize(base::Bind(&GCMKeyStore::CreateKeysAfterInitialize, + weak_factory_.GetWeakPtr(), app_id, callback)); } void GCMKeyStore::CreateKeysAfterInitialize(const std::string& app_id, @@ -93,9 +94,10 @@ void GCMKeyStore::CreateKeysAfterInitialize(const std::string& app_id, entries_to_save->push_back(std::make_pair(app_id, encryption_data)); - database_->UpdateEntries(entries_to_save.Pass(), keys_to_remove.Pass(), - base::Bind(&GCMKeyStore::DidStoreKeys, this, app_id, - *pair, callback)); + database_->UpdateEntries( + entries_to_save.Pass(), keys_to_remove.Pass(), + base::Bind(&GCMKeyStore::DidStoreKeys, weak_factory_.GetWeakPtr(), app_id, + *pair, callback)); } void GCMKeyStore::DidStoreKeys(const std::string& app_id, @@ -117,8 +119,8 @@ void GCMKeyStore::DidStoreKeys(const std::string& app_id, void GCMKeyStore::DeleteKeys(const std::string& app_id, const DeleteCallback& callback) { - LazyInitialize(base::Bind(&GCMKeyStore::DeleteKeysAfterInitialize, this, - app_id, callback)); + LazyInitialize(base::Bind(&GCMKeyStore::DeleteKeysAfterInitialize, + weak_factory_.GetWeakPtr(), app_id, callback)); } void GCMKeyStore::DeleteKeysAfterInitialize(const std::string& app_id, @@ -139,7 +141,8 @@ void GCMKeyStore::DeleteKeysAfterInitialize(const std::string& app_id, database_->UpdateEntries( entries_to_save.Pass(), keys_to_remove.Pass(), - base::Bind(&GCMKeyStore::DidDeleteKeys, this, app_id, callback)); + base::Bind(&GCMKeyStore::DidDeleteKeys, weak_factory_.GetWeakPtr(), + app_id, callback)); } void GCMKeyStore::DidDeleteKeys(const std::string& app_id, @@ -171,8 +174,8 @@ void GCMKeyStore::LazyInitialize(const base::Closure& done_closure) { database_.reset(new leveldb_proto::ProtoDatabaseImpl<EncryptionData>( blocking_task_runner_)); - database_->Init(key_store_path_, - base::Bind(&GCMKeyStore::DidInitialize, this)); + database_->Init(key_store_path_, base::Bind(&GCMKeyStore::DidInitialize, + weak_factory_.GetWeakPtr())); } void GCMKeyStore::DidInitialize(bool success) { @@ -184,7 +187,8 @@ void GCMKeyStore::DidInitialize(bool success) { return; } - database_->LoadEntries(base::Bind(&GCMKeyStore::DidLoadKeys, this)); + database_->LoadEntries( + base::Bind(&GCMKeyStore::DidLoadKeys, weak_factory_.GetWeakPtr())); } void GCMKeyStore::DidLoadKeys(bool success, diff --git a/components/gcm_driver/crypto/gcm_key_store.h b/components/gcm_driver/crypto/gcm_key_store.h index d224d5c..41de906 100644 --- a/components/gcm_driver/crypto/gcm_key_store.h +++ b/components/gcm_driver/crypto/gcm_key_store.h @@ -6,19 +6,27 @@ #define COMPONENTS_GCM_DRIVER_CRYPTO_GCM_KEY_STORE_H_ #include <map> +#include <string> +#include <vector> #include "base/callback_forward.h" +#include "base/files/file_path.h" #include "base/macros.h" #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" +#include "base/memory/weak_ptr.h" #include "components/gcm_driver/crypto/proto/gcm_encryption_data.pb.h" #include "components/gcm_driver/gcm_delayed_task_controller.h" -#include "components/leveldb_proto/proto_database.h" namespace base { class SequencedTaskRunner; } +namespace leveldb_proto { +template <typename T> +class ProtoDatabase; +} + namespace gcm { // Key storage for use with encrypted messages received from Google Cloud @@ -28,10 +36,7 @@ namespace gcm { // This class is backed by a proto database and might end up doing file I/O on // a background task runner. For this reason, all public APIs take a callback // rather than returning the result. Do not rely on the timing of the callbacks. -// -// Started operations will be completed even after the reference to the store -// has been freed (asynchronous operations increase reference count to |this|). -class GCMKeyStore : public base::RefCounted<GCMKeyStore> { +class GCMKeyStore { public: using KeysCallback = base::Callback<void(const KeyPair& pair)>; using DeleteCallback = base::Callback<void(bool success)>; @@ -39,6 +44,7 @@ class GCMKeyStore : public base::RefCounted<GCMKeyStore> { GCMKeyStore( const base::FilePath& key_store_path, const scoped_refptr<base::SequencedTaskRunner>& blocking_task_runner); + ~GCMKeyStore(); // Retrieves the public/private key-pair associated with |app_id|, and // invokes |callback| when they are available, or when an error occurred. @@ -53,10 +59,6 @@ class GCMKeyStore : public base::RefCounted<GCMKeyStore> { void DeleteKeys(const std::string& app_id, const DeleteCallback& callback); private: - friend class base::RefCounted<GCMKeyStore>; - - virtual ~GCMKeyStore(); - // Initializes the database if necessary, and runs |done_closure| when done. void LazyInitialize(const base::Closure& done_closure); @@ -104,6 +106,8 @@ class GCMKeyStore : public base::RefCounted<GCMKeyStore> { // Mapping of an app id to the loaded EncryptedData structure. std::map<std::string, KeyPair> key_pairs_; + base::WeakPtrFactory<GCMKeyStore> weak_factory_; + DISALLOW_COPY_AND_ASSIGN(GCMKeyStore); }; diff --git a/components/gcm_driver/crypto/gcm_key_store_unittest.cc b/components/gcm_driver/crypto/gcm_key_store_unittest.cc index 4eff619..56aae87 100644 --- a/components/gcm_driver/crypto/gcm_key_store_unittest.cc +++ b/components/gcm_driver/crypto/gcm_key_store_unittest.cc @@ -30,7 +30,7 @@ class GCMKeyStoreTest : public ::testing::Test { } void TearDown() override { - gcm_key_store_ = nullptr; + gcm_key_store_.reset(); // |gcm_key_store_| owns a ProtoDatabaseImpl whose destructor deletes the // underlying LevelDB database on the task runner. @@ -40,8 +40,8 @@ class GCMKeyStoreTest : public ::testing::Test { // Creates the GCM Key Store instance. May be called from within a test's body // to re-create the key store, causing the database to re-open. void CreateKeyStore() { - gcm_key_store_ = new GCMKeyStore(scoped_temp_dir_.path(), - message_loop_.task_runner()); + gcm_key_store_.reset( + new GCMKeyStore(scoped_temp_dir_.path(), message_loop_.task_runner())); } // Callback to use with GCMKeyStore::{GetKeys, CreateKeys} calls. @@ -65,7 +65,7 @@ class GCMKeyStoreTest : public ::testing::Test { base::MessageLoop message_loop_; base::ScopedTempDir scoped_temp_dir_; - scoped_refptr<GCMKeyStore> gcm_key_store_; + scoped_ptr<GCMKeyStore> gcm_key_store_; }; TEST_F(GCMKeyStoreTest, CreatedByDefault) { |