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

chrome/browser/performance_monitor/database.cc

 // Copyright (c) 2012 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/browser/performance_monitor/database.h"
 
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/logging.h"
 #include "base/path_service.h"
 #include "base/stl_util.h"
 #include "base/string_number_conversions.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/performance_monitor/key_builder.h"
 #include "chrome/common/chrome_paths.h"
 #include "content/public/browser/browser_thread.h"
 #include "third_party/leveldatabase/src/include/leveldb/db.h"
-#include "third_party/leveldatabase/src/include/leveldb/iterator.h"
-#include "third_party/leveldatabase/src/include/leveldb/write_batch.h"
 
 namespace performance_monitor {
 namespace {
 const char kDbDir[] = "Performance Monitor Databases";
 const char kRecentDb[] = "Recent Metrics";
 const char kMaxValueDb[] = "Max Value Metrics";
 const char kEventDb[] = "Events";
 const char kStateDb[] = "Configuration";
 const char kActiveIntervalDb[] = "Active Interval";
 const char kMetricDb[] = "Metrics";
@@ skipping 12 matching lines @@
                    base::Time::FromInternalValue(end_time_int));
 }
 
 double StringToDouble(const std::string& s) {
   double value = 0.0;
   if (!base::StringToDouble(s, &value))
     LOG(ERROR) << "Failed to convert " << s << " to double.";
   return value;
 }
 
-// Returns an event from the given JSON string; the scoped_ptr will be NULL if
-// we are unable to properly parse the JSON.
-scoped_ptr<Event> EventFromJSON(const std::string& data) {
-  Value* value = base::JSONReader::Read(data);
-  DictionaryValue* dict = NULL;
-  if (!value || !value->GetAsDictionary(&dict))
-    return scoped_ptr<Event>();
-
-  return Event::FromValue(scoped_ptr<DictionaryValue>(dict));
-}
-
 }  // namespace
 
 const char Database::kDatabaseSequenceToken[] =
     "_performance_monitor_db_sequence_token_";
 
 TimeRange::TimeRange() {
 }
 
 TimeRange::TimeRange(base::Time start_time, base::Time end_time)
     : start(start_time),
@@ skipping 75 matching lines @@
 
 Database::EventVector Database::GetEvents(EventType type,
                                           const base::Time& start,
                                           const base::Time& end) {
   CHECK(!content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   EventVector events;
   std::string start_key =
       key_builder_->CreateEventKey(start, EVENT_UNDEFINED);
   std::string end_key =
       key_builder_->CreateEventKey(end, EVENT_NUMBER_OF_EVENTS);
-  leveldb::WriteBatch invalid_entries;
   scoped_ptr<leveldb::Iterator> it(event_db_->NewIterator(read_options_));
   for (it->Seek(start_key);
        it->Valid() && it->key().ToString() <= end_key;
        it->Next()) {
     if (type != EVENT_UNDEFINED) {
       EventType key_type =
           key_builder_->EventKeyToEventType(it->key().ToString());
       if (key_type != type)
         continue;
     }
-    scoped_ptr<Event> event = EventFromJSON(it->value().ToString());
-    if (!event.get()) {
-      invalid_entries.Delete(it->key());
-      LOG(ERROR) << "Found invalid event in the database. JSON: '"
-                 <<  it->value().ToString()
-                 << "'. Erasing event from the database.";
+    base::DictionaryValue* dict = NULL;
+    if (!base::JSONReader::Read(
+        it->value().ToString())->GetAsDictionary(&dict)) {
+      LOG(ERROR) << "Unable to convert database event to JSON.";
       continue;
     }
+    scoped_ptr<Event> event =
+        Event::FromValue(scoped_ptr<base::DictionaryValue>(dict));
+    if (!event.get())
+      continue;
     events.push_back(linked_ptr<Event>(event.release()));
   }
-  event_db_->Write(write_options_, &invalid_entries);
   return events;
 }
 
 Database::EventTypeSet Database::GetEventTypes(const base::Time& start,
                                                const base::Time& end) {
   CHECK(!content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   EventTypeSet results;
   std::string start_key =
       key_builder_->CreateEventKey(start, EVENT_UNDEFINED);
   std::string end_key =
       key_builder_->CreateEventKey(end, EVENT_NUMBER_OF_EVENTS);
   scoped_ptr<leveldb::Iterator> it(event_db_->NewIterator(read_options_));
   for (it->Seek(start_key);
        it->Valid() && it->key().ToString() <= end_key;
        it->Next()) {
     EventType key_type =
         key_builder_->EventKeyToEventType(it->key().ToString());
     results.insert(key_type);
   }
   return results;
 }
 
 bool Database::AddMetric(const std::string& activity,
-                         const Metric& metric) {
+                         MetricType metric,
+                         const std::string& value) {
   CHECK(!content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
-  if (!metric.IsValid()) {
-    LOG(ERROR) << "Metric to be added is invalid. Type: " << metric.type
-               << ", Time: " << metric.time.ToInternalValue()
-               << ", Value: " << metric.value << ". Ignoring.";
-    return false;
-  }
-
   UpdateActiveInterval();
+  base::Time timestamp = clock_->GetTime();
   std::string recent_key =
-      key_builder_->CreateRecentKey(metric.time, metric.type, activity);
+      key_builder_->CreateRecentKey(timestamp, metric, activity);
   std::string metric_key =
-      key_builder_->CreateMetricKey(metric.time, metric.type, activity);
+      key_builder_->CreateMetricKey(timestamp, metric, activity);
   std::string recent_map_key =
-      key_builder_->CreateRecentMapKey(metric.type, activity);
+      key_builder_->CreateRecentMapKey(metric, activity);
   // Use recent_map_ to quickly find the key that must be removed.
   RecentMap::iterator old_it = recent_map_.find(recent_map_key);
   if (old_it != recent_map_.end())
     recent_db_->Delete(write_options_, old_it->second);
   recent_map_[recent_map_key] = recent_key;
   leveldb::Status recent_status =
-      recent_db_->Put(write_options_, recent_key, metric.ValueAsString());
+      recent_db_->Put(write_options_, recent_key, value);
   leveldb::Status metric_status =
-      metric_db_->Put(write_options_, metric_key, metric.ValueAsString());
+      metric_db_->Put(write_options_, metric_key, value);
 
-  bool max_value_success =
-      UpdateMaxValue(activity, metric.type, metric.ValueAsString());
+  bool max_value_success = UpdateMaxValue(activity, metric, value);
   return recent_status.ok() && metric_status.ok() && max_value_success;
 }
 
 bool Database::UpdateMaxValue(const std::string& activity,
                               MetricType metric,
                               const std::string& value) {
   std::string max_value_key(
       key_builder_->CreateMaxValueKey(metric, activity));
   bool has_key = ContainsKey(max_value_map_, max_value_key);
   if ((has_key && StringToDouble(value) > max_value_map_[max_value_key]) ||
@@ skipping 89 matching lines @@
   CHECK(!content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   std::string recent_map_key =
       key_builder_->CreateRecentMapKey(metric_type, activity);
   if (!ContainsKey(recent_map_, recent_map_key))
     return false;
   std::string recent_key = recent_map_[recent_map_key];
 
   std::string result;
   leveldb::Status status = recent_db_->Get(read_options_, recent_key, &result);
   if (status.ok())
-    *metric = Metric(metric_type,
-                     key_builder_->SplitRecentKey(recent_key).time,
-                     result);
+    *metric = Metric(key_builder_->SplitRecentKey(recent_key).time, result);
   return status.ok();
 }
 
 Database::MetricVector Database::GetStatsForActivityAndMetric(
     const std::string& activity,
     MetricType metric_type,
     const base::Time& start,
     const base::Time& end) {
   CHECK(!content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   MetricVector results;
   std::string start_key =
       key_builder_->CreateMetricKey(start, metric_type, activity);
   std::string end_key =
       key_builder_->CreateMetricKey(end, metric_type, activity);
-  leveldb::WriteBatch invalid_entries;
   scoped_ptr<leveldb::Iterator> it(metric_db_->NewIterator(read_options_));
   for (it->Seek(start_key);
        it->Valid() && it->key().ToString() <= end_key;
        it->Next()) {
     MetricKey split_key =
         key_builder_->SplitMetricKey(it->key().ToString());
-    if (split_key.activity == activity) {
-      Metric metric(metric_type, split_key.time, it->value().ToString());
-      if (!metric.IsValid()) {
-        invalid_entries.Delete(it->key());
-        LOG(ERROR) << "Found bad metric in the database. Type: "
-                   << metric.type << ", Time: " << metric.time.ToInternalValue()
-                   << ", Value: " << metric.value
-                   << ". Erasing metric from database.";
-        continue;
-      }
-      results.push_back(metric);
-    }
+    if (split_key.activity == activity)
+      results.push_back(Metric(split_key.time, it->value().ToString()));
   }
-  metric_db_->Write(write_options_, &invalid_entries);
   return results;
 }
 
 Database::MetricVectorMap Database::GetStatsForMetricByActivity(
     MetricType metric_type,
     const base::Time& start,
     const base::Time& end) {
   CHECK(!content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   MetricVectorMap results;
   std::string start_key =
       key_builder_->CreateMetricKey(start, metric_type, std::string());
   std::string end_key =
       key_builder_->CreateMetricKey(end, metric_type, std::string());
-  leveldb::WriteBatch invalid_entries;
   scoped_ptr<leveldb::Iterator> it(metric_db_->NewIterator(read_options_));
   for (it->Seek(start_key);
        it->Valid() && it->key().ToString() <= end_key;
        it->Next()) {
     MetricKey split_key = key_builder_->SplitMetricKey(it->key().ToString());
     if (!results[split_key.activity].get()) {
       results[split_key.activity] =
           linked_ptr<MetricVector >(new MetricVector());
     }
-    Metric metric(metric_type, split_key.time, it->value().ToString());
-    if (!metric.IsValid()) {
-      invalid_entries.Delete(it->key());
-      LOG(ERROR) << "Found bad metric in the database. Type: "
-                 << metric.type << ", Time: " << metric.time.ToInternalValue()
-                 << ", Value: " << metric.value
-                 << ". Erasing metric from database.";
-      continue;
-    }
-    results[split_key.activity]->push_back(metric);
+    results[split_key.activity]->push_back(
+        Metric(split_key.time, it->value().ToString()));
   }
-  metric_db_->Write(write_options_, &invalid_entries);
   return results;
 }
 
 Database::Database(const FilePath& path)
     : key_builder_(new KeyBuilder()),
       path_(path),
       read_options_(leveldb::ReadOptions()),
       write_options_(leveldb::WriteOptions()) {
   InitDBs();
   LoadRecents();
@@ skipping 99 matching lines @@
     start_time_key_ = key_builder_->CreateActiveIntervalKey(current_time);
     end_time = start_time_key_;
   } else {
     end_time = key_builder_->CreateActiveIntervalKey(clock_->GetTime());
   }
   last_update_time_ = current_time;
   active_interval_db_->Put(write_options_, start_time_key_, end_time);
 }
 
 }  // namespace performance_monitor