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}

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',