diff options
-rw-r--r-- | chrome/browser/extensions/extension_system_impl.cc | 22 | ||||
-rw-r--r-- | chrome/browser/extensions/extension_system_impl.h | 5 | ||||
-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 |
9 files changed, 228 insertions, 1 deletions
diff --git a/chrome/browser/extensions/extension_system_impl.cc b/chrome/browser/extensions/extension_system_impl.cc index 47c8295..af180ba 100644 --- a/chrome/browser/extensions/extension_system_impl.cc +++ b/chrome/browser/extensions/extension_system_impl.cc @@ -4,6 +4,8 @@ #include "chrome/browser/extensions/extension_system_impl.h" +#include <algorithm> + #include "base/base_switches.h" #include "base/bind.h" #include "base/command_line.h" @@ -47,7 +49,9 @@ #include "extensions/browser/runtime_data.h" #include "extensions/browser/service_worker_manager.h" #include "extensions/browser/state_store.h" +#include "extensions/browser/uninstall_ping_sender.h" #include "extensions/common/constants.h" +#include "extensions/common/manifest_url_handlers.h" #if defined(ENABLE_NOTIFICATIONS) #include "chrome/browser/notifications/notifier_state_tracker.h" @@ -76,6 +80,21 @@ const char kRulesDatabaseUMAClientName[] = "Rules"; namespace extensions { +namespace { + +// Helper to serve as an UninstallPingSender::Filter callback. +UninstallPingSender::FilterResult ShouldSendUninstallPing( + const Extension* extension, + UninstallReason reason) { + if (extension && (extension->from_webstore() || + ManifestURL::UpdatesFromGallery(extension))) { + return UninstallPingSender::SEND_PING; + } + return UninstallPingSender::DO_NOT_SEND_PING; +} + +} // namespace + // // ExtensionSystemImpl::Shared // @@ -161,6 +180,9 @@ void ExtensionSystemImpl::Shared::Init(bool extensions_enabled) { ExtensionPrefs::Get(profile_), Blacklist::Get(profile_), autoupdate_enabled, extensions_enabled, &ready_)); + uninstall_ping_sender_.reset(new UninstallPingSender( + ExtensionRegistry::Get(profile_), base::Bind(&ShouldSendUninstallPing))); + // These services must be registered before the ExtensionService tries to // load any extensions. { diff --git a/chrome/browser/extensions/extension_system_impl.h b/chrome/browser/extensions/extension_system_impl.h index 30522b1..bff34f3 100644 --- a/chrome/browser/extensions/extension_system_impl.h +++ b/chrome/browser/extensions/extension_system_impl.h @@ -5,6 +5,8 @@ #ifndef CHROME_BROWSER_EXTENSIONS_EXTENSION_SYSTEM_IMPL_H_ #define CHROME_BROWSER_EXTENSIONS_EXTENSION_SYSTEM_IMPL_H_ +#include <string> + #include "base/macros.h" #include "build/build_config.h" #include "extensions/browser/extension_system.h" @@ -17,6 +19,7 @@ namespace extensions { class ExtensionSystemSharedFactory; class NavigationObserver; class StateStoreNotificationObserver; +class UninstallPingSender; // The ExtensionSystem for ProfileImpl and OffTheRecordProfileImpl. // Implementation details: non-shared services are owned by @@ -117,6 +120,8 @@ class ExtensionSystemImpl : public ExtensionSystem { // For verifying the contents of extensions read from disk. scoped_refptr<ContentVerifier> content_verifier_; + scoped_ptr<UninstallPingSender> uninstall_ping_sender_; + #if defined(OS_CHROMEOS) scoped_ptr<chromeos::DeviceLocalAccountManagementPolicyProvider> device_local_account_management_policy_provider_; 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', |