From c5f6bd9750900f8baae0e23c7d273f5f08316ad2 Mon Sep 17 00:00:00 2001 From: jianli Date: Wed, 27 May 2015 19:23:45 -0700 Subject: [PATCH] Remove Instance ID and token data when an app with gcm permission is uninstalled BUG=477084 TEST=test updated TBR=kalman@chromium.org Review URL: https://codereview.chromium.org/1158563007 Cr-Commit-Position: refs/heads/master@{#331721} --- .../browser/extensions/extension_gcm_app_handler.cc | 19 +++++++++++++++++-- chrome/browser/extensions/extension_gcm_app_handler.h | 4 +++- .../extensions/extension_gcm_app_handler_unittest.cc | 15 ++++++++++++++- components/gcm_driver/gcm_driver.cc | 5 +++++ components/gcm_driver/gcm_driver.h | 4 ++++ components/gcm_driver/instance_id/instance_id_impl.cc | 4 +--- 6 files changed, 44 insertions(+), 7 deletions(-) diff --git a/chrome/browser/extensions/extension_gcm_app_handler.cc b/chrome/browser/extensions/extension_gcm_app_handler.cc index 6a9f27538590..c07bd543f3fd 100644 --- a/chrome/browser/extensions/extension_gcm_app_handler.cc +++ b/chrome/browser/extensions/extension_gcm_app_handler.cc @@ -136,11 +136,17 @@ void ExtensionGCMAppHandler::OnExtensionUninstalled( const Extension* extension, extensions::UninstallReason reason) { if (IsGCMPermissionEnabled(extension)) { - GetGCMDriver()->Unregister( + // Let's first remove InstanceID data. GCM unregistration will be triggered + // after the asynchronous call is returned in OnDeleteTokensCompleted. + gcm::InstanceIDHandler* instance_id_handler = + GetGCMDriver()->GetInstanceIDHandler(); + DCHECK(instance_id_handler); + instance_id_handler->DeleteAllTokensForApp( extension->id(), - base::Bind(&ExtensionGCMAppHandler::OnUnregisterCompleted, + base::Bind(&ExtensionGCMAppHandler::OnDeleteTokensCompleted, weak_factory_.GetWeakPtr(), extension->id())); + instance_id_handler->RemoveInstanceIDData(extension->id()); } } @@ -161,6 +167,15 @@ void ExtensionGCMAppHandler::OnUnregisterCompleted( RemoveAppHandler(app_id); } +void ExtensionGCMAppHandler::OnDeleteTokensCompleted( + const std::string& app_id, gcm::GCMClient::Result result) { + GetGCMDriver()->Unregister( + app_id, + base::Bind(&ExtensionGCMAppHandler::OnUnregisterCompleted, + weak_factory_.GetWeakPtr(), + app_id)); +} + void ExtensionGCMAppHandler::AddAppHandler(const std::string& app_id) { GetGCMDriver()->AddAppHandler(app_id, this); } diff --git a/chrome/browser/extensions/extension_gcm_app_handler.h b/chrome/browser/extensions/extension_gcm_app_handler.h index 3e2c00f00f67..3f361bacd246 100644 --- a/chrome/browser/extensions/extension_gcm_app_handler.h +++ b/chrome/browser/extensions/extension_gcm_app_handler.h @@ -60,12 +60,14 @@ class ExtensionGCMAppHandler : public gcm::GCMAppHandler, // Could be overridden by testing purpose. virtual void OnUnregisterCompleted(const std::string& app_id, gcm::GCMClient::Result result); + virtual void OnDeleteTokensCompleted(const std::string& app_id, + gcm::GCMClient::Result result); virtual void AddAppHandler(const std::string& app_id); virtual void RemoveAppHandler(const std::string& app_id); gcm::GCMDriver* GetGCMDriver() const; -private: + private: friend class BrowserContextKeyedAPIFactory; // ExtensionRegistryObserver implementation. diff --git a/chrome/browser/extensions/extension_gcm_app_handler_unittest.cc b/chrome/browser/extensions/extension_gcm_app_handler_unittest.cc index 5355ee0be636..4669ea37cb52 100644 --- a/chrome/browser/extensions/extension_gcm_app_handler_unittest.cc +++ b/chrome/browser/extensions/extension_gcm_app_handler_unittest.cc @@ -132,6 +132,7 @@ class FakeExtensionGCMAppHandler : public ExtensionGCMAppHandler { : ExtensionGCMAppHandler(profile), waiter_(waiter), unregistration_result_(gcm::GCMClient::UNKNOWN_ERROR), + delete_tokens_result_(gcm::GCMClient::UNKNOWN_ERROR), app_handler_count_drop_to_zero_(false) { } @@ -153,6 +154,12 @@ class FakeExtensionGCMAppHandler : public ExtensionGCMAppHandler { waiter_->SignalCompleted(); } + void OnDeleteTokensCompleted(const std::string& app_id, + gcm::GCMClient::Result result) override { + delete_tokens_result_ = result; + ExtensionGCMAppHandler::OnDeleteTokensCompleted(app_id, result); + } + void RemoveAppHandler(const std::string& app_id) override { ExtensionGCMAppHandler::RemoveAppHandler(app_id); if (!GetGCMDriver()->app_handlers().size()) @@ -162,6 +169,9 @@ class FakeExtensionGCMAppHandler : public ExtensionGCMAppHandler { gcm::GCMClient::Result unregistration_result() const { return unregistration_result_; } + gcm::GCMClient::Result delete_tokens_result() const { + return delete_tokens_result_; + } bool app_handler_count_drop_to_zero() const { return app_handler_count_drop_to_zero_; } @@ -169,6 +179,7 @@ class FakeExtensionGCMAppHandler : public ExtensionGCMAppHandler { private: Waiter* waiter_; gcm::GCMClient::Result unregistration_result_; + gcm::GCMClient::Result delete_tokens_result_; bool app_handler_count_drop_to_zero_; DISALLOW_COPY_AND_ASSIGN(FakeExtensionGCMAppHandler); @@ -418,9 +429,11 @@ TEST_F(ExtensionGCMAppHandlerTest, UnregisterOnExtensionUninstall) { waiter()->WaitUntilCompleted(); EXPECT_EQ(gcm::GCMClient::SUCCESS, registration_result()); - // Unregistration should be triggered when the extension is uninstalled. + // Both token deletion and unregistration should be triggered when the + // extension is uninstalled. UninstallExtension(extension.get()); waiter()->WaitUntilCompleted(); + EXPECT_EQ(gcm::GCMClient::SUCCESS, gcm_app_handler()->delete_tokens_result()); EXPECT_EQ(gcm::GCMClient::SUCCESS, gcm_app_handler()->unregistration_result()); } diff --git a/components/gcm_driver/gcm_driver.cc b/components/gcm_driver/gcm_driver.cc index 10adbaeb4c32..82278172aab2 100644 --- a/components/gcm_driver/gcm_driver.cc +++ b/components/gcm_driver/gcm_driver.cc @@ -22,6 +22,11 @@ InstanceIDHandler::InstanceIDHandler() { InstanceIDHandler::~InstanceIDHandler() { } +void InstanceIDHandler::DeleteAllTokensForApp( + const std::string& app_id, const DeleteTokenCallback& callback) { + DeleteToken(app_id, "*", "*", callback); +} + GCMDriver::GCMDriver() : weak_ptr_factory_(this) { } diff --git a/components/gcm_driver/gcm_driver.h b/components/gcm_driver/gcm_driver.h index 689d721cf09d..9122d84a5c89 100644 --- a/components/gcm_driver/gcm_driver.h +++ b/components/gcm_driver/gcm_driver.h @@ -35,6 +35,10 @@ class InstanceIDHandler { InstanceIDHandler(); virtual ~InstanceIDHandler(); + // Delete all tokens assoicated with |app_id|. + void DeleteAllTokensForApp(const std::string& app_id, + const DeleteTokenCallback& callback); + // Token service. virtual void GetToken(const std::string& app_id, const std::string& authorized_entity, diff --git a/components/gcm_driver/instance_id/instance_id_impl.cc b/components/gcm_driver/instance_id/instance_id_impl.cc index e9c347e9c59f..3850eb19ee63 100644 --- a/components/gcm_driver/instance_id/instance_id_impl.cc +++ b/components/gcm_driver/instance_id/instance_id_impl.cc @@ -188,10 +188,8 @@ void InstanceIDImpl::DoDeleteID(const DeleteIDCallback& callback) { return; } - GetInstanceIDHandler()->DeleteToken( + GetInstanceIDHandler()->DeleteAllTokensForApp( app_id(), - "*", - "*", base::Bind(&InstanceIDImpl::OnDeleteIDCompleted, weak_ptr_factory_.GetWeakPtr(), callback)); -- 2.11.4.GIT