CPM Extension Event Watching Browsertests

chrome/browser/performance_monitor/performance_monitor_browsertest.cc

Index: chrome/browser/performance_monitor/performance_monitor_browsertest.cc
diff --git a/chrome/browser/performance_monitor/performance_monitor_browsertest.cc b/chrome/browser/performance_monitor/performance_monitor_browsertest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..9c2b43d4915a4519410a80d4da6d86054265e2e4
--- /dev/null
+++ b/chrome/browser/performance_monitor/performance_monitor_browsertest.cc
@@ -0,0 +1,280 @@
+// Copyright (c) 2010 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 "chrome/test/base/in_process_browser_test.h"
+
+#include "base/file_path.h"
+#include "base/logging.h"
+#include "base/path_service.h"
+#include "base/threading/sequenced_worker_pool.h"
+#include "chrome/browser/performance_monitor/database.h"
+#include "chrome/browser/performance_monitor/performance_monitor.h"
+#include "chrome/browser/extensions/extension_browsertest.h"
+#include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/unpacked_installer.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/common/chrome_notification_types.h"
+#include "chrome/common/chrome_paths.h"
+#include "chrome/common/extensions/extension.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "content/public/browser/notification_registrar.h"
+#include "content/public/browser/notification_service.h"
+
+using extensions::Extension;
+using performance_monitor::Event;
+
+namespace {
+
+// Helper struct to store the information of an extension; this is needed if the
+// pointer to the extension ever becomes invalid (e.g., if we uninstall the
+// extension).
+struct ExtensionBasicInfo {
+  // Empty constructor for stl-container-happiness.
+  ExtensionBasicInfo() {
+  }
+  explicit ExtensionBasicInfo(const Extension* extension)
+      : description(extension->description()),
+        id(extension->id()),
+        name(extension->name()),
+        url(extension->url().spec()),
+        version(extension->VersionString()),
+        location(extension->location()) {
+  }
+
+  std::string description;
+  std::string id;
+  std::string name;
+  std::string url;
+  std::string version;
+  Extension::Location location;
+};
+
+// Compare the fields of |extension| to those in |value|; this is a check to
+// make sure the extension data was recorded properly in the event.
+void ValidateExtensionInfo(const ExtensionBasicInfo extension,
+                           const DictionaryValue* value) {
+  std::string extension_description;
+  std::string extension_id;
+  std::string extension_name;
+  std::string extension_url;
+  std::string extension_version;
+  int extension_location;
+
+  ASSERT_TRUE(value->GetString("extension_description",
+                               &extension_description));
+  ASSERT_EQ(extension.description, extension_description);
+  ASSERT_TRUE(value->GetString("extension_id", &extension_id));
+  ASSERT_EQ(extension.id, extension_id);
+  ASSERT_TRUE(value->GetString("extension_name", &extension_name));
+  ASSERT_EQ(extension.name, extension_name);
+  ASSERT_TRUE(value->GetString("extension_url", &extension_url));
+  ASSERT_EQ(extension.url, extension_url);
+  ASSERT_TRUE(value->GetString("extension_version", &extension_version));
+  ASSERT_EQ(extension.version, extension_version);
+  ASSERT_TRUE(value->GetInteger("extension_location", &extension_location));
+  ASSERT_EQ(extension.location, extension_location);
+}
+
+// Check that we received the proper number of events, that each event is of the
+// proper type, and that each event recorded the proper information about the
+// extension.
+void CheckExtensionEvents(std::vector<int> expected_event_types,
+                          std::vector<ExtensionBasicInfo> extension_infos,
+                          const std::vector<linked_ptr<Event> >& events) {
+  ASSERT_EQ(expected_event_types.size(), events.size());
+
+  for (size_t i = 0; i < expected_event_types.size(); ++i) {
+    const base::DictionaryValue* value;
+    ASSERT_TRUE(events[i]->data()->GetAsDictionary(&value));

[Yoyo Zhou, 2012/06/27 19:59:25]

data() is already a DictionaryValue, so you don't need to GetAsDictionary:

ValidateExtensionInfo(extension_infos[i], events[i]->data());

[Devlin, 2012/06/27 21:04:06]

Whoops, forgot we changed this...
Done.

+    ValidateExtensionInfo(extension_infos[i], value);
+    int event_type;
+    ASSERT_TRUE(value->GetInteger("type", &event_type));
+    ASSERT_EQ(expected_event_types[i], event_type);
+  }
+}
+
+}  // namespace
+
+namespace performance_monitor {
+
+// The main class to test PerformanceMonitor; since much of the information in
+// PerformanceMonitor comes from extensions, we extend ExtensionBrowserTest.

[Yoyo Zhou, 2012/06/27 19:59:25]

nit: This is exactly what ExtensionBrowserTest is for; you don't need the
comment.

[Devlin, 2012/06/27 21:04:06]

Done.

+class PerformanceMonitorBrowserTest : public ExtensionBrowserTest {
+ public:
+  virtual void SetUpOnMainThread() {
+    CHECK(db_dir_.CreateUniqueTempDir());
+    performance_monitor_ = PerformanceMonitor::GetInstance();
+    performance_monitor_->SetDatabasePath(db_dir_.path());
+    performance_monitor_->Start();
+
+    // Wait for DB to finish setting up.
+    content::BrowserThread::GetBlockingPool()->FlushForTesting();
+  }
+
+  void GetEventsOnBackgroundThread(std::vector<linked_ptr<Event> >* events) {
+    *events = performance_monitor_->database()->GetEvents();
+  }
+
+  // A handle for getting the events from the database, which must be done on
+  // the background thread. Since we are testing, and can mock synchronicity

[Yoyo Zhou, 2012/06/27 19:59:25]

nit: s/and/we/

[Devlin, 2012/06/27 21:04:06]

Done.

+  // with FlushForTesting().
+  std::vector<linked_ptr<Event> > GetEvents() {
+    std::vector<linked_ptr<Event> > events;
+    content::BrowserThread::PostBlockingPoolSequencedTask(
+        Database::kDatabaseSequenceToken,
+        FROM_HERE,
+        base::Bind(&PerformanceMonitorBrowserTest::GetEventsOnBackgroundThread,
+                   base::Unretained(this),
+                   &events));
+
+    content::BrowserThread::GetBlockingPool()->FlushForTesting();
+    return events;
+  }
+
+  PerformanceMonitor* performance_monitor() const {
+    return performance_monitor_;
+  }
+
+ protected:
+  ScopedTempDir db_dir_;
+  PerformanceMonitor* performance_monitor_;

[Yoyo Zhou, 2012/06/27 19:59:25]

This is leaked. Use a scoped_ptr.

[Devlin, 2012/06/27 21:04:06]

I could be wrong, but my understanding is as follows:
 - PerformanceMonitor inherits from base::Singleton
 - Upon GetInstance(), the Singleton will register with the AtExitManager, and
will be destroyed on shutdown (AtExitManager::ProcessCallbacksNow()).
Is this wrong?

[Yoyo Zhou, 2012/06/27 21:55:51]

Whoops, I wasn't reading carefully enough. It should be fine.

+};
+
+// Test that PerformanceMonitor will correctly record an extension installation
+// event.
+IN_PROC_BROWSER_TEST_F(PerformanceMonitorBrowserTest, InstallExtensionEvent) {
+  FilePath extension_path;
+  PathService::Get(chrome::DIR_TEST_DATA, &extension_path);
+  extension_path = extension_path.AppendASCII("performance_monitor")
+                                 .AppendASCII("extensions")
+                                 .AppendASCII("simple_extension_v1");
+  const Extension* extension = LoadExtension(extension_path);
+
+  std::vector<ExtensionBasicInfo> extension_infos;
+  extension_infos.push_back(ExtensionBasicInfo(extension));
+
+  std::vector<int> expected_event_types;
+  expected_event_types.push_back(EVENT_EXTENSION_INSTALL);
+
+  std::vector<linked_ptr<Event> > events = GetEvents();
+  CheckExtensionEvents(expected_event_types, extension_infos, events);
+}
+
+// Test that PerformanceMonitor will correctly record events as an extension is
+// disabled and enabled.
+IN_PROC_BROWSER_TEST_F(PerformanceMonitorBrowserTest,
+                       DisableAndEnableExtensionEvent) {
+  const int kNumEvents = 3;
+
+  FilePath extension_path;
+  PathService::Get(chrome::DIR_TEST_DATA, &extension_path);
+  extension_path = extension_path.AppendASCII("performance_monitor")
+                                 .AppendASCII("extensions")
+                                 .AppendASCII("simple_extension_v1");
+  const Extension* extension = LoadExtension(extension_path);
+
+  DisableExtension(extension->id());
+  EnableExtension(extension->id());
+
+  std::vector<ExtensionBasicInfo> extension_infos;
+  // There will be three events in all, each pertaining to the same extension:
+  //   Extension Install
+  //   Extension Disable (Unload)
+  //   Extension Enable
+  for (int i = 0; i < kNumEvents; ++i)
+    extension_infos.push_back(ExtensionBasicInfo(extension));
+
+  std::vector<int> expected_event_types;
+  expected_event_types.push_back(EVENT_EXTENSION_INSTALL);
+  expected_event_types.push_back(EVENT_EXTENSION_UNLOAD);
+  expected_event_types.push_back(EVENT_EXTENSION_ENABLE);
+
+  std::vector<linked_ptr<Event> > events = GetEvents();
+  CheckExtensionEvents(expected_event_types, extension_infos, events);
+
+  // There will be an additional field on the Disable event: Unload Reason.

[Yoyo Zhou, 2012/06/27 19:59:25]

the Unload event

[Devlin, 2012/06/27 21:04:06]

Done.

+  const base::DictionaryValue* value;
+  int unload_reason = 0;
+  ASSERT_TRUE(events[1]->data()->GetAsDictionary(&value));

[Yoyo Zhou, 2012/06/27 19:59:25]

ditto

[Devlin, 2012/06/27 21:04:06]

Done.

+  ASSERT_TRUE(value->GetInteger("unload_reason", &unload_reason));
+  ASSERT_EQ(extension_misc::UNLOAD_REASON_DISABLE, unload_reason);
+}
+
+// Test that PerformanceMonitor correctly records an extension update event.
+IN_PROC_BROWSER_TEST_F(PerformanceMonitorBrowserTest, UpdateExtensionEvent) {
+  ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+
+  FilePath test_data_dir;
+  PathService::Get(chrome::DIR_TEST_DATA, &test_data_dir);
+  test_data_dir = test_data_dir.AppendASCII("performance_monitor")
+                               .AppendASCII("extensions");
+
+  // We need two versions of the same extension.
+  FilePath pem_path = test_data_dir.AppendASCII("simple_extension.pem");
+  FilePath path_v1_ = PackExtensionWithOptions(
+      test_data_dir.AppendASCII("simple_extension_v1"),
+      temp_dir.path().AppendASCII("simple_extension1.crx"),
+      pem_path,
+      FilePath());
+  FilePath path_v2_ = PackExtensionWithOptions(
+      test_data_dir.AppendASCII("simple_extension_v2"),
+      temp_dir.path().AppendASCII("simple_extension2.crx"),
+      pem_path,
+      FilePath());
+
+  const extensions::Extension* extension = InstallExtension(path_v1_, 1);
+
+  std::vector<ExtensionBasicInfo> extension_infos;

[Yoyo Zhou, 2012/06/27 19:59:25]

Move this down to where the rest of extension_infos is built up.

[Devlin, 2012/06/27 21:04:06]

Can't :(  InstallExtension returns an Extension*; once we update the extension,
the ptr becomes invalid, so we need to insert the extension prior to updating
the extension (this is one of the reasons I had to make the ExtensionInfo
struct).

+  extension_infos.push_back(ExtensionBasicInfo(extension));
+
+  ExtensionService* extension_service =
+      browser()->profile()->GetExtensionService();
+
+  CrxInstaller* crx_installer = NULL;
+
+  ASSERT_TRUE(extension_service->
+      UpdateExtension(extension->id(), path_v2_, GURL(), &crx_installer));
+
+  // Observe implementation in ExtensionBrowserTest; this will exit the loop
+  // upon receipt of the notification.
+  content::NotificationRegistrar registrar;

[Yoyo Zhou, 2012/06/27 19:59:25]

You probably want something like WindowedNotificationObserver to avoid possible
races.

[Devlin, 2012/06/27 21:04:06]

Ooooh, fancy. Didn't know about that. Thanks :)

+  registrar.Add(this, chrome::NOTIFICATION_CRX_INSTALLER_DONE,
+                content::Source<CrxInstaller>(crx_installer));
+
+  ui_test_utils::RunMessageLoop();
+
+  extension = extension_service->GetExtensionById(
+      extension_infos[0].id, false); // don't include disabled extensions.
+
+  // The total series of events for this process will be:
+  //   Extension Install - install version 1
+  //   Extension Install - install version 2
+  //   Extension Unload  - disable version 1
+  //   Extension Update  - signal the udate to version 2
+  // We push back the corresponding ExtensionBasicInfos.
+  extension_infos.push_back(ExtensionBasicInfo(extension));
+  extension_infos.push_back(extension_infos[0]);
+  extension_infos.push_back(extension_infos[1]);
+
+  std::vector<int> expected_event_types;
+  expected_event_types.push_back(EVENT_EXTENSION_INSTALL);
+  expected_event_types.push_back(EVENT_EXTENSION_INSTALL);
+  expected_event_types.push_back(EVENT_EXTENSION_UNLOAD);
+  expected_event_types.push_back(EVENT_EXTENSION_UPDATE);
+
+  std::vector<linked_ptr<Event> > events = GetEvents();
+
+  CheckExtensionEvents(expected_event_types, extension_infos, events);
+
+  // There will be an additional field: The unload reason.
+  const base::DictionaryValue* value;
+  int unload_reason = 0;
+  ASSERT_TRUE(events[2]->data()->GetAsDictionary(&value));

[Yoyo Zhou, 2012/06/27 19:59:25]

ditto

[Devlin, 2012/06/27 21:04:06]

Done.

+  ASSERT_TRUE(value->GetInteger("unload_reason", &unload_reason));
+  ASSERT_EQ(extension_misc::UNLOAD_REASON_UPDATE, unload_reason);
+}
+
+}  // namespace performance_monitor