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

chrome/browser/performance_monitor/performance_monitor.h

Index: chrome/browser/performance_monitor/performance_monitor.h
diff --git a/chrome/browser/performance_monitor/performance_monitor.h b/chrome/browser/performance_monitor/performance_monitor.h
index a51253c11829b26a101b3007996e2157b82bd9ce..0dd1ddab32d845f7c02079bd6f427c52e49a7d1d 100644
--- a/chrome/browser/performance_monitor/performance_monitor.h
+++ b/chrome/browser/performance_monitor/performance_monitor.h
@@ -131,8 +131,9 @@ class PerformanceMonitor : public content::NotificationObserver {
   void AddEventOnBackgroundThread(scoped_ptr<Event> event);
 
   // Since Database::AddMetric() is overloaded, base::Bind() does not work and
-  // we need a helper function.
-  void AddMetricOnBackgroundThread(MetricType type, const std::string& value);
+  // we need a helper function. Deliberately not const & so that we will
+  // construct a new metric on the background thread.

[Yoyo Zhou, 2012/09/10 23:52:44]

I'm not sure this does what you think. You are still constructing Metric where
you post the callback, and then making a copy of it, if I understand correctly.

[Devlin, 2012/09/11 18:52:17]

I was modeling this after the io_data one. In that, Battre recommended I pass in
a copy, rather than a const ref, so that it would be thread-safe (see here:
https://chromiumcodereview.appspot.com/10829342/diff2/9003:10007/chrome/brows...
). I think that a const& would be safe here so long as we always construct the
Metric in the call (and the callback owns it), but would it still be safe if we
did:

Metric metric(...);
base::Bind(..., metric)

or would PerformanceMonitor's thread still own it?

[Yoyo Zhou, 2012/09/13 00:09:20]

On reviewing the documentation in callback.h, this looks correct. Sorry for the
confusion.

+  void AddMetricOnBackgroundThread(Metric metric);
 
   // Notify any listeners that PerformanceMonitor has finished the initializing.
   void NotifyInitialized();