Revert 157168 - Add guards to metric values; erase bad events/metrics from db

chrome/browser/performance_monitor/database_unittest.cc

Index: chrome/browser/performance_monitor/database_unittest.cc
===================================================================
--- chrome/browser/performance_monitor/database_unittest.cc	(revision 157181)
+++ chrome/browser/performance_monitor/database_unittest.cc	(working copy)
@@ -12,72 +12,15 @@
 #include "base/scoped_temp_dir.h"
 #include "base/time.h"
 #include "chrome/browser/performance_monitor/database.h"
-#include "chrome/browser/performance_monitor/key_builder.h"
 #include "chrome/browser/performance_monitor/metric.h"
 #include "chrome/browser/performance_monitor/performance_monitor_util.h"
 #include "chrome/common/extensions/extension.h"
 #include "testing/gtest/include/gtest/gtest.h"
-#include "third_party/leveldatabase/src/include/leveldb/db.h"
-#include "third_party/leveldatabase/src/include/leveldb/iterator.h"
 
 using extensions::Extension;
 
 namespace performance_monitor {
 
-// A class which is friended by Database, in order to hold the private methods
-// and avoid friending all the different test classes.
-class DatabaseTestHelper {
- public:
-  explicit DatabaseTestHelper(Database* database) : database_(database) { };
-  ~DatabaseTestHelper() { };
-
-  bool Close() { return database_->Close(); }
-
-  // Override the check for a metric's validity and insert it in the database.
-  // Note: This does not do extraneous updates, like updating the recent_db or
-  // the max_value_map.
-  bool AddInvalidMetric(std::string activity, Metric metric) {
-    std::string metric_key =
-        database_->key_builder_->CreateMetricKey(database_->clock_->GetTime(),
-                                                 metric.type,
-                                                 activity);
-    leveldb::Status status =
-        database_->metric_db_->Put(database_->write_options_,
-                                   metric_key,
-                                   metric.ValueAsString());
-    return status.ok();
-  }
-
-  // Writes an invalid event to the database; since events are stored as JSON
-  // strings, this is equivalent to writing a garbage string.
-  bool AddInvalidEvent(base::Time time, EventType type) {
-    std::string key = database_->key_builder_->CreateEventKey(time, type);
-    leveldb::Status status =
-        database_->event_db_->Put(database_->write_options_, key, "fake_event");
-    return status.ok();
-  }
-
-  size_t GetNumberOfMetricEntries() {
-    return GetNumberOfEntries(database_->metric_db_.get());
-  }
-  size_t GetNumberOfEventEntries() {
-    return GetNumberOfEntries(database_->event_db_.get());
-  }
-
- private:
-  // Returns the number of entries in a given database.
-  size_t GetNumberOfEntries(leveldb::DB* level_db) {
-    size_t number_of_entries = 0;
-    scoped_ptr<leveldb::Iterator> iter(
-        level_db->NewIterator(database_->read_options_));
-    for (iter->SeekToFirst(); iter->Valid(); iter->Next())
-      ++number_of_entries;
-    return number_of_entries;
-  }
-
-  Database* database_;
-};
-
 // A clock that increments every access. Great for testing.
 class TestingClock : public Database::Clock {
  public:
@@ -164,18 +107,14 @@
   }
 
   void PopulateDB() {
-    db_->AddMetric(kProcessChromeAggregate,
-                   Metric(METRIC_CPU_USAGE, clock_->GetTime(), 50.5));
-    db_->AddMetric(activity_,
-                   Metric(METRIC_CPU_USAGE, clock_->GetTime(), 13.1));
-    db_->AddMetric(kProcessChromeAggregate,
-                   Metric(METRIC_PRIVATE_MEMORY_USAGE,
-                          clock_->GetTime(),
-                          1000000.0));
-    db_->AddMetric(activity_,
-                   Metric(METRIC_PRIVATE_MEMORY_USAGE,
-                          clock_->GetTime(),
-                          3000000.0));
+    db_->AddMetric(kProcessChromeAggregate, METRIC_CPU_USAGE,
+                   std::string("50.5"));
+    db_->AddMetric(activity_, METRIC_CPU_USAGE, std::string("13.1"));
+
+    db_->AddMetric(kProcessChromeAggregate, METRIC_PRIVATE_MEMORY_USAGE,
+                   std::string("1000000"));
+    db_->AddMetric(activity_, METRIC_PRIVATE_MEMORY_USAGE,
+                   std::string("3000000"));
   }
 
   scoped_ptr<Database> db_;
@@ -190,8 +129,7 @@
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
   scoped_ptr<Database> db = Database::Create(temp_dir.path());
   ASSERT_TRUE(db.get());
-  DatabaseTestHelper helper(db.get());
-  ASSERT_TRUE(helper.Close());
+  ASSERT_TRUE(db->Close());
 }
 
 TEST(PerformanceMonitorDatabaseSetupTest, ActiveInterval) {
@@ -203,19 +141,17 @@
 
   scoped_ptr<Database> db_1 = Database::Create(temp_dir.path());
   db_1->set_clock(scoped_ptr<Database::Clock>(clock_1));
-  DatabaseTestHelper helper1(db_1.get());
   db_1->AddStateValue("test", "test");
   ASSERT_TRUE(db_1.get());
-  ASSERT_TRUE(helper1.Close());
+  ASSERT_TRUE(db_1->Close());
 
   TestingClock* clock_2 = new TestingClock(*clock_1);
   base::Time mid_time = clock_2->GetTime();
   scoped_ptr<Database> db_2 = Database::Create(temp_dir.path());
   db_2->set_clock(scoped_ptr<Database::Clock>(clock_2));
-  DatabaseTestHelper helper2(db_2.get());
   db_2->AddStateValue("test", "test");
   ASSERT_TRUE(db_2.get());
-  ASSERT_TRUE(helper2.Close());
+  ASSERT_TRUE(db_2->Close());
 
   TestingClock* clock_3 = new TestingClock(*clock_2);
   base::Time end_time = clock_3->GetTime();
@@ -289,15 +225,13 @@
   EXPECT_EQ(1000000,
             db_->GetMaxStatsForActivityAndMetric(METRIC_PRIVATE_MEMORY_USAGE));
 
-  db_->AddMetric(kProcessChromeAggregate,
-                 Metric(METRIC_PRIVATE_MEMORY_USAGE, clock_->GetTime(), 99.0));
+  db_->AddMetric(kProcessChromeAggregate, METRIC_PRIVATE_MEMORY_USAGE,
+                 std::string("99"));
   EXPECT_EQ(1000000,
             db_->GetMaxStatsForActivityAndMetric(METRIC_PRIVATE_MEMORY_USAGE));
 
-  db_->AddMetric(kProcessChromeAggregate,
-                 Metric(METRIC_PRIVATE_MEMORY_USAGE,
-                        clock_->GetTime(),
-                        6000000.0));
+  db_->AddMetric(kProcessChromeAggregate, METRIC_PRIVATE_MEMORY_USAGE,
+                 std::string("6000000"));
   EXPECT_EQ(6000000,
             db_->GetMaxStatsForActivityAndMetric(METRIC_PRIVATE_MEMORY_USAGE));
 }
@@ -326,23 +260,6 @@
   EXPECT_TRUE(events[1]->data()->Equals(install_event_2_->data()));
 }
 
-TEST_F(PerformanceMonitorDatabaseEventTest, InvalidEvents) {
-  DatabaseTestHelper helper(db_.get());
-
-  // Insert an invalid event into the database; verify it is inserted.
-  size_t original_number_of_entries = helper.GetNumberOfEventEntries();
-  ASSERT_TRUE(helper.AddInvalidEvent(
-      clock_->GetTime(), EVENT_EXTENSION_INSTALL));
-  ASSERT_EQ(original_number_of_entries + 1u, helper.GetNumberOfEventEntries());
-
-  // Should not retrieve the invalid event.
-  Database::EventVector events = db_->GetEvents();
-  ASSERT_EQ(original_number_of_entries, events.size());
-
-  // Invalid event should have been deleted.
-  ASSERT_EQ(original_number_of_entries, helper.GetNumberOfEventEntries());
-}
-
 TEST_F(PerformanceMonitorDatabaseEventTest, GetUnusedEventType) {
   Database::EventVector events = db_->GetEvents(EVENT_EXTENSION_DISABLE);
   ASSERT_TRUE(events.empty());
@@ -426,8 +343,7 @@
   ASSERT_EQ(1u, stats.size());
   EXPECT_EQ(13.1, stats[0].value);
   base::Time before = clock_->GetTime();
-  db_->AddMetric(activity_,
-                 Metric(METRIC_CPU_USAGE, clock_->GetTime(), 18.0));
+  db_->AddMetric(activity_, METRIC_CPU_USAGE, std::string("18"));
   stats = db_->GetStatsForActivityAndMetric(activity_, METRIC_CPU_USAGE,
                                             before, clock_->GetTime());
   ASSERT_EQ(1u, stats.size());
@@ -459,8 +375,7 @@
       METRIC_CPU_USAGE, clock_->GetTime(), clock_->GetTime());
   EXPECT_EQ(0u, stats_map.size());
   base::Time before = clock_->GetTime();
-  db_->AddMetric(activity_,
-                 Metric(METRIC_CPU_USAGE, clock_->GetTime(), 18.0));
+  db_->AddMetric(activity_, METRIC_CPU_USAGE, std::string("18"));
   stats_map = db_->GetStatsForMetricByActivity(METRIC_CPU_USAGE, before,
                                                clock_->GetTime());
   ASSERT_EQ(1u, stats_map.size());
@@ -469,43 +384,9 @@
   EXPECT_EQ(18, stats->at(0).value);
 }
 
-TEST_F(PerformanceMonitorDatabaseMetricTest, InvalidMetrics) {
-  DatabaseTestHelper helper(db_.get());
-  Metric invalid_metric(METRIC_CPU_USAGE, clock_->GetTime(), -5.0);
-  ASSERT_FALSE(invalid_metric.IsValid());
-
-  // Find the original number of entries in the database.
-  size_t original_number_of_entries = helper.GetNumberOfMetricEntries();
-  Database::MetricVector stats = db_->GetStatsForActivityAndMetric(
-      activity_, METRIC_CPU_USAGE, base::Time(), clock_->GetTime());
-  size_t original_number_of_cpu_entries = stats.size();
-
-  // Check that the database normally refuses to insert bad metrics.
-  EXPECT_FALSE(db_->AddMetric(invalid_metric));
-
-  // Verify that it was not inserted into the database.
-  stats = db_->GetStatsForActivityAndMetric(
-      activity_, METRIC_CPU_USAGE, base::Time(), clock_->GetTime());
-  ASSERT_EQ(original_number_of_cpu_entries, stats.size());
-
-  // Forcefully insert it into the database.
-  ASSERT_TRUE(helper.AddInvalidMetric(activity_, invalid_metric));
-  ASSERT_EQ(original_number_of_entries + 1u, helper.GetNumberOfMetricEntries());
-
-  // Try to retrieve it; should only get one result.
-  stats = db_->GetStatsForActivityAndMetric(
-      activity_, METRIC_CPU_USAGE, base::Time(), clock_->GetTime());
-  ASSERT_EQ(original_number_of_cpu_entries, stats.size());
-
-  // Entry should have been deleted in the database.
-  ASSERT_EQ(original_number_of_entries, helper.GetNumberOfMetricEntries());
-}
-
 TEST_F(PerformanceMonitorDatabaseMetricTest, GetFullRange) {
-  db_->AddMetric(kProcessChromeAggregate,
-                 Metric(METRIC_CPU_USAGE, clock_->GetTime(), 3.4));
-  db_->AddMetric(kProcessChromeAggregate,
-                 Metric(METRIC_CPU_USAGE, clock_->GetTime(), 21.0));
+  db_->AddMetric(kProcessChromeAggregate, METRIC_CPU_USAGE, std::string("3.4"));
+  db_->AddMetric(kProcessChromeAggregate, METRIC_CPU_USAGE, std::string("21"));
   Database::MetricVector stats =
       db_->GetStatsForActivityAndMetric(METRIC_CPU_USAGE);
   ASSERT_EQ(3u, stats.size());
@@ -516,13 +397,10 @@
 
 TEST_F(PerformanceMonitorDatabaseMetricTest, GetRange) {
   base::Time start = clock_->GetTime();
-  db_->AddMetric(kProcessChromeAggregate,
-                 Metric(METRIC_CPU_USAGE, clock_->GetTime(), 3.0));
-  db_->AddMetric(kProcessChromeAggregate,
-                 Metric(METRIC_CPU_USAGE, clock_->GetTime(), 9.0));
+  db_->AddMetric(kProcessChromeAggregate, METRIC_CPU_USAGE, std::string("3"));
+  db_->AddMetric(kProcessChromeAggregate, METRIC_CPU_USAGE, std::string("9"));
   base::Time end = clock_->GetTime();
-  db_->AddMetric(kProcessChromeAggregate,
-                 Metric(METRIC_CPU_USAGE, clock_->GetTime(), 21.0));
+  db_->AddMetric(kProcessChromeAggregate, METRIC_CPU_USAGE, std::string("21"));
   Database::MetricVector stats =
       db_->GetStatsForActivityAndMetric(METRIC_CPU_USAGE, start, end);
   ASSERT_EQ(2u, stats.size());