diff options
author | asargent <asargent@chromium.org> | 2016-03-21 16:47:33 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2016-03-21 23:49:06 +0000 |
commit | 8380bd3915ac60c78dd82ee3f2f5c8940383046a (patch) | |
tree | 77fc14305f8e1a3a4495a2d2e672be7212d3f493 /extensions | |
parent | 5f35094fbf8fdb3fdd4115d4dcfb748e7953c6ba (diff) | |
download | chromium_src-8380bd3915ac60c78dd82ee3f2f5c8940383046a.zip chromium_src-8380bd3915ac60c78dd82ee3f2f5c8940383046a.tar.gz chromium_src-8380bd3915ac60c78dd82ee3f2f5c8940383046a.tar.bz2 |
Add code to implement extension uninstall pings
We'll only send these for extensions hosted in the webstore
BUG=501641
Review URL: https://codereview.chromium.org/1817653002
Cr-Commit-Position: refs/heads/master@{#382440}
Diffstat (limited to 'extensions')
-rw-r--r-- | extensions/browser/uninstall_ping_sender.cc | 33 | ||||
-rw-r--r-- | extensions/browser/uninstall_ping_sender.h | 51 | ||||
-rw-r--r-- | extensions/browser/uninstall_reason.h | 4 | ||||
-rw-r--r-- | extensions/browser/updater/update_service.cc | 6 | ||||
-rw-r--r-- | extensions/browser/updater/update_service.h | 8 | ||||
-rw-r--r-- | extensions/browser/updater/update_service_unittest.cc | 98 | ||||
-rw-r--r-- | extensions/extensions.gypi | 2 |
7 files changed, 201 insertions, 1 deletions
diff --git a/extensions/browser/uninstall_ping_sender.cc b/extensions/browser/uninstall_ping_sender.cc new file mode 100644 index 0000000..003cf13 --- /dev/null +++ b/extensions/browser/uninstall_ping_sender.cc @@ -0,0 +1,33 @@ +// Copyright 2016 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 "extensions/browser/uninstall_ping_sender.h" + +#include "base/version.h" +#include "extensions/browser/extension_registry.h" +#include "extensions/browser/updater/update_service.h" + +namespace extensions { + +UninstallPingSender::UninstallPingSender(ExtensionRegistry* registry, + const Filter& filter) + : filter_(filter), observer_(this) { + observer_.Add(registry); +} + +UninstallPingSender::~UninstallPingSender() {} + +void UninstallPingSender::OnExtensionUninstalled( + content::BrowserContext* browser_context, + const Extension* extension, + UninstallReason reason) { + if (filter_.Run(extension, reason) == SEND_PING) { + UpdateService* updater = UpdateService::Get(browser_context); + base::Version version = + extension->version() ? *extension->version() : base::Version("0"); + updater->SendUninstallPing(extension->id(), version, reason); + } +} + +} // namespace extensions diff --git a/extensions/browser/uninstall_ping_sender.h b/extensions/browser/uninstall_ping_sender.h new file mode 100644 index 0000000..a349d5c --- /dev/null +++ b/extensions/browser/uninstall_ping_sender.h @@ -0,0 +1,51 @@ +// Copyright 2016 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. + +#ifndef EXTENSIONS_BROWSER_UNINSTALL_PING_SENDER_H_ +#define EXTENSIONS_BROWSER_UNINSTALL_PING_SENDER_H_ + +#include "base/callback.h" +#include "base/macros.h" +#include "base/scoped_observer.h" +#include "extensions/browser/extension_registry_observer.h" + +namespace content { +class BrowserContext; +} + +namespace extensions { + +// A class that watches the ExtensionRegistry for uninstall events, and +// uses the UpdateService to send uninstall pings. +class UninstallPingSender : public ExtensionRegistryObserver { + public: + enum FilterResult { SEND_PING, DO_NOT_SEND_PING }; + + // A callback function that will be called each time an extension is + // uninstalled, with the result used to determine if a ping should be + // sent or not. + using Filter = base::Callback<FilterResult(const Extension* extension, + UninstallReason reason)>; + + UninstallPingSender(ExtensionRegistry* registry, const Filter& filter); + ~UninstallPingSender() override; + + protected: + // ExtensionRegistryObserver: + void OnExtensionUninstalled(content::BrowserContext* browser_context, + const Extension* extension, + UninstallReason reason) override; + + // Callback for determining whether to send uninstall pings. + Filter filter_; + + ScopedObserver<ExtensionRegistry, ExtensionRegistryObserver> observer_; + + private: + DISALLOW_COPY_AND_ASSIGN(UninstallPingSender); +}; + +} // namespace extensions + +#endif // EXTENSIONS_BROWSER_UNINSTALL_PING_SENDER_H_ diff --git a/extensions/browser/uninstall_reason.h b/extensions/browser/uninstall_reason.h index 0da23cf..8484aa9 100644 --- a/extensions/browser/uninstall_reason.h +++ b/extensions/browser/uninstall_reason.h @@ -7,6 +7,8 @@ namespace extensions { +// Do not remove/reorder these, as they are used in uninstall ping data and we +// depend on their values being stable. enum UninstallReason { UNINSTALL_REASON_FOR_TESTING, // Used for testing code only UNINSTALL_REASON_USER_INITIATED, // User performed some UI gesture @@ -24,6 +26,8 @@ enum UninstallReason { UNINSTALL_REASON_INTERNAL_MANAGEMENT, // Internal extensions (see usages) UNINSTALL_REASON_REINSTALL, UNINSTALL_REASON_COMPONENT_REMOVED, + + UNINSTALL_REASON_MAX, // Should always be the last value }; // The source of an uninstall. Do *NOT* adjust the order of these, as they are diff --git a/extensions/browser/updater/update_service.cc b/extensions/browser/updater/update_service.cc index ad40abd..d356f59 100644 --- a/extensions/browser/updater/update_service.cc +++ b/extensions/browser/updater/update_service.cc @@ -42,6 +42,12 @@ void UpdateService::Shutdown() { context_ = nullptr; } +void UpdateService::SendUninstallPing(const std::string& id, + const Version& version, + int reason) { + update_client_->SendUninstallPing(id, version, reason); +} + void UpdateService::StartUpdateCheck(std::vector<std::string> extension_ids) { if (!update_client_) return; diff --git a/extensions/browser/updater/update_service.h b/extensions/browser/updater/update_service.h index 0ee321c..992f55e 100644 --- a/extensions/browser/updater/update_service.h +++ b/extensions/browser/updater/update_service.h @@ -13,6 +13,10 @@ #include "base/memory/scoped_ptr.h" #include "components/keyed_service/core/keyed_service.h" +namespace base { +class Version; +} + namespace content { class BrowserContext; } @@ -36,6 +40,10 @@ class UpdateService : public KeyedService { void Shutdown() override; + void SendUninstallPing(const std::string& id, + const base::Version& version, + int reason); + // Starts an update check for each of |extension_ids|. If there are any // updates available, they will be downloaded, checked for integrity, // unpacked, and then passed off to the ExtensionSystem::InstallUpdate method diff --git a/extensions/browser/updater/update_service_unittest.cc b/extensions/browser/updater/update_service_unittest.cc index 67c3087..eb8213a 100644 --- a/extensions/browser/updater/update_service_unittest.cc +++ b/extensions/browser/updater/update_service_unittest.cc @@ -18,8 +18,10 @@ #include "extensions/browser/extensions_test.h" #include "extensions/browser/mock_extension_system.h" #include "extensions/browser/test_extensions_browser_client.h" +#include "extensions/browser/uninstall_ping_sender.h" #include "extensions/browser/updater/update_service.h" #include "extensions/common/extension_builder.h" +#include "extensions/common/test_util.h" #include "extensions/common/value_builder.h" #include "testing/gtest/include/gtest/gtest.h" @@ -33,6 +35,16 @@ class FakeUpdateClient : public update_client::UpdateClient { // the Update function. std::vector<update_client::CrxComponent>* data() { return &data_; } + // Used for tests that uninstall pings get requested properly. + struct UninstallPing { + std::string id; + Version version; + int reason; + UninstallPing(const std::string& id, const Version& version, int reason) + : id(id), version(version), reason(reason) {} + }; + std::vector<UninstallPing>& uninstall_pings() { return uninstall_pings_; } + // update_client::UpdateClient void AddObserver(Observer* observer) override {} void RemoveObserver(Observer* observer) override {} @@ -51,13 +63,19 @@ class FakeUpdateClient : public update_client::UpdateClient { void Stop() override {} void SendUninstallPing(const std::string& id, const Version& version, - int reason) override {} + int reason) override { + uninstall_pings_.emplace_back(id, version, reason); + } protected: friend class base::RefCounted<FakeUpdateClient>; ~FakeUpdateClient() override {} std::vector<update_client::CrxComponent> data_; + std::vector<UninstallPing> uninstall_pings_; + + private: + DISALLOW_COPY_AND_ASSIGN(FakeUpdateClient); }; FakeUpdateClient::FakeUpdateClient() {} @@ -74,6 +92,17 @@ namespace extensions { namespace { +// A global variable for controlling whether uninstalls should cause uninstall +// pings to be sent. +UninstallPingSender::FilterResult g_should_ping = + UninstallPingSender::DO_NOT_SEND_PING; + +// Helper method to serve as an uninstall ping filter. +UninstallPingSender::FilterResult ShouldPing(const Extension* extension, + UninstallReason reason) { + return g_should_ping; +} + // A fake ExtensionSystem that lets us intercept calls to install new // versions of an extension. class FakeExtensionSystem : public MockExtensionSystem { @@ -250,6 +279,73 @@ TEST_F(UpdateServiceTest, BasicUpdateOperations) { EXPECT_NE(requests->at(0).temp_dir.value(), new_version_dir.path().value()); } +TEST_F(UpdateServiceTest, UninstallPings) { + UninstallPingSender sender(ExtensionRegistry::Get(browser_context()), + base::Bind(&ShouldPing)); + + // Build 3 extensions. + scoped_refptr<Extension> extension1 = + test_util::BuildExtension(ExtensionBuilder()) + .SetID(crx_file::id_util::GenerateId("1")) + .MergeManifest(DictionaryBuilder().Set("version", "1.2").Build()) + .Build(); + scoped_refptr<Extension> extension2 = + test_util::BuildExtension(ExtensionBuilder()) + .SetID(crx_file::id_util::GenerateId("2")) + .MergeManifest(DictionaryBuilder().Set("version", "2.3").Build()) + .Build(); + scoped_refptr<Extension> extension3 = + test_util::BuildExtension(ExtensionBuilder()) + .SetID(crx_file::id_util::GenerateId("3")) + .MergeManifest(DictionaryBuilder().Set("version", "3.4").Build()) + .Build(); + EXPECT_TRUE(extension1->id() != extension2->id() && + extension1->id() != extension3->id() && + extension2->id() != extension3->id()); + + ExtensionRegistry* registry = ExtensionRegistry::Get(browser_context()); + + // Run tests for each uninstall reason. + for (int reason_val = static_cast<int>(UNINSTALL_REASON_FOR_TESTING); + reason_val < static_cast<int>(UNINSTALL_REASON_MAX); ++reason_val) { + UninstallReason reason = static_cast<UninstallReason>(reason_val); + + // Start with 2 enabled and 1 disabled extensions. + EXPECT_TRUE(registry->AddEnabled(extension1)) << reason; + EXPECT_TRUE(registry->AddEnabled(extension2)) << reason; + EXPECT_TRUE(registry->AddDisabled(extension3)) << reason; + + // Uninstall the first extension, instructing our filter not to send pings, + // and verify none were sent. + g_should_ping = UninstallPingSender::DO_NOT_SEND_PING; + EXPECT_TRUE(registry->RemoveEnabled(extension1->id())) << reason; + registry->TriggerOnUninstalled(extension1.get(), reason); + EXPECT_TRUE(update_client()->uninstall_pings().empty()) << reason; + + // Uninstall the second and third extensions, instructing the filter to + // send pings, and make sure we got the expected data. + g_should_ping = UninstallPingSender::SEND_PING; + EXPECT_TRUE(registry->RemoveEnabled(extension2->id())) << reason; + registry->TriggerOnUninstalled(extension2.get(), reason); + EXPECT_TRUE(registry->RemoveDisabled(extension3->id())) << reason; + registry->TriggerOnUninstalled(extension3.get(), reason); + + std::vector<FakeUpdateClient::UninstallPing>& pings = + update_client()->uninstall_pings(); + ASSERT_EQ(2u, pings.size()) << reason; + + EXPECT_EQ(extension2->id(), pings[0].id) << reason; + EXPECT_EQ(*extension2->version(), pings[0].version) << reason; + EXPECT_EQ(reason, pings[0].reason) << reason; + + EXPECT_EQ(extension3->id(), pings[1].id) << reason; + EXPECT_EQ(*extension3->version(), pings[1].version) << reason; + EXPECT_EQ(reason, pings[1].reason) << reason; + + pings.clear(); + } +} + } // namespace } // namespace extensions diff --git a/extensions/extensions.gypi b/extensions/extensions.gypi index 99897d6..e1c30bf 100644 --- a/extensions/extensions.gypi +++ b/extensions/extensions.gypi @@ -757,6 +757,8 @@ 'browser/state_store.h', 'browser/suggest_permission_util.cc', 'browser/suggest_permission_util.h', + 'browser/uninstall_ping_sender.cc', + 'browser/uninstall_ping_sender.h', 'browser/uninstall_reason.h', 'browser/update_observer.h', 'browser/updater/extension_cache.h', |