Back r2151 since it causes ui tests failures.

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

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

5 files changed, 181 insertions, 550 deletions

diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc
index 764e733..e574ef8a 100644
--- a/chrome/browser/browser_main.cc
+++ b/chrome/browser/browser_main.cc
@@ -468,12 +468,8 @@ int BrowserMain(CommandLine &parsed_command_line, int show_command,
     }
     metrics = browser_process->metrics_service();
     DCHECK(metrics);
-    // If the user permits metrics reporting with the checkbox in the
-    // prefs, we turn on recording.
-    bool enabled = local_state->GetBoolean(prefs::kMetricsReportingEnabled);
-    metrics->SetUserPermitsUpload(enabled);
-    if (enabled)
-      metrics->Start();
+    // Start user experience metrics recording, if enabled.
+    metrics->SetRecording(local_state->GetBoolean(prefs::kMetricsIsRecording));
   }
   InstallJankometer(parsed_command_line);
 
@@ -494,7 +490,7 @@ int BrowserMain(CommandLine &parsed_command_line, int show_command,
   }
 
   if (metrics)
-    metrics->Stop();
+    metrics->SetRecording(false);  // Force persistent save.
 
   // browser_shutdown takes care of deleting browser_process, so we need to
   // release it.
diff --git a/chrome/browser/metrics_log.cc b/chrome/browser/metrics_log.cc
index 51e84d1..d72e1ae 100644
--- a/chrome/browser/metrics_log.cc
+++ b/chrome/browser/metrics_log.cc
@@ -28,6 +28,7 @@ inline const unsigned char* UnsignedChar(const char* input) {
 // static
 void MetricsLog::RegisterPrefs(PrefService* local_state) {
   local_state->RegisterListPref(prefs::kStabilityPluginStats);
+  local_state->RegisterBooleanPref(prefs::kMetricsIsRecording, true);
 }
 
 MetricsLog::MetricsLog(const std::string& client_id, int session_id)
diff --git a/chrome/browser/metrics_service.cc b/chrome/browser/metrics_service.cc
index 473c326..f01494b 100644
--- a/chrome/browser/metrics_service.cc
+++ b/chrome/browser/metrics_service.cc
@@ -327,23 +327,19 @@ void MetricsService::RegisterPrefs(PrefService* local_state) {
 }
 
 MetricsService::MetricsService()
-    : recording_active_(false),
-      reporting_active_(false),
-      user_permits_upload_(false),
-      server_permits_upload_(true),
+    : recording_(false),
+      reporting_(true),
       pending_log_(NULL),
       pending_log_text_(""),
       current_fetch_(NULL),
       current_log_(NULL),
-      idle_since_last_transmission_(false),
       state_(INITIALIZED),
       next_window_id_(0),
       log_sender_factory_(this),
       state_saver_factory_(this),
       logged_samples_(),
       interlog_duration_(TimeDelta::FromSeconds(kInitialInterlogDuration)),
-      log_event_limit_(kInitialEventLimit),
-      upload_on_(true),
+      event_limit_(kInitialEventLimit),
       timer_pending_(false) {
   DCHECK(IsSingleThreaded());
   InitializeMetricsState();
@@ -353,25 +349,10 @@ MetricsService::~MetricsService() {
   SetRecording(false);
 }
 
-void MetricsService::SetUserPermitsUpload(bool enabled) {
-  HandleIdleSinceLastTransmission(false);
-  user_permits_upload_ = enabled;
-}
-
-void MetricsService::Start() {
-  SetRecording(true);
-  SetReporting(true);
-}
-
-void MetricsService::Stop() {
-  SetReporting(false);
-  SetRecording(false);
-}
-
 void MetricsService::SetRecording(bool enabled) {
   DCHECK(IsSingleThreaded());
 
-  if (enabled == recording_active_)
+  if (enabled == recording_)
     return;
 
   if (enabled) {
@@ -385,25 +366,29 @@ void MetricsService::SetRecording(bool enabled) {
     if (state_ > INITIAL_LOG_READY && unsent_logs())
       state_ = SEND_OLD_INITIAL_LOGS;
   }
-  recording_active_ = enabled;
+  recording_ = enabled;
 }
 
-bool MetricsService::recording_active() const {
+bool MetricsService::IsRecording() const {
   DCHECK(IsSingleThreaded());
-  return recording_active_;
+  return recording_;
 }
 
-void MetricsService::SetReporting(bool enable) {
-  if (reporting_active_ != enable) {
-    reporting_active_ = enable;
-    if (reporting_active_)
+bool MetricsService::EnableReporting(bool enable) {
+  bool done = GoogleUpdateSettings::SetCollectStatsConsent(enable);
+  if (!done) {
+    bool update_pref = GoogleUpdateSettings::GetCollectStatsConsent();
+    if (enable != update_pref) {
+      DLOG(INFO) << "METRICS: Unable to set crash report status to " << enable;
+      return false;
+    }
+  }
+  if (reporting_ != enable) {
+    reporting_ = enable;
+    if (reporting_)
       StartLogTransmissionTimer();
   }
-}
-
-bool MetricsService::reporting_active() const {
-  DCHECK(IsSingleThreaded());
-  return reporting_active_;
+  return true;
 }
 
 void MetricsService::Observe(NotificationType type,
@@ -474,25 +459,7 @@ void MetricsService::Observe(NotificationType type,
       NOTREACHED();
       break;
   }
-
-  HandleIdleSinceLastTransmission(false);
-
-  if (current_log_)
-    DLOG(INFO) << "METRICS: NUMBER OF LOGS = " << current_log_->num_events();
-}
-
-void MetricsService::HandleIdleSinceLastTransmission(bool in_idle) {
-  // If there wasn't a lot of action, maybe the computer was asleep, in which
-  // case, the log transmissions should have stopped.  Here we start them up
-  // again.
-  if (in_idle) {
-    idle_since_last_transmission_ = true;
-  } else {
-    if (idle_since_last_transmission_) {
-      idle_since_last_transmission_ = false;
-      StartLogTransmissionTimer();
-    }
-  }
+  StartLogTransmissionTimer();
 }
 
 void MetricsService::RecordCleanShutdown() {
@@ -548,6 +515,9 @@ void MetricsService::InitializeMetricsState() {
   ++session_id_;
   pref->SetInteger(prefs::kMetricsSessionID, session_id_);
 
+  bool done = EnableReporting(GoogleUpdateSettings::GetCollectStatsConsent());
+  DCHECK(done);
+
   // Stability bookkeeping
   IncrementPrefValue(prefs::kStabilityLaunchCount);
 
@@ -738,7 +708,7 @@ void MetricsService::PushPendingLogsToUnsentLists() {
     if (state_ == INITIAL_LOG_READY) {
       // We may race here, and send second copy of initial log later.
       unsent_initial_logs_.push_back(pending_log_text_);
-      state_ = SEND_OLD_INITIAL_LOGS;
+      state_ = SENDING_CURRENT_LOGS;
     } else {
       PushPendingLogTextToUnsentOngoingLogs();
     }
@@ -753,11 +723,6 @@ void MetricsService::PushPendingLogsToUnsentLists() {
 }
 
 void MetricsService::PushPendingLogTextToUnsentOngoingLogs() {
-  // If UMA response told us not to upload, there's no need to save the pending
-  // log.  It wasn't supposed to be uploaded anyway.
-  if (!upload_on_)
-    return;
-
   if (pending_log_text_.length() > kUploadLogAvoidRetransmitSize) {
     UMA_HISTOGRAM_COUNTS(L"UMA.Large Accumulated Log Not Persisted",
                          static_cast<int>(pending_log_text_.length()));
@@ -770,32 +735,14 @@ void MetricsService::PushPendingLogTextToUnsentOngoingLogs() {
 // Transmission of logs methods
 
 void MetricsService::StartLogTransmissionTimer() {
-  // If we're not reporting, there's no point in starting a log transmission
-  // timer.
-  if (!reporting_active())
-    return;
-
   if (!current_log_)
     return;  // Recorder is shutdown.
-
-  // If there is already a timer running, we leave it running.
-  // If timer_pending is true because the fetch is waiting for a response,
-  // we return for now and let the response handler start the timer.
-  if (timer_pending_)
+  if (timer_pending_ || !reporting_)
     return;
-
-  // Finally, if somehow we got here and the program is still idle since the
-  // last transmission, we shouldn't wake everybody up by starting a timer.
-  if (idle_since_last_transmission_)
+  // If there is no work to do, don't set a timer yet.
+  if (!current_log_->num_events() && !pending_log() && !unsent_logs())
     return;
-
-  // Before starting the timer, set timer_pending_ to true.
   timer_pending_ = true;
-
-  // Right before the UMA transmission gets started, there's one more thing we'd
-  // like to record: the histogram of memory usage, so we spawn a task to
-  // collect the memory details and when that task is finished, we arrange for
-  // TryToStartTransmission to take over.
   MessageLoop::current()->PostDelayedTask(FROM_HERE,
       log_sender_factory_.
           NewRunnableMethod(&MetricsService::CollectMemoryDetails),
@@ -805,152 +752,75 @@ void MetricsService::StartLogTransmissionTimer() {
 void MetricsService::TryToStartTransmission() {
   DCHECK(IsSingleThreaded());
 
-  // This function should only be called via timer, so timer_pending_
-  // should be true.
-  DCHECK(timer_pending_);
-  timer_pending_ = false;
+  DCHECK(timer_pending_);  // ONLY call via timer.
 
   DCHECK(!current_fetch_.get());
+  if (current_fetch_.get())
+    return;  // Redundant defensive coding.
 
-  // If we're getting no notifications, then the log won't have much in it, and
-  // it's possible the computer is about to go to sleep, so don't upload and
-  // don't restart the transmission timer.
-  if (idle_since_last_transmission_)
-    return;
-
-  // If somehow there is a fetch in progress, we return setting timer_pending_
-  // to true and hope things work out.
-  if (current_fetch_.get()) {
-    timer_pending_ = true;
-    return;
-  }
-
-  // If uploads are forbidden by UMA response, there's no point in keeping
-  // the current_log_, and the more often we delete it, the less likely it is
-  // to expand forever.
-  if (!upload_on_ && current_log_) {
-    StopRecording(NULL);
-    StartRecording();
-  }
+  timer_pending_ = false;
 
   if (!current_log_)
     return;  // Logging was disabled.
-  if (!reporting_active())
+  if (!reporting_ )
     return;  // Don't do work if we're not going to send anything now.
 
-  MakePendingLog();
-
-  // MakePendingLog should have put something in the pending log, if it didn't,
-  // we start the timer again, return and hope things work out.
-  if (!pending_log()) {
-    StartLogTransmissionTimer();
-    return;
-  }
-
-  // If we're not supposed to upload any UMA data because the response or the
-  // user said so, cancel the upload at this point, but start the timer.
-  if (!TransmissionPermitted()) {
-    DiscardPendingLog();
-    StartLogTransmissionTimer();
-    return;
-  }
-
-  PrepareFetchWithPendingLog();
-
-  if (!current_fetch_.get()) {
-    // Compression failed, and log discarded :-/.
-    DiscardPendingLog();
-    StartLogTransmissionTimer();  // Maybe we'll do better next time
-    // TODO(jar): If compression failed, we should have created a tiny log and
-    // compressed that, so that we can signal that we're losing logs.
-    return;
-  }
-
-  DCHECK(!timer_pending_);
-
-  // The URL fetch is a like timer in that after a while we get called back
-  // so we set timer_pending_ true just as we start the url fetch.
-  timer_pending_ = true;
-  current_fetch_->Start();
-
-  HandleIdleSinceLastTransmission(true);
-}
-
-
-void MetricsService::MakePendingLog() {
-  if (pending_log())
-    return;
-
-  switch (state_) {
-    case INITIALIZED:
-    case PLUGIN_LIST_REQUESTED:  // We should be further along by now.
-      DCHECK(false);
-      return;
-
-    case PLUGIN_LIST_ARRIVED:
-      // We need to wait for the initial log to be ready before sending
-      // anything, because the server will tell us whether it wants to hear
-      // from us.
-      PrepareInitialLog();
-      DCHECK(state_ == PLUGIN_LIST_ARRIVED);
-      RecallUnsentLogs();
-      state_ = INITIAL_LOG_READY;
-      break;
+  if (!pending_log())
+    switch (state_) {
+      case INITIALIZED:  // We must be further along by now.
+        DCHECK(false);
+        return;
+
+      case PLUGIN_LIST_REQUESTED:
+        StartLogTransmissionTimer();
+        return;
+
+      case PLUGIN_LIST_ARRIVED:
+        // We need to wait for the initial log to be ready before sending
+        // anything, because the server will tell us whether it wants to hear
+        // from us.
+        PrepareInitialLog();
+        DCHECK(state_ == PLUGIN_LIST_ARRIVED);
+        RecallUnsentLogs();
+        state_ = INITIAL_LOG_READY;
+        break;
 
-    case SEND_OLD_INITIAL_LOGS:
+      case SEND_OLD_INITIAL_LOGS:
         if (!unsent_initial_logs_.empty()) {
           pending_log_text_ = unsent_initial_logs_.back();
           break;
         }
-      state_ = SENDING_OLD_LOGS;
-      // Fall through.
-
-    case SENDING_OLD_LOGS:
-      if (!unsent_ongoing_logs_.empty()) {
-        pending_log_text_ = unsent_ongoing_logs_.back();
-        break;
-      }
-      state_ = SENDING_CURRENT_LOGS;
-      // Fall through.
-
-    case SENDING_CURRENT_LOGS:
-      StopRecording(&pending_log_);
-      StartRecording();
-      break;
-
-    default:
-      DCHECK(false);
-      return;
-  }
-
-  DCHECK(pending_log());
-}
+        state_ = SENDING_OLD_LOGS;
+        // Fall through.
 
-bool MetricsService::TransmissionPermitted() const {
-  // If the user forbids uploading that's they're business, and we don't upload
-  // anything.  If the server forbids uploading, that's our business, so we take
-  // that to mean it forbids current logs, but we still send up the inital logs
-  // and any old logs.
-
-  if (!user_permits_upload_)
-    return false;
-
-  if (server_permits_upload_) {
-    return true;
-  } else {
-    switch (state_) {
-      case INITIAL_LOG_READY:
-      case SEND_OLD_INITIAL_LOGS:
       case SENDING_OLD_LOGS:
-        return true;
+        if (!unsent_ongoing_logs_.empty()) {
+          pending_log_text_ = unsent_ongoing_logs_.back();
+          break;
+        }
+        state_ = SENDING_CURRENT_LOGS;
+        // Fall through.
 
       case SENDING_CURRENT_LOGS:
+        if (!current_log_->num_events())
+          return;  // Nothing to send.
+        StopRecording(&pending_log_);
+        StartRecording();
+        break;
+
       default:
-        return false;
-    }
+        DCHECK(false);
+        return;
   }
+  DCHECK(pending_log());
+
+  PreparePendingLogForTransmission();
+  if (!current_fetch_.get())
+    return;  // Compression failed, and log discarded :-/.
 
-  return false;
+  DCHECK(!timer_pending_);
+  timer_pending_ = true;  // The URL fetch is a pseudo timer.
+  current_fetch_->Start();
 }
 
 void MetricsService::CollectMemoryDetails() {
@@ -1048,7 +918,7 @@ void MetricsService::PreparePendingLogText() {
                               original_size);
 }
 
-void MetricsService::PrepareFetchWithPendingLog() {
+void MetricsService::PreparePendingLogForTransmission() {
   DCHECK(pending_log());
   DCHECK(!current_fetch_.get());
   PreparePendingLogText();
@@ -1171,7 +1041,6 @@ void MetricsService::OnURLFetchComplete(const URLFetcher* source,
   if (response_code != 200) {
     HandleBadResponseCode();
   } else {  // Success.
-    DLOG(INFO) << "METRICS RESPONSE DATA: " << data;
     switch (state_) {
       case INITIAL_LOG_READY:
         state_ = SEND_OLD_INITIAL_LOGS;
@@ -1196,7 +1065,7 @@ void MetricsService::OnURLFetchComplete(const URLFetcher* source,
         DCHECK(false);
         break;
     }
-
+    DLOG(INFO) << "METRICS RESPONSE DATA: " << data;
     DiscardPendingLog();
     // Since we sent a log, make sure our in-memory state is recorded to disk.
     PrefService* local_state = g_browser_process->local_state();
@@ -1245,10 +1114,9 @@ void MetricsService::HandleBadResponseCode() {
 
 void MetricsService::GetSettingsFromResponseData(const std::string& data) {
   // We assume that the file is structured as a block opened by <response>
-  // and that inside response, there is a block opened by tag <chrome_config>
-  // other tags are ignored for now except the content of <chrome_config>.
-  DLOG(INFO) << "METRICS: getting settings from response data: " << data;
-
+  // and that inside response, there is a block opened by tag <config>
+  // other tags are ignored for now except the content of <config>.
+  DLOG(INFO) << data;
   int data_size = static_cast<int>(data.size());
   if (data_size < 0) {
     DLOG(INFO) << "METRICS: server response data bad size " <<
@@ -1258,173 +1126,60 @@ void MetricsService::GetSettingsFromResponseData(const std::string& data) {
   xmlDocPtr doc = xmlReadMemory(data.c_str(), data_size,
                                 "", NULL, 0);
   DCHECK(doc);
-  // If the document is malformed, we just use the settings that were there.
-  if (!doc) {
-    DLOG(INFO) << "METRICS: reading xml from server response data failed";
+  // if the document is malformed, we just use the settings that were there
+  if (!doc)
     return;
-  }
 
-  xmlNodePtr top_node = xmlDocGetRootElement(doc), chrome_config_node = NULL;
-  // Here, we find the chrome_config node by name.
+  xmlNodePtr top_node = xmlDocGetRootElement(doc), config_node = NULL;
+  // Here, we find the config node by name.
   for (xmlNodePtr p = top_node->children; p; p = p->next) {
-    if (xmlStrEqual(p->name, BAD_CAST "chrome_config")) {
-      chrome_config_node = p;
+    if (xmlStrEqual(p->name, BAD_CAST "config")) {
+      config_node = p;
       break;
     }
   }
   // If the server data is formatted wrong and there is no
   // config node where we expect, we just drop out.
-  if (chrome_config_node != NULL)
-    GetSettingsFromChromeConfigNode(chrome_config_node);
+  if (config_node != NULL)
+    GetSettingsFromConfigNode(config_node);
   xmlFreeDoc(doc);
 }
 
-void MetricsService::GetSettingsFromChromeConfigNode(
-    xmlNodePtr chrome_config_node) {
-  // Iterate through all children of the config node.
-  for (xmlNodePtr current_node = chrome_config_node->children;
-       current_node;
-       current_node = current_node->next) {
-    // If we find the upload tag, we appeal to another function
-    // GetSettingsFromUploadNode to read all the data in it.
-    if (xmlStrEqual(current_node->name, BAD_CAST "upload")) {
-      GetSettingsFromUploadNode(current_node);
+void MetricsService::GetSettingsFromConfigNode(xmlNodePtr config_node) {
+  for (xmlNodePtr current_node = config_node->children;
+      current_node;
+      current_node = current_node->next) {
+    // If the node is collectors list, we iterate through the children
+    // to get the types of collectors.
+    if (xmlStrEqual(current_node->name, BAD_CAST "collectors")) {
+      collectors_.clear();
+      // Iterate through children and get the property "type".
+      for (xmlNodePtr sub_node = current_node->children;
+          sub_node;
+          sub_node = sub_node->next) {
+        if (xmlStrEqual(sub_node->name, BAD_CAST "collector")) {
+          xmlChar* type_value = xmlGetProp(sub_node, BAD_CAST "type");
+          collectors_.insert(reinterpret_cast<char*>(type_value));
+        }
+      }
       continue;
     }
-  }
-}
-
-void MetricsService::InheretedProperties::OverwriteWhereNeeded(
-    xmlNodePtr node) {
-  xmlChar* salt_value = xmlGetProp(node, BAD_CAST "salt");
-  if (salt_value)  // If the property isn't there, xmlGetProp returns NULL.
-      salt = atoi(reinterpret_cast<char*>(salt_value));
-  // If the property isn't there, we keep the value the property had before
-
-  xmlChar* denominator_value = xmlGetProp(node, BAD_CAST "denominator");
-  if (denominator_value)
-     denominator = atoi(reinterpret_cast<char*>(denominator_value));
-}
-
-void MetricsService::GetSettingsFromUploadNode(xmlNodePtr upload_node) {
-  InheretedProperties props;
-  GetSettingsFromUploadNodeRecursive(upload_node, props, "", true);
-}
-
-void MetricsService::GetSettingsFromUploadNodeRecursive(xmlNodePtr node,
-    InheretedProperties props, std::string path_prefix, bool uploadOn) {
-  props.OverwriteWhereNeeded(node);
-
-  // The bool uploadOn is set to true if the data represented by current
-  // node should be uploaded. This gets inhereted in the tree; the children
-  // of a node that has already been rejected for upload get rejected for
-  // upload.
-  uploadOn = uploadOn && NodeProbabilityTest(node, props);
-
-  // The path is a / separated list of the node names ancestral to the current
-  // one. So, if you want to check if the current node has a certain name,
-  // compare to name.  If you want to check if it is a certan tag at a certain
-  // place in the tree, compare to the whole path.
-  std::string name = std::string(reinterpret_cast<const char*>(node->name));
-  std::string path = path_prefix + "/" + name;
-
-  if (path == "/upload") {
-    xmlChar* upload_interval_val = xmlGetProp(node, BAD_CAST "interval");
-    if (upload_interval_val) {
-      interlog_duration_ = TimeDelta::FromSeconds(
-          atoi(reinterpret_cast<char*>(upload_interval_val)));
-    }
-    // The member variable upload_on_ refers to whether anything gets uploaded
-    // the argument uploadOn refers to whether this particular node is uploaded.
-    // If the top level node <upload> is not uploaded, then nothing is.
-    upload_on_ = uploadOn;
-  }
-  if (path == "/upload/logs") {
-    xmlChar* log_event_limit_val = xmlGetProp(node, BAD_CAST "event_limit");
-    if (log_event_limit_val)
-      log_event_limit_ = atoi(reinterpret_cast<char*>(log_event_limit_val));
-  }
-  if (name == "histogram") {
-    xmlChar* type_value = xmlGetProp(node, BAD_CAST "type");
-    if (type_value) {
-      std::string type = (reinterpret_cast<char*>(type_value));
-      if (uploadOn)
-        histograms_to_upload_.insert(type);
-      else
-        histograms_to_omit_.insert(type);
+    // Search for other tags, limit and upload.  Again if the server data
+    // does not contain those tags, the settings remain unchanged.
+    if (xmlStrEqual(current_node->name, BAD_CAST "limit")) {
+      xmlChar* event_limit_value = xmlGetProp(current_node, BAD_CAST "events");
+      event_limit_ = atoi(reinterpret_cast<char*>(event_limit_value));
+      continue;
     }
-  }
-  if (name == "log") {
-    xmlChar* type_value = xmlGetProp(node, BAD_CAST "type");
-    if (type_value) {
-      std::string type = (reinterpret_cast<char*>(type_value));
-      if (uploadOn)
-        logs_to_upload_.insert(type);
-      else
-        logs_to_omit_.insert(type);
+    if (xmlStrEqual(current_node->name, BAD_CAST "upload")) {
+      xmlChar* upload_interval_val = xmlGetProp(current_node,
+          BAD_CAST "interval");
+      int upload_interval_sec =
+        atoi(reinterpret_cast<char*>(upload_interval_val));
+      interlog_duration_ = TimeDelta::FromSeconds(upload_interval_sec);
+      continue;
     }
   }
-
-  // Recursive call.  If the node is a leaf i.e. if it ends in a "/>", then it
-  // doesn't have children, so node->children is NULL, and this loop doesn't
-  // call (that's how the recursion ends).
-  for (xmlNodePtr child_node = node->children;
-      child_node;
-      child_node = child_node->next) {
-    GetSettingsFromUploadNodeRecursive(child_node, props, path, uploadOn);
-  }
-}
-
-bool MetricsService::NodeProbabilityTest(xmlNodePtr node,
-    InheretedProperties props) const {
-  // Default value of probability on any node is 1, but recall that
-  // its parents can already have been rejected for upload.
-  double probability = 1;
-
-  // If a probability is specified in the node, we use it instead.
-  xmlChar* probability_value = xmlGetProp(node, BAD_CAST "probability");
-  if (probability_value)
-      probability = atoi(reinterpret_cast<char*>(probability_value));
-
-  return ProbabilityTest(probability, props.salt, props.denominator);
-}
-
-bool MetricsService::ProbabilityTest(double probability,
-                                     int salt,
-                                     int denominator) const {
-  // Okay, first we figure out how many of the digits of the
-  // client_id_ we need in order to make a nice pseudorandomish
-  // number in the range [0,denominator).  Too many digits is
-  // fine.
-  int relevant_digits = static_cast<int>(
-    ::log10(static_cast<double>(denominator))+1.0);
-
-  // n is the length of the client_id_ string
-  size_t n = client_id_.size();
-
-  // idnumber is a positive integer generated from the client_id_.
-  // It plus salt is going to give us our pseudorandom number.
-  int idnumber = 0;
-  const char* client_id_c_str = client_id_.c_str();
-
-  // Here we hash the relevant digits of the client_id_
-  // string somehow to get a big integer idnumber (could be negative
-  // from wraparound)
-  int big = 1;
-  for (size_t j = n-1; j >= 0; --j) {
-    idnumber += static_cast<int>(client_id_c_str[j])*big;
-    big *= 10;
-  }
-
-  // Mod id number by denominator making sure to get a non-negative
-  // answer.
-  idnumber = ((idnumber%denominator)+denominator)%denominator;
-
-  // ((idnumber+salt)%denominator)/denominator is in the range [0,1]
-  // if it's less than probability we call that an affirmative coin
-  // toss.
-  return static_cast<double>((idnumber+salt)%denominator) <
-      probability*denominator;
 }
 
 void MetricsService::LogWindowChange(NotificationType type,
@@ -1692,8 +1447,6 @@ void MetricsService::RecordCurrentHistograms() {
        histograms.end() != it;
        it++) {
     if ((*it)->flags() & kUmaTargetedHistogramFlag)
-      // TODO(petersont): only record historgrams if they are not precluded
-      // by the UMA response data.
       RecordHistogram(**it);
   }
 }
@@ -1758,3 +1511,4 @@ static bool IsSingleThreaded() {
     thread_id = GetCurrentThreadId();
   return GetCurrentThreadId() == thread_id;
 }
+
diff --git a/chrome/browser/metrics_service.h b/chrome/browser/metrics_service.h
index 83d1763..bda5d1b 100644
--- a/chrome/browser/metrics_service.h
+++ b/chrome/browser/metrics_service.h
@@ -54,19 +54,26 @@ class MetricsService : public NotificationObserver,
   MetricsService();
   virtual ~MetricsService();
 
-  // Sets whether the user permits uploading.  This is meant to match what the
-  // the checkbox in the wrench menu.
-  void SetUserPermitsUpload(bool enabled);
-
-  // Start/stop the metrics recording and uploading machine.  These should be
-  // used on startup and when the user clicks the checkbox in the prefs.
-  void Start();
-  void Stop();
-
   // At startup, prefs needs to be called with a list of all the pref names and
   // types we'll be using.
   static void RegisterPrefs(PrefService* local_state);
 
+  // Sets and gets whether metrics recording is active.
+  // SetRecording(false) also forces a persistent save of logging state (if
+  // anything has been recorded, or transmitted).
+  void SetRecording(bool enabled);
+  bool IsRecording() const;
+
+  // Enable/disable transmission of accumulated logs and crash reports (dumps).
+  // Return value "true" indicates setting was definitively set as requested).
+  // Return value of "false" indicates that the enable state is effectively
+  // stuck in the other logical setting.
+  // Google Update maintains the authoritative preference in the registry, so
+  // the caller *might* not be able to actually change the setting.
+  // It is always possible to set this to at least one value, which matches the
+  // current value reported by querying Google Update.
+  bool EnableReporting(bool enable);
+
   // Implementation of NotificationObserver
   virtual void Observe(NotificationType type,
                        const NotificationSource& source,
@@ -118,57 +125,25 @@ class MetricsService : public NotificationObserver,
   class GetPluginListTask;
   class GetPluginListTaskComplete;
 
-  // Sets whether the UMA server response data permits uploading.
-
-  // Sets and gets whether metrics recording is active.
-  // SetRecording(false) also forces a persistent save of logging state (if
-  // anything has been recorded, or transmitted).
-  void SetRecording(bool enabled);
-  bool recording_active() const;
-
-  // Enable/disable transmission of accumulated logs and crash reports (dumps).
-  // Return value "true" indicates setting was definitively set as requested).
-  // Return value of "false" indicates that the enable state is effectively
-  // stuck in the other logical setting.
-  // Google Update maintains the authoritative preference in the registry, so
-  // the caller *might* not be able to actually change the setting.
-  // It is always possible to set this to at least one value, which matches the
-  // current value reported by querying Google Update.
-  void SetReporting(bool enabled);
-  bool reporting_active() const;
-
-  // If in_idle is true, sets idle_since_last_transmission to true.
-  // If in_idle is false and idle_since_last_transmission_ is true, sets
-  // idle_since_last_transmission to false and starts the timer (provided
-  // starting the timer is permitted).
-  void HandleIdleSinceLastTransmission(bool in_idle);
-
   // Set up client ID, session ID, etc.
   void InitializeMetricsState();
-
   // Generates a new client ID to use to identify self to metrics server.
   static std::string GenerateClientID();
-
   // Schedule the next save of LocalState information.  This is called
   // automatically by the task that performs each save to schedule the next one.
   void ScheduleNextStateSave();
-
   // Save the LocalState information immediately. This should not be called by
   // anybody other than the scheduler to avoid doing too many writes. When you
   // make a change, call ScheduleNextStateSave() instead.
   void SaveLocalState();
 
   // Called to start recording user experience metrics.
-  // Constructs a new, empty current_log_.
   void StartRecording();
-
   // 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(MetricsLog** log);
-
   void ListenerRegistration(bool start_listening);
-
   // Adds or Removes (depending on the value of is_add) the given observer
   // to the given notification type for all sources.
   static void AddOrRemoveObserver(NotificationObserver* observer,
@@ -176,32 +151,20 @@ class MetricsService : public NotificationObserver,
                                   bool is_add);
 
   // Deletes pending_log_ and current_log_, and pushes their text into the
-  // appropriate unsent_log vectors.  Called when Chrome shuts down.
+  // appropriate unsent_log vectors.
   void PushPendingLogsToUnsentLists();
 
   // Save the pending_log_text_ persistently in a pref for transmission when we
   // next run.  Note that IF this text is "too large," we just dicard it.
-  void PushPendingLogTextToUnsentOngoingLogs();
+  void MetricsService::PushPendingLogTextToUnsentOngoingLogs();
 
   // Start timer for next log transmission.
   void StartLogTransmissionTimer();
   // Do not call TryToStartTransmission() directly.
   // Use StartLogTransmissionTimer() to schedule a call.
   void TryToStartTransmission();
-
-  // Takes whatever log should be uploaded next (according to the state_)
-  // and makes it the pending log.  If pending_log_ is not NULL,
-  // MakePendingLog does nothing and returns.
-  void MakePendingLog();
-
-  // Determines from state_ and permissions set out by the server and by
-  // the user whether the pending_log_ should be sent or discarded.  Called by
-  // TryToStartTransmission.
-  bool TransmissionPermitted() const;
-
   // Internal function to collect process memory information.
   void CollectMemoryDetails();
-
   // 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();
@@ -216,12 +179,8 @@ class MetricsService : public NotificationObserver,
   void RecallUnsentLogs();
   // Convert pending_log_ to XML in pending_log_text_ for transmission.
   void PreparePendingLogText();
-
   // Convert pending_log_ to XML, compress it, and prepare to pass to server.
-  // Upon return, current_fetch_ should be reset with its upload data set to
-  // a compressed copy of the pending log.
-  void PrepareFetchWithPendingLog();
-
+  void PreparePendingLogForTransmission();
   // Discard pending_log_, and clear pending_log_text_. Called after processing
   // of this log is complete.
   void DiscardPendingLog();
@@ -240,50 +199,14 @@ class MetricsService : public NotificationObserver,
   // a response code not equal to 200.
   void HandleBadResponseCode();
 
-  // Class to hold all attributes that gets inhereted by children in the UMA
-  // response data xml tree.  This is to make it convenient in the
-  // recursive function that does the tree traversal to pass all such
-  // data in the recursive call.  If you want to add more such attributes,
-  // add them to this class.
-  class InheretedProperties {
-    public:
-    InheretedProperties() : salt(123123), denominator(1000000) {}
-    int salt, denominator;
-    // salt and denominator are inhereted from parent nodes, but not
-    // probability the default value of probability is 1.
-
-    // When a new node is reached it might have fields set to set
-    // the inhereted properties to something else for that node
-    // (and it's children).  Call this method to overwrite those settings.
-    void OverwriteWhereNeeded(xmlNodePtr node);
-  };
-
   // Called by OnURLFetchComplete with data as the argument
   // parses the xml returned by the server in the call to OnURLFetchComplete
   // and extracts settings for subsequent frequency and content of log posts.
   void GetSettingsFromResponseData(const std::string& data);
 
   // This is a helper function for GetSettingsFromResponseData which iterates
-  // through the xml tree at the level of the <chrome_config> node.
-  void GetSettingsFromChromeConfigNode(xmlNodePtr chrome_config_node);
-
-  // GetSettingsFromUploadNode handles iteration over the children of the
-  // <upload> child of the <chrome_config> node.  It calls the recursive
-  // function GetSettingsFromUploadNodeRecursive which does the actual
-  // tree traversal.
-  void GetSettingsFromUploadNode(xmlNodePtr upload_node);
-  void GetSettingsFromUploadNodeRecursive(xmlNodePtr node,
-      InheretedProperties props,
-      std::string path_prefix,
-      bool uploadOn);
-
-  // NodeProbabilityTest gets called at every node in the tree traversal
-  // performed by GetSettingsFromUploadNodeRecursive.  It determines from
-  // the inhereted attributes (salt, denominator) and the probability
-  // assiciated with the node whether that node and its contents should
-  // contribute to the upload.
-  bool NodeProbabilityTest(xmlNodePtr node, InheretedProperties props) const;
-  bool ProbabilityTest(double probability, int salt, int denominator) const;
+  // through the xml tree at the level of the <config> node.
+  void GetSettingsFromConfigNode(xmlNodePtr config_node);
 
   // Records a window-related notification.
   void LogWindowChange(NotificationType type,
@@ -332,10 +255,9 @@ class MetricsService : public NotificationObserver,
   // buffered plugin stability statistics.
   void RecordCurrentState(PrefService* pref);
 
-  // Record complete list of histograms into the current log.
-  // Called when we close a log.
+  // Record complete list of histograms.  Called when we close a log.
   void RecordCurrentHistograms();
-  // Record a specific histogram .
+  // Record a specific histogram.
   void RecordHistogram(const Histogram& histogram);
 
   // Logs the initiation of a page load
@@ -358,21 +280,8 @@ class MetricsService : public NotificationObserver,
   // Sets the value of the specified path in prefs and schedules a save.
   void RecordBooleanPrefValue(const wchar_t* path, bool value);
 
-  // The variables recording_active_ and reporting_active_ say whether
-  // recording and reporting are currently happening.  These should not
-  // be set directly, but by calling SetRecording and SetReporting.
-  bool recording_active_;
-  bool reporting_active_;
-
-  // The variable user_permits_upload_ is meant to coinside with the check
-  // box in options window that lets the user control whether to upload.
-  bool user_permits_upload_;
-
-  // The variable server_permits_upload_ is set true when the response
-  // data forbids uploading.  This should coinside with the "die roll"
-  // with probability in the upload tag of the response data came out
-  // affirmative.
-  bool server_permits_upload_;
+  bool recording_;
+  bool reporting_;  // if false, metrics logs are discarded rather than sent
 
   // The progession of states made by the browser are recorded in the following
   // state.
@@ -380,25 +289,16 @@ class MetricsService : public NotificationObserver,
 
   // 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 identifier that's sent to the server with the log reports.
   std::string client_id_;
-
-  // Whether the MetricsService object has received any notifications since
-  // the last time a transmission was sent.
-  bool idle_since_last_transmission_;
-
   // A number that identifies the how many times the app has been launched.
   int session_id_;
 
@@ -440,20 +340,16 @@ class MetricsService : public NotificationObserver,
   // quickly transmit those unsent logs while we continue to build a log.
   TimeDelta interlog_duration_;
 
-  // The maximum number of events which get transmitted in a log.  This defaults
-  // to a constant and otherwise is provided by the UMA server in the server
-  // response data.
-  int log_event_limit_;
-
-  // Whether or not any uma data gets uploaded.
-  bool upload_on_;
+  // The maximum number of events which get transmitted in the log.  This is
+  // provided by the UMA server in the server response data.
+  int event_limit_;
 
-  // The types of data that are to be included in the logs and histograms
-  // according to the UMA response data.
-  std::set<std::string> logs_to_upload_, logs_to_omit_;
-  std::set<std::string> histograms_to_upload_, histograms_to_omit_;
+  // The types of data that are to be included in the log.  These are called
+  // "collectors" in the server response data.
+  std::set<std::string> collectors_;
 
-  // Indicate that a timer for sending the next log has already been queued.
+  // Indicate that a timer for sending the next log has already been queued,
+  // or that a URLFetch (i.e., log transmission) is in progress.
   bool timer_pending_;
 
   DISALLOW_EVIL_CONSTRUCTORS(MetricsService);
diff --git a/chrome/browser/views/options/advanced_contents_view.cc b/chrome/browser/views/options/advanced_contents_view.cc
index ccfb98a..1bb4343 100644
--- a/chrome/browser/views/options/advanced_contents_view.cc
+++ b/chrome/browser/views/options/advanced_contents_view.cc
@@ -34,7 +34,6 @@
 #include "chrome/common/pref_member.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/resource_bundle.h"
-#include "chrome/installer/util/google_update_settings.h"
 #include "chrome/views/background.h"
 #include "chrome/views/checkbox.h"
 #include "chrome/views/combo_box.h"
@@ -324,8 +323,6 @@ class GeneralSection : public AdvancedSection,
   StringPrefMember auto_open_files_;
   BooleanPrefMember enable_metrics_recording_;
 
-  void ResolveMetricsReportingEnabled();
-
   DISALLOW_EVIL_CONSTRUCTORS(GeneralSection);
 };
 
@@ -345,15 +342,24 @@ void GeneralSection::ButtonPressed(ChromeViews::NativeButton* sender) {
                             profile()->GetPrefs());
   } else if (sender == reporting_enabled_checkbox_) {
     bool enabled = reporting_enabled_checkbox_->IsSelected();
-    if (enabled)
-      UserMetricsRecordAction(L"Options_MetricsReportingCheckbox_Enable",
+    // Do what we can, but we might not be able to get what was asked for.
+    bool done = g_browser_process->metrics_service()->EnableReporting(enabled);
+    if (!done) {
+      enabled = !enabled;
+      done = g_browser_process->metrics_service()->EnableReporting(enabled);
+      DCHECK(done);
+      reporting_enabled_checkbox_->SetIsSelected(enabled);
+    } else {
+      if (enabled) {
+        UserMetricsRecordAction(L"Options_MetricsReportingCheckbox_Enable",
                                 profile()->GetPrefs());
-    else
-      UserMetricsRecordAction(L"Options_MetricsReportingCheckbox_Disable",
+      } else {
+        UserMetricsRecordAction(L"Options_MetricsReportingCheckbox_Disable",
                                 profile()->GetPrefs());
-    ResolveMetricsReportingEnabled();
-    if (enabled == reporting_enabled_checkbox_->IsSelected())
+      }
       RestartMessageBox::ShowMessageBox(GetRootWindow());
+    }
+    enable_metrics_recording_.SetValue(enabled);
   }
 }
 
@@ -438,37 +444,15 @@ void GeneralSection::NotifyPrefChanged(const std::wstring* pref_name) {
     reset_file_handlers_button_->SetEnabled(enabled);
   }
   if (!pref_name || *pref_name == prefs::kMetricsReportingEnabled) {
-    ResolveMetricsReportingEnabled();
-  }
-}
-
-void GeneralSection::ResolveMetricsReportingEnabled() {
-  bool enabled = reporting_enabled_checkbox_->IsSelected();
-
-  GoogleUpdateSettings::SetCollectStatsConsent(enabled);
-  bool update_pref = GoogleUpdateSettings::GetCollectStatsConsent();
-
-  if (enabled != update_pref) {
-    DLOG(INFO) <<
-        "GENERAL SECTION: Unable to set crash report status to " <<
-        enabled;
-  }
-
-  // Only change the pref if GoogleUpdateSettings::GetCollectStatsConsent
-  // succeeds.
-  enabled = update_pref;
-
-  MetricsService* metrics = g_browser_process->metrics_service();
-  DCHECK(metrics);
-  if (metrics) {
-    metrics->SetUserPermitsUpload(enabled);
-    if (enabled)
-      metrics->Start();
-    else
-      metrics->Stop();
+    bool enabled = enable_metrics_recording_.GetValue();
+    bool done = g_browser_process->metrics_service()->EnableReporting(enabled);
+    if (!done) {
+      enabled = !enabled;
+      done = g_browser_process->metrics_service()->EnableReporting(enabled);
+      DCHECK(done);
+    }
+    reporting_enabled_checkbox_->SetIsSelected(enabled);
   }
-
-  reporting_enabled_checkbox_->SetIsSelected(enabled);
 }
 
 ////////////////////////////////////////////////////////////////////////////////