Revert 49354 - causing regression in MetricsLogTest.ChromeOSEmptyRecord on CrOS

Original r49354 description: Preparation CL for adding crash metrics UMA counters to ChromeFrame. Basically this would mean that metrics reporting would be done directly
by ChromeFrame. Currently ChromeFrame uses Chrome to upload this data. 

Refactored the chrome metrics service and metrics logging functionality into base classes defined in chrome\common\metrics_helpers.cc/.h. While
this refactoring is by no means complete it is a first step to avoid needless code duplication between chrome and chrome frame.

Bug=46057

Review URL: http://codereview.chromium.org/2744003

TBR=ananta@chromium.org
Review URL: http://codereview.chromium.org/2779013

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@49382 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 642 insertions, 44 deletions

diff --git a/chrome/browser/metrics/metrics_log.cc b/chrome/browser/metrics/metrics_log.cc
index f638857f..cf25e58 100644
--- a/chrome/browser/metrics/metrics_log.cc
+++ b/chrome/browser/metrics/metrics_log.cc
@@ -26,46 +26,318 @@
 
 #define OPEN_ELEMENT_FOR_SCOPE(name) ScopedElement scoped_element(this, name)
 
+using base::Time;
+using base::TimeDelta;
+
 // http://blogs.msdn.com/oldnewthing/archive/2004/10/25/247180.aspx
 #if defined(OS_WIN)
 extern "C" IMAGE_DOS_HEADER __ImageBase;
 #endif
 
-MetricsLog::MetricsLog(const std::string& client_id, int session_id)
-    : MetricsLogBase(client_id, session_id, MetricsLog::GetVersionString()) {}
+// static
+std::string MetricsLog::version_extension_;
 
-MetricsLog::~MetricsLog() {}
+// libxml take xmlChar*, which is unsigned char*
+inline const unsigned char* UnsignedChar(const char* input) {
+  return reinterpret_cast<const unsigned char*>(input);
+}
 
 // static
 void MetricsLog::RegisterPrefs(PrefService* local_state) {
   local_state->RegisterListPref(prefs::kStabilityPluginStats);
 }
 
-int64 MetricsLog::GetIncrementalUptime(PrefService* pref) {
-  base::TimeTicks now = base::TimeTicks::Now();
-  static base::TimeTicks last_updated_time(now);
-  int64 incremental_time = (now - last_updated_time).InSeconds();
-  last_updated_time = now;
+MetricsLog::MetricsLog(const std::string& client_id, int session_id)
+    : start_time_(Time::Now()),
+      client_id_(client_id),
+      session_id_(IntToString(session_id)),
+      locked_(false),
+      doc_(NULL),
+      buffer_(NULL),
+      writer_(NULL),
+      num_events_(0) {
+
+  buffer_ = xmlBufferCreate();
+  DCHECK(buffer_);
+
+#if defined(OS_CHROMEOS)
+  writer_ = xmlNewTextWriterDoc(&doc_, /* compression */ 0);
+#else
+  writer_ = xmlNewTextWriterMemory(buffer_, /* compression */ 0);
+#endif  // OS_CHROMEOS
+  DCHECK(writer_);
+
+  int result = xmlTextWriterSetIndent(writer_, 2);
+  DCHECK_EQ(0, result);
+
+  StartElement("log");
+  WriteAttribute("clientid", client_id_);
+  WriteInt64Attribute("buildtime", GetBuildTime());
+  WriteAttribute("appversion", GetVersionString());
+}
 
-  if (incremental_time > 0) {
-    int64 metrics_uptime = pref->GetInt64(prefs::kUninstallMetricsUptimeSec);
-    metrics_uptime += incremental_time;
-    pref->SetInt64(prefs::kUninstallMetricsUptimeSec, metrics_uptime);
+MetricsLog::~MetricsLog() {
+  FreeDocWriter();
+
+  if (buffer_) {
+    xmlBufferFree(buffer_);
+    buffer_ = NULL;
+  }
+}
+
+void MetricsLog::CloseLog() {
+  DCHECK(!locked_);
+  locked_ = true;
+
+  int result = xmlTextWriterEndDocument(writer_);
+  DCHECK_GE(result, 0);
+
+  result = xmlTextWriterFlush(writer_);
+  DCHECK_GE(result, 0);
+
+#if defined(OS_CHROMEOS)
+  xmlNodePtr root = xmlDocGetRootElement(doc_);
+  if (!hardware_class_.empty()) {
+    // The hardware class is determined after the first ongoing log is
+    // constructed, so this adds the root element's "hardwareclass"
+    // attribute when the log is closed instead.
+    xmlNewProp(root, UnsignedChar("hardwareclass"),
+               UnsignedChar(hardware_class_.c_str()));
   }
 
-  return incremental_time;
+  // Flattens the XML tree into a character buffer.
+  PerfTimer dump_timer;
+  result = xmlNodeDump(buffer_, doc_, root, /* level */ 0, /* format */ 1);
+  DCHECK_GE(result, 0);
+  UMA_HISTOGRAM_TIMES("UMA.XMLNodeDumpTime", dump_timer.Elapsed());
+
+  PerfTimer free_timer;
+  FreeDocWriter();
+  UMA_HISTOGRAM_TIMES("UMA.XMLWriterDestructionTime", free_timer.Elapsed());
+#endif  // OS_CHROMEOS
 }
 
-std::string MetricsLog::GetInstallDate() const {
-  PrefService* pref = g_browser_process->local_state();
-  if (pref) {
-    return WideToUTF8(pref->GetString(prefs::kMetricsClientIDTimestamp));
-  } else {
-    NOTREACHED();
-    return "0";
+int MetricsLog::GetEncodedLogSize() {
+  DCHECK(locked_);
+  return buffer_->use;
+}
+
+bool MetricsLog::GetEncodedLog(char* buffer, int buffer_size) {
+  DCHECK(locked_);
+  if (buffer_size < GetEncodedLogSize())
+    return false;
+
+  memcpy(buffer, buffer_->content, GetEncodedLogSize());
+  return true;
+}
+
+int MetricsLog::GetElapsedSeconds() {
+  return static_cast<int>((Time::Now() - start_time_).InSeconds());
+}
+
+std::string MetricsLog::CreateHash(const std::string& value) {
+  MD5Context ctx;
+  MD5Init(&ctx);
+  MD5Update(&ctx, value.data(), value.length());
+
+  MD5Digest digest;
+  MD5Final(&digest, &ctx);
+
+  uint64 reverse_uint64;
+  // UMA only uses first 8 chars of hash. We use the above uint64 instead
+  // of a unsigned char[8] so that we don't run into strict aliasing issues
+  // in the LOG statement below when trying to interpret reverse as a uint64.
+  unsigned char* reverse = reinterpret_cast<unsigned char *>(&reverse_uint64);
+  DCHECK(arraysize(digest.a) >= sizeof(reverse_uint64));
+  for (size_t i = 0; i < sizeof(reverse_uint64); ++i)
+    reverse[i] = digest.a[sizeof(reverse_uint64) - i - 1];
+  // The following log is VERY helpful when folks add some named histogram into
+  // the code, but forgot to update the descriptive list of histograms.  When
+  // that happens, all we get to see (server side) is a hash of the histogram
+  // name.  We can then use this logging to find out what histogram name was
+  // being hashed to a given MD5 value by just running the version of Chromium
+  // in question with --enable-logging.
+  LOG(INFO) << "Metrics: Hash numeric [" << value << "]=["
+      << reverse_uint64 << "]";
+  return std::string(reinterpret_cast<char*>(digest.a), arraysize(digest.a));
+}
+
+std::string MetricsLog::CreateBase64Hash(const std::string& string) {
+  std::string encoded_digest;
+  if (base::Base64Encode(CreateHash(string), &encoded_digest)) {
+    DLOG(INFO) << "Metrics: Hash [" << encoded_digest << "]=[" << string << "]";
+    return encoded_digest;
+  }
+  return std::string();
+}
+
+void MetricsLog::RecordUserAction(const char* key) {
+  DCHECK(!locked_);
+
+  std::string command_hash = CreateBase64Hash(key);
+  if (command_hash.empty()) {
+    NOTREACHED() << "Unable generate encoded hash of command: " << key;
+    return;
+  }
+
+  OPEN_ELEMENT_FOR_SCOPE("uielement");
+  WriteAttribute("action", "command");
+  WriteAttribute("targetidhash", command_hash);
+
+  // TODO(jhughes): Properly track windows.
+  WriteIntAttribute("window", 0);
+  WriteCommonEventAttributes();
+
+  ++num_events_;
+}
+
+void MetricsLog::RecordLoadEvent(int window_id,
+                                 const GURL& url,
+                                 PageTransition::Type origin,
+                                 int session_index,
+                                 TimeDelta load_time) {
+  DCHECK(!locked_);
+
+  OPEN_ELEMENT_FOR_SCOPE("document");
+  WriteAttribute("action", "load");
+  WriteIntAttribute("docid", session_index);
+  WriteIntAttribute("window", window_id);
+  WriteAttribute("loadtime", Int64ToString(load_time.InMilliseconds()));
+
+  std::string origin_string;
+
+  switch (PageTransition::StripQualifier(origin)) {
+    // TODO(jhughes): Some of these mappings aren't right... we need to add
+    // some values to the server's enum.
+    case PageTransition::LINK:
+    case PageTransition::MANUAL_SUBFRAME:
+      origin_string = "link";
+      break;
+
+    case PageTransition::TYPED:
+      origin_string = "typed";
+      break;
+
+    case PageTransition::AUTO_BOOKMARK:
+      origin_string = "bookmark";
+      break;
+
+    case PageTransition::AUTO_SUBFRAME:
+    case PageTransition::RELOAD:
+      origin_string = "refresh";
+      break;
+
+    case PageTransition::GENERATED:
+    case PageTransition::KEYWORD:
+      origin_string = "global-history";
+      break;
+
+    case PageTransition::START_PAGE:
+      origin_string = "start-page";
+      break;
+
+    case PageTransition::FORM_SUBMIT:
+      origin_string = "form-submit";
+      break;
+
+    default:
+      NOTREACHED() << "Received an unknown page transition type: " <<
+                       PageTransition::StripQualifier(origin);
+  }
+  if (!origin_string.empty())
+    WriteAttribute("origin", origin_string);
+
+  WriteCommonEventAttributes();
+
+  ++num_events_;
+}
+
+void MetricsLog::RecordWindowEvent(WindowEventType type,
+                                   int window_id,
+                                   int parent_id) {
+  DCHECK(!locked_);
+
+  OPEN_ELEMENT_FOR_SCOPE("window");
+  WriteAttribute("action", WindowEventTypeToString(type));
+  WriteAttribute("windowid", IntToString(window_id));
+  if (parent_id >= 0)
+    WriteAttribute("parent", IntToString(parent_id));
+  WriteCommonEventAttributes();
+
+  ++num_events_;
+}
+
+std::string MetricsLog::GetCurrentTimeString() {
+  return Uint64ToString(Time::Now().ToTimeT());
+}
+
+// These are the attributes that are common to every event.
+void MetricsLog::WriteCommonEventAttributes() {
+  WriteAttribute("session", session_id_);
+  WriteAttribute("time", GetCurrentTimeString());
+}
+
+void MetricsLog::WriteAttribute(const std::string& name,
+                                const std::string& value) {
+  DCHECK(!locked_);
+  DCHECK(!name.empty());
+
+  int result = xmlTextWriterWriteAttribute(writer_,
+                                           UnsignedChar(name.c_str()),
+                                           UnsignedChar(value.c_str()));
+  DCHECK_GE(result, 0);
+}
+
+void MetricsLog::WriteIntAttribute(const std::string& name, int value) {
+  WriteAttribute(name, IntToString(value));
+}
+
+void MetricsLog::WriteInt64Attribute(const std::string& name, int64 value) {
+  WriteAttribute(name, Int64ToString(value));
+}
+
+// static
+const char* MetricsLog::WindowEventTypeToString(WindowEventType type) {
+  switch (type) {
+    case WINDOW_CREATE:  return "create";
+    case WINDOW_OPEN:    return "open";
+    case WINDOW_CLOSE:   return "close";
+    case WINDOW_DESTROY: return "destroy";
+
+    default:
+      NOTREACHED();
+      return "unknown";  // Can't return NULL as this is used in a required
+                         // attribute.
+  }
+}
+
+void MetricsLog::FreeDocWriter() {
+  if (writer_) {
+    xmlFreeTextWriter(writer_);
+    writer_ = NULL;
+  }
+
+  if (doc_) {
+    xmlFreeDoc(doc_);
+    doc_ = NULL;
   }
 }
 
+void MetricsLog::StartElement(const char* name) {
+  DCHECK(!locked_);
+  DCHECK(name);
+
+  int result = xmlTextWriterStartElement(writer_, UnsignedChar(name));
+  DCHECK_GE(result, 0);
+}
+
+void MetricsLog::EndElement() {
+  DCHECK(!locked_);
+
+  int result = xmlTextWriterEndElement(writer_);
+  DCHECK_GE(result, 0);
+}
+
 // static
 std::string MetricsLog::GetVersionString() {
   scoped_ptr<FileVersionInfo> version_info(
@@ -84,6 +356,45 @@ std::string MetricsLog::GetVersionString() {
   return std::string();
 }
 
+// static
+int64 MetricsLog::GetBuildTime() {
+  static int64 integral_build_time = 0;
+  if (!integral_build_time) {
+    Time time;
+    const char* kDateTime = __DATE__ " " __TIME__ " GMT";
+    bool result = Time::FromString(ASCIIToWide(kDateTime).c_str(), &time);
+    DCHECK(result);
+    integral_build_time = static_cast<int64>(time.ToTimeT());
+  }
+  return integral_build_time;
+}
+
+// static
+int64 MetricsLog::GetIncrementalUptime(PrefService* pref) {
+  base::TimeTicks now = base::TimeTicks::Now();
+  static base::TimeTicks last_updated_time(now);
+  int64 incremental_time = (now - last_updated_time).InSeconds();
+  last_updated_time = now;
+
+  if (incremental_time > 0) {
+    int64 metrics_uptime = pref->GetInt64(prefs::kUninstallMetricsUptimeSec);
+    metrics_uptime += incremental_time;
+    pref->SetInt64(prefs::kUninstallMetricsUptimeSec, metrics_uptime);
+  }
+
+  return incremental_time;
+}
+
+std::string MetricsLog::GetInstallDate() const {
+  PrefService* pref = g_browser_process->local_state();
+  if (pref) {
+    return WideToUTF8(pref->GetString(prefs::kMetricsClientIDTimestamp));
+  } else {
+    NOTREACHED();
+    return "0";
+  }
+}
+
 void MetricsLog::RecordIncrementalStabilityElements() {
   DCHECK(!locked_);
 
@@ -440,3 +751,32 @@ void MetricsLog::RecordOmniboxOpenedURL(const AutocompleteLog& log) {
 
   ++num_events_;
 }
+
+// TODO(JAR): A The following should really be part of the histogram class.
+// Internal state is being needlessly exposed, and it would be hard to reuse
+// this code. If we moved this into the Histogram class, then we could use
+// the same infrastructure for logging StatsCounters, RatesCounters, etc.
+void MetricsLog::RecordHistogramDelta(const Histogram& histogram,
+                                      const Histogram::SampleSet& snapshot) {
+  DCHECK(!locked_);
+  DCHECK_NE(0, snapshot.TotalCount());
+  snapshot.CheckSize(histogram);
+
+  // We will ignore the MAX_INT/infinite value in the last element of range[].
+
+  OPEN_ELEMENT_FOR_SCOPE("histogram");
+
+  WriteAttribute("name", CreateBase64Hash(histogram.histogram_name()));
+
+  WriteInt64Attribute("sum", snapshot.sum());
+  WriteInt64Attribute("sumsquares", snapshot.square_sum());
+
+  for (size_t i = 0; i < histogram.bucket_count(); i++) {
+    if (snapshot.counts(i)) {
+      OPEN_ELEMENT_FOR_SCOPE("histogrambucket");
+      WriteIntAttribute("min", histogram.ranges(i));
+      WriteIntAttribute("max", histogram.ranges(i + 1));
+      WriteIntAttribute("count", snapshot.counts(i));
+    }
+  }
+}
diff --git a/chrome/browser/metrics/metrics_log.h b/chrome/browser/metrics/metrics_log.h
index 720e55a..d9d5f38 100644
--- a/chrome/browser/metrics/metrics_log.h
+++ b/chrome/browser/metrics/metrics_log.h
@@ -8,8 +8,14 @@
 #ifndef CHROME_BROWSER_METRICS_METRICS_LOG_H_
 #define CHROME_BROWSER_METRICS_METRICS_LOG_H_
 
+#include <libxml/xmlwriter.h>
+
+#include <string>
+#include <vector>
+
 #include "base/basictypes.h"
-#include "chrome/common/metrics_helpers.h"
+#include "base/histogram.h"
+#include "base/time.h"
 #include "chrome/common/page_transition_types.h"
 #include "webkit/glue/plugins/webplugininfo.h"
 
@@ -18,7 +24,7 @@ class DictionaryValue;
 class GURL;
 class PrefService;
 
-class MetricsLog : public MetricsLogBase {
+class MetricsLog {
  public:
   // Creates a new metrics log
   // client_id is the identifier for this profile on this installation
@@ -28,6 +34,29 @@ class MetricsLog : public MetricsLogBase {
 
   static void RegisterPrefs(PrefService* prefs);
 
+  // Records a user-initiated action.
+  void RecordUserAction(const char* key);
+
+  enum WindowEventType {
+    WINDOW_CREATE = 0,
+    WINDOW_OPEN,
+    WINDOW_CLOSE,
+    WINDOW_DESTROY
+  };
+
+  void RecordWindowEvent(WindowEventType type, int window_id, int parent_id);
+
+  // Records a page load.
+  // window_id - the index of the tab in which the load took place
+  // url - which URL was loaded
+  // origin - what kind of action initiated the load
+  // load_time - how long it took to load the page
+  void RecordLoadEvent(int window_id,
+                       const GURL& url,
+                       PageTransition::Type origin,
+                       int session_index,
+                       base::TimeDelta load_time);
+
   // Records the current operating environment.  Takes the list of installed
   // plugins as a parameter because that can't be obtained synchronously
   // from the UI thread.
@@ -41,33 +70,113 @@ class MetricsLog : public MetricsLogBase {
   // user uses the Omnibox to open a URL.
   void RecordOmniboxOpenedURL(const AutocompleteLog& log);
 
+  // Record any changes in a given histogram for transmission.
+  void RecordHistogramDelta(const Histogram& histogram,
+                            const Histogram::SampleSet& snapshot);
+
   // Record recent delta for critical stability metrics.  We can't wait for a
   // restart to gather these, as that delay biases our observation away from
   // users that run happily for a looooong time.  We send increments with each
   // uma log upload, just as we send histogram data.
   void RecordIncrementalStabilityElements();
 
+  // Stop writing to this record and generate the encoded representation.
+  // None of the Record* methods can be called after this is called.
+  void CloseLog();
+
+  // These methods allow retrieval of the encoded representation of the
+  // record.  They can only be called after CloseLog() has been called.
+  // GetEncodedLog returns false if buffer_size is less than
+  // GetEncodedLogSize();
+  int GetEncodedLogSize();
+  bool GetEncodedLog(char* buffer, int buffer_size);
+
+  // Returns the amount of time in seconds that this log has been in use.
+  int GetElapsedSeconds();
+
+  int num_events() { return num_events_; }
   void set_hardware_class(const std::string& hardware_class) {
     hardware_class_ = hardware_class;
   }
 
-  // Get the amount of uptime in seconds since this function was last called.
-  // This updates the cumulative uptime metric for uninstall as a side effect.
-  static int64 GetIncrementalUptime(PrefService* pref);
+  // Creates an MD5 hash of the given value, and returns hash as a byte
+  // buffer encoded as a std::string.
+  static std::string CreateHash(const std::string& value);
+
+  // Return a base64-encoded MD5 hash of the given string.
+  static std::string CreateBase64Hash(const std::string& string);
 
   // Get the current version of the application as a string.
   static std::string GetVersionString();
 
-  virtual MetricsLog* AsMetricsLog() {
-    return this;
+  // Get the GMT buildtime for the current binary, expressed in seconds since
+  // January 1, 1970 GMT.
+  // The value is used to identify when a new build is run, so that previous
+  // reliability stats, from other builds, can be abandoned.
+  static int64 GetBuildTime();
+
+  // Get the amount of uptime in seconds since this function was last called.
+  // This updates the cumulative uptime metric for uninstall as a side effect.
+  static int64 GetIncrementalUptime(PrefService* pref);
+
+  // Use |extension| in all uploaded appversions in addition to the standard
+  // version string.
+  static void set_version_extension(const std::string& extension) {
+    version_extension_ = extension;
   }
 
+ protected:
+  // Returns a string containing the current time.
+  // Virtual so that it can be overridden for testing.
+  virtual std::string GetCurrentTimeString();
+
  private:
+  // Helper class that invokes StartElement from constructor, and EndElement
+  // from destructor.
+  //
+  // Use the macro OPEN_ELEMENT_FOR_SCOPE to help avoid usage problems.
+  class ScopedElement {
+   public:
+    ScopedElement(MetricsLog* log, const std::string& name) : log_(log) {
+      DCHECK(log);
+      log->StartElement(name.c_str());
+    }
+
+    ScopedElement(MetricsLog* log, const char* name) : log_(log) {
+      DCHECK(log);
+      log->StartElement(name);
+    }
+
+    ~ScopedElement() {
+      log_->EndElement();
+    }
+
+   private:
+     MetricsLog* log_;
+  };
+  friend class ScopedElement;
+
+  static const char* WindowEventTypeToString(WindowEventType type);
+
+  // Frees the resources allocated by the XML document writer: the
+  // main writer object as well as the XML tree structure, if
+  // applicable.
+  void FreeDocWriter();
+
+  // Convenience versions of xmlWriter functions
+  void StartElement(const char* name);
+  void EndElement();
+  void WriteAttribute(const std::string& name, const std::string& value);
+  void WriteIntAttribute(const std::string& name, int value);
+  void WriteInt64Attribute(const std::string& name, int64 value);
+
+  // Write the attributes that are common to every metrics event type.
+  void WriteCommonEventAttributes();
+
   // Returns the date at which the current metrics client ID was created as
   // a string containing milliseconds since the epoch, or "0" if none was found.
   std::string GetInstallDate() const;
 
-
   // Writes application stability metrics (as part of the profile log).
   // NOTE: Has the side-effect of clearing those counts.
   void WriteStabilityElement();
@@ -99,8 +208,26 @@ class MetricsLog : public MetricsLogBase {
   void WriteProfileMetrics(const std::wstring& key,
                            const DictionaryValue& profile_metrics);
 
+  // An extension that is appended to the appversion in each log.
+  static std::string version_extension_;
+
+  base::Time start_time_;
+  base::Time end_time_;
+
+  std::string client_id_;
+  std::string session_id_;
   std::string hardware_class_;
 
+  // locked_ is true when record has been packed up for sending, and should
+  // no longer be written to.  It is only used for sanity checking and is
+  // not a real lock.
+  bool locked_;
+
+  xmlDocPtr doc_;
+  xmlBufferPtr buffer_;
+  xmlTextWriterPtr writer_;
+  int num_events_;  // the number of events recorded in this log
+
   DISALLOW_COPY_AND_ASSIGN(MetricsLog);
 };
 
diff --git a/chrome/browser/metrics/metrics_service.cc b/chrome/browser/metrics/metrics_service.cc
index 1d5673a..cc46ab6 100644
--- a/chrome/browser/metrics/metrics_service.cc
+++ b/chrome/browser/metrics/metrics_service.cc
@@ -408,11 +408,15 @@ MetricsService::MetricsService()
       user_permits_upload_(false),
       server_permits_upload_(true),
       state_(INITIALIZED),
+      pending_log_(NULL),
+      pending_log_text_(),
       current_fetch_(NULL),
+      current_log_(NULL),
       idle_since_last_transmission_(false),
       next_window_id_(0),
       ALLOW_THIS_IN_INITIALIZER_LIST(log_sender_factory_(this)),
       ALLOW_THIS_IN_INITIALIZER_LIST(state_saver_factory_(this)),
+      logged_samples_(),
       interlog_duration_(TimeDelta::FromSeconds(kInitialInterlogDuration)),
       log_event_limit_(kInitialEventLimit),
       timer_pending_(false) {
@@ -422,6 +426,14 @@ MetricsService::MetricsService()
 
 MetricsService::~MetricsService() {
   SetRecording(false);
+  if (pending_log_) {
+    delete pending_log_;
+    pending_log_ = NULL;
+  }
+  if (current_log_) {
+    delete current_log_;
+    current_log_ = NULL;
+  }
 }
 
 void MetricsService::SetUserPermitsUpload(bool enabled) {
@@ -585,13 +597,10 @@ void MetricsService::Observe(NotificationType type,
       LogKeywords(Source<TemplateURLModel>(source).ptr());
       break;
 
-    case NotificationType::OMNIBOX_OPENED_URL: {
-      MetricsLog* current_log = current_log_->AsMetricsLog();
-      DCHECK(current_log);
-      current_log->RecordOmniboxOpenedURL(
+    case NotificationType::OMNIBOX_OPENED_URL:
+      current_log_->RecordOmniboxOpenedURL(
           *Details<AutocompleteLog>(details).ptr());
       break;
-    }
 
     case NotificationType::BOOKMARK_MODEL_LOADED: {
       Profile* p = Source<Profile>(source).ptr();
@@ -839,13 +848,11 @@ void MetricsService::StartRecording() {
   }
 }
 
-void MetricsService::StopRecording(MetricsLogBase** log) {
+void MetricsService::StopRecording(MetricsLog** log) {
   if (!current_log_)
     return;
 
-  MetricsLog* current_log = current_log_->AsMetricsLog();
-  DCHECK(current_log);
-  current_log->set_hardware_class(hardware_class_);  // Adds to ongoing logs.
+  current_log_->set_hardware_class(hardware_class_);  // Adds to ongoing logs.
 
   // TODO(jar): Integrate bounds on log recording more consistently, so that we
   // can stop recording logs that are too big much sooner.
@@ -862,13 +869,13 @@ void MetricsService::StopRecording(MetricsLogBase** log) {
   // end of all log transmissions (initial log handles this separately).
   // Don't bother if we're going to discard current_log_.
   if (log) {
-    current_log->RecordIncrementalStabilityElements();
+    current_log_->RecordIncrementalStabilityElements();
     RecordCurrentHistograms();
   }
 
   current_log_->CloseLog();
   if (log)
-    *log = current_log;
+    *log = current_log_;
   else
     delete current_log_;
   current_log_ = NULL;
@@ -1134,7 +1141,7 @@ void MetricsService::PrepareInitialLog() {
   log->RecordEnvironment(plugins_, profile_dictionary_.get());
 
   // Histograms only get written to current_log_, so setup for the write.
-  MetricsLogBase* save_log = current_log_;
+  MetricsLog* save_log = current_log_;
   current_log_ = log;
   RecordCurrentHistograms();  // Into current_log_... which is really log.
   current_log_ = save_log;
@@ -1233,6 +1240,53 @@ void MetricsService::PrepareFetchWithPendingLog() {
   current_fetch_->set_upload_data(kMetricsType, compressed_log);
 }
 
+void MetricsService::DiscardPendingLog() {
+  if (pending_log_) {  // Shutdown might have deleted it!
+    delete pending_log_;
+    pending_log_ = NULL;
+  }
+  pending_log_text_.clear();
+}
+
+// This implementation is based on the Firefox MetricsService implementation.
+bool MetricsService::Bzip2Compress(const std::string& input,
+                                   std::string* output) {
+  bz_stream stream = {0};
+  // As long as our input is smaller than the bzip2 block size, we should get
+  // the best compression.  For example, if your input was 250k, using a block
+  // size of 300k or 500k should result in the same compression ratio.  Since
+  // our data should be under 100k, using the minimum block size of 100k should
+  // allocate less temporary memory, but result in the same compression ratio.
+  int result = BZ2_bzCompressInit(&stream,
+                                  1,   // 100k (min) block size
+                                  0,   // quiet
+                                  0);  // default "work factor"
+  if (result != BZ_OK) {  // out of memory?
+    return false;
+  }
+
+  output->clear();
+
+  stream.next_in = const_cast<char*>(input.data());
+  stream.avail_in = static_cast<int>(input.size());
+  // NOTE: we don't need a BZ_RUN phase since our input buffer contains
+  //       the entire input
+  do {
+    output->resize(output->size() + 1024);
+    stream.next_out = &((*output)[stream.total_out_lo32]);
+    stream.avail_out = static_cast<int>(output->size()) - stream.total_out_lo32;
+    result = BZ2_bzCompress(&stream, BZ_FINISH);
+  } while (result == BZ_FINISH_OK);
+  if (result != BZ_STREAM_END)  // unknown failure?
+    return false;
+  result = BZ2_bzCompressEnd(&stream);
+  DCHECK(result == BZ_OK);
+
+  output->resize(stream.total_out_lo32);
+
+  return true;
+}
+
 static const char* StatusToString(const URLRequestStatus& status) {
   switch (status.status()) {
     case URLRequestStatus::SUCCESS:
@@ -1822,6 +1876,51 @@ void MetricsService::RecordCurrentState(PrefService* pref) {
   RecordPluginChanges(pref);
 }
 
+void MetricsService::RecordCurrentHistograms() {
+  DCHECK(current_log_);
+
+  StatisticsRecorder::Histograms histograms;
+  StatisticsRecorder::GetHistograms(&histograms);
+  for (StatisticsRecorder::Histograms::iterator it = histograms.begin();
+       histograms.end() != it;
+       ++it) {
+         if ((*it)->flags() & Histogram::kUmaTargetedHistogramFlag)
+      // TODO(petersont): Only record historgrams if they are not precluded by
+      // the UMA response data.
+      // Bug http://code.google.com/p/chromium/issues/detail?id=2739.
+      RecordHistogram(**it);
+  }
+}
+
+void MetricsService::RecordHistogram(const Histogram& histogram) {
+  // Get up-to-date snapshot of sample stats.
+  Histogram::SampleSet snapshot;
+  histogram.SnapshotSample(&snapshot);
+
+  const std::string& histogram_name = histogram.histogram_name();
+
+  // Find the already sent stats, or create an empty set.
+  LoggedSampleMap::iterator it = logged_samples_.find(histogram_name);
+  Histogram::SampleSet* already_logged;
+  if (logged_samples_.end() == it) {
+    // Add new entry
+    already_logged = &logged_samples_[histogram.histogram_name()];
+    already_logged->Resize(histogram);  // Complete initialization.
+  } else {
+    already_logged = &(it->second);
+    // Deduct any stats we've already logged from our snapshot.
+    snapshot.Subtract(*already_logged);
+  }
+
+  // snapshot now contains only a delta to what we've already_logged.
+
+  if (snapshot.TotalCount() > 0) {
+    current_log_->RecordHistogramDelta(histogram, snapshot);
+    // Add new data into our running total.
+    already_logged->Add(snapshot);
+  }
+}
+
 static bool IsSingleThreaded() {
   static PlatformThreadId thread_id = 0;
   if (!thread_id)
diff --git a/chrome/browser/metrics/metrics_service.h b/chrome/browser/metrics/metrics_service.h
index 016640d..9292856 100644
--- a/chrome/browser/metrics/metrics_service.h
+++ b/chrome/browser/metrics/metrics_service.h
@@ -21,7 +21,6 @@
 #include "base/values.h"
 #include "chrome/browser/metrics/metrics_log.h"
 #include "chrome/common/child_process_info.h"
-#include "chrome/common/metrics_helpers.h"
 #include "chrome/common/net/url_fetcher.h"
 #include "chrome/common/notification_registrar.h"
 #include "webkit/glue/plugins/webplugininfo.h"
@@ -71,8 +70,7 @@ struct ChildProcessStats {
 };
 
 class MetricsService : public NotificationObserver,
-                       public URLFetcher::Delegate,
-                       public MetricsServiceBase {
+                       public URLFetcher::Delegate {
  public:
   MetricsService();
   virtual ~MetricsService();
@@ -152,6 +150,9 @@ class MetricsService : public NotificationObserver,
     SENDING_CURRENT_LOGS,   // Sending standard current logs as they acrue.
   };
 
+  // Maintain a map of histogram names to the sample stats we've sent.
+  typedef std::map<std::string, Histogram::SampleSet> LoggedSampleMap;
+
   class InitTask;
   class InitTaskComplete;
 
@@ -217,7 +218,7 @@ class MetricsService : public NotificationObserver,
   // Called to stop recording user experience metrics.  The caller takes
   // ownership of the resulting MetricsLog object via the log parameter,
   // or passes in NULL to indicate that the log should simply be deleted.
-  void StopRecording(MetricsLogBase** log);
+  void StopRecording(MetricsLog** log);
 
   // Deletes pending_log_ and current_log_, and pushes their text into the
   // appropriate unsent_log vectors.  Called when Chrome shuts down.
@@ -249,6 +250,10 @@ class MetricsService : public NotificationObserver,
   // TryToStartTransmission.
   bool TransmissionPermitted() const;
 
+  // Check to see if there is a log that needs to be, or is being, transmitted.
+  bool pending_log() const {
+    return pending_log_ || !pending_log_text_.empty();
+  }
   // Check to see if there are any unsent logs from previous sessions.
   bool unsent_logs() const {
     return !unsent_initial_logs_.empty() || !unsent_ongoing_logs_.empty();
@@ -265,6 +270,11 @@ class MetricsService : public NotificationObserver,
   // a compressed copy of the pending log.
   void PrepareFetchWithPendingLog();
 
+  // Discard pending_log_, and clear pending_log_text_. Called after processing
+  // of this log is complete.
+  void DiscardPendingLog();
+  // Compress the report log in input using bzip2, store the result in output.
+  bool Bzip2Compress(const std::string& input, std::string* output);
   // Implementation of URLFetcher::Delegate. Called after transmission
   // completes (either successfully or with failure).
   virtual void OnURLFetchComplete(const URLFetcher* source,
@@ -377,6 +387,13 @@ class MetricsService : public NotificationObserver,
   // collecting stats from renderers.
   void CollectRendererHistograms();
 
+  // Record complete list of histograms into the current log.
+  // Called when we close a log.
+  void RecordCurrentHistograms();
+
+  // Record a specific histogram .
+  void RecordHistogram(const Histogram& histogram);
+
   // Logs the initiation of a page load
   void LogLoadStarted();
 
@@ -424,9 +441,20 @@ class MetricsService : public NotificationObserver,
   // The list of plugins which was retrieved on the file thread.
   std::vector<WebPluginInfo> plugins_;
 
+  // A log that we are currently transmiting, or about to try to transmit.
+  MetricsLog* pending_log_;
+
+  // An alternate form of pending_log_.  We persistently save this text version
+  // into prefs if we can't transmit it.  As a result, sometimes all we have is
+  // the text version (recalled from a previous session).
+  std::string pending_log_text_;
+
   // The outstanding transmission appears as a URL Fetch operation.
   scoped_ptr<URLFetcher> current_fetch_;
 
+  // The log that we are still appending to.
+  MetricsLog* current_log_;
+
   // The URL for the metrics server.
   std::wstring server_url_;
 
@@ -468,6 +496,10 @@ class MetricsService : public NotificationObserver,
   // at creation time from the prefs.
   scoped_ptr<DictionaryValue> profile_dictionary_;
 
+  // For histograms, record what we've already logged (as a sample for each
+  // histogram) so that we can send only the delta with the next log.
+  MetricsService::LoggedSampleMap logged_samples_;
+
   // The interval between consecutive log transmissions (to avoid hogging the
   // outbound network link).  This is usually also the duration for which we
   // build up a log, but if other unsent-logs from previous sessions exist, we