This is the same change as issue 1633 ( http://codereview.chromium.org/1633 ) together with the disabling of a test called MetricsServiceTest.CloseRenderersNormally which the change makes obsolete (see bug 2522.)

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

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

6 files changed, 554 insertions, 181 deletions

diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc
index 15703a2..7023dfb 100644
--- a/chrome/browser/browser_main.cc
+++ b/chrome/browser/browser_main.cc
@@ -483,8 +483,14 @@ int BrowserMain(CommandLine &parsed_command_line, int show_command,
     }
     metrics = browser_process->metrics_service();
     DCHECK(metrics);
-    // Start user experience metrics recording, if enabled.
-    metrics->SetRecording(local_state->GetBoolean(prefs::kMetricsIsRecording));
+    // If the user permits metrics reporting with the checkbox in the
+    // prefs, we turn on recording.
+    bool enabled = local_state->GetBoolean(prefs::kMetricsReportingEnabled);
+    enabled = true;
+
+    metrics->SetUserPermitsUpload(enabled);
+    if (enabled)
+      metrics->Start();
   }
   InstallJankometer(parsed_command_line);
 
@@ -505,7 +511,7 @@ int BrowserMain(CommandLine &parsed_command_line, int show_command,
   }
 
   if (metrics)
-    metrics->SetRecording(false);  // Force persistent save.
+    metrics->Stop();
 
   // 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 bb2c2bb..0036fdc 100644
--- a/chrome/browser/metrics_log.cc
+++ b/chrome/browser/metrics_log.cc
@@ -29,7 +29,6 @@ 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 f01494b..473c326 100644
--- a/chrome/browser/metrics_service.cc
+++ b/chrome/browser/metrics_service.cc
@@ -327,19 +327,23 @@ void MetricsService::RegisterPrefs(PrefService* local_state) {
 }
 
 MetricsService::MetricsService()
-    : recording_(false),
-      reporting_(true),
+    : recording_active_(false),
+      reporting_active_(false),
+      user_permits_upload_(false),
+      server_permits_upload_(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)),
-      event_limit_(kInitialEventLimit),
+      log_event_limit_(kInitialEventLimit),
+      upload_on_(true),
       timer_pending_(false) {
   DCHECK(IsSingleThreaded());
   InitializeMetricsState();
@@ -349,10 +353,25 @@ 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_)
+  if (enabled == recording_active_)
     return;
 
   if (enabled) {
@@ -366,29 +385,25 @@ void MetricsService::SetRecording(bool enabled) {
     if (state_ > INITIAL_LOG_READY && unsent_logs())
       state_ = SEND_OLD_INITIAL_LOGS;
   }
-  recording_ = enabled;
+  recording_active_ = enabled;
 }
 
-bool MetricsService::IsRecording() const {
+bool MetricsService::recording_active() const {
   DCHECK(IsSingleThreaded());
-  return recording_;
+  return recording_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_)
+void MetricsService::SetReporting(bool enable) {
+  if (reporting_active_ != enable) {
+    reporting_active_ = enable;
+    if (reporting_active_)
       StartLogTransmissionTimer();
   }
-  return true;
+}
+
+bool MetricsService::reporting_active() const {
+  DCHECK(IsSingleThreaded());
+  return reporting_active_;
 }
 
 void MetricsService::Observe(NotificationType type,
@@ -459,7 +474,25 @@ void MetricsService::Observe(NotificationType type,
       NOTREACHED();
       break;
   }
-  StartLogTransmissionTimer();
+
+  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();
+    }
+  }
 }
 
 void MetricsService::RecordCleanShutdown() {
@@ -515,9 +548,6 @@ void MetricsService::InitializeMetricsState() {
   ++session_id_;
   pref->SetInteger(prefs::kMetricsSessionID, session_id_);
 
-  bool done = EnableReporting(GoogleUpdateSettings::GetCollectStatsConsent());
-  DCHECK(done);
-
   // Stability bookkeeping
   IncrementPrefValue(prefs::kStabilityLaunchCount);
 
@@ -708,7 +738,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_ = SENDING_CURRENT_LOGS;
+      state_ = SEND_OLD_INITIAL_LOGS;
     } else {
       PushPendingLogTextToUnsentOngoingLogs();
     }
@@ -723,6 +753,11 @@ 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()));
@@ -735,14 +770,32 @@ 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 (timer_pending_ || !reporting_)
+
+  // 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_)
     return;
-  // If there is no work to do, don't set a timer yet.
-  if (!current_log_->num_events() && !pending_log() && !unsent_logs())
+
+  // 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_)
     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),
@@ -752,75 +805,152 @@ void MetricsService::StartLogTransmissionTimer() {
 void MetricsService::TryToStartTransmission() {
   DCHECK(IsSingleThreaded());
 
-  DCHECK(timer_pending_);  // ONLY call via timer.
+  // This function should only be called via timer, so timer_pending_
+  // should be true.
+  DCHECK(timer_pending_);
+  timer_pending_ = false;
 
   DCHECK(!current_fetch_.get());
-  if (current_fetch_.get())
-    return;  // Redundant defensive coding.
 
-  timer_pending_ = false;
+  // 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();
+  }
 
   if (!current_log_)
     return;  // Logging was disabled.
-  if (!reporting_ )
+  if (!reporting_active())
     return;  // Don't do work if we're not going to send anything now.
 
-  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;
+  MakePendingLog();
 
-      case SEND_OLD_INITIAL_LOGS:
+  // 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;
+
+    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.
+      state_ = SENDING_OLD_LOGS;
+      // Fall through.
 
-      case SENDING_CURRENT_LOGS:
-        if (!current_log_->num_events())
-          return;  // Nothing to send.
-        StopRecording(&pending_log_);
-        StartRecording();
+    case SENDING_OLD_LOGS:
+      if (!unsent_ongoing_logs_.empty()) {
+        pending_log_text_ = unsent_ongoing_logs_.back();
         break;
+      }
+      state_ = SENDING_CURRENT_LOGS;
+      // Fall through.
 
-      default:
-        DCHECK(false);
-        return;
+    case SENDING_CURRENT_LOGS:
+      StopRecording(&pending_log_);
+      StartRecording();
+      break;
+
+    default:
+      DCHECK(false);
+      return;
   }
+
   DCHECK(pending_log());
+}
 
-  PreparePendingLogForTransmission();
-  if (!current_fetch_.get())
-    return;  // Compression failed, and log discarded :-/.
+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.
 
-  DCHECK(!timer_pending_);
-  timer_pending_ = true;  // The URL fetch is a pseudo timer.
-  current_fetch_->Start();
+  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;
+
+      case SENDING_CURRENT_LOGS:
+      default:
+        return false;
+    }
+  }
+
+  return false;
 }
 
 void MetricsService::CollectMemoryDetails() {
@@ -918,7 +1048,7 @@ void MetricsService::PreparePendingLogText() {
                               original_size);
 }
 
-void MetricsService::PreparePendingLogForTransmission() {
+void MetricsService::PrepareFetchWithPendingLog() {
   DCHECK(pending_log());
   DCHECK(!current_fetch_.get());
   PreparePendingLogText();
@@ -1041,6 +1171,7 @@ 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;
@@ -1065,7 +1196,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();
@@ -1114,9 +1245,10 @@ 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 <config>
-  // other tags are ignored for now except the content of <config>.
-  DLOG(INFO) << data;
+  // 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;
+
   int data_size = static_cast<int>(data.size());
   if (data_size < 0) {
     DLOG(INFO) << "METRICS: server response data bad size " <<
@@ -1126,60 +1258,173 @@ 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)
+  // 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";
     return;
+  }
 
-  xmlNodePtr top_node = xmlDocGetRootElement(doc), config_node = NULL;
-  // Here, we find the config node by name.
+  xmlNodePtr top_node = xmlDocGetRootElement(doc), chrome_config_node = NULL;
+  // Here, we find the chrome_config node by name.
   for (xmlNodePtr p = top_node->children; p; p = p->next) {
-    if (xmlStrEqual(p->name, BAD_CAST "config")) {
-      config_node = p;
+    if (xmlStrEqual(p->name, BAD_CAST "chrome_config")) {
+      chrome_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 (config_node != NULL)
-    GetSettingsFromConfigNode(config_node);
+  if (chrome_config_node != NULL)
+    GetSettingsFromChromeConfigNode(chrome_config_node);
   xmlFreeDoc(doc);
 }
 
-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));
-        }
-      }
+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);
       continue;
     }
-    // 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;
+  }
+}
+
+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)));
     }
-    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;
+    // 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);
+    }
+  }
+  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);
     }
   }
+
+  // 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,
@@ -1447,6 +1692,8 @@ 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);
   }
 }
@@ -1511,4 +1758,3 @@ 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 a4d8caf..83d1763 100644
--- a/chrome/browser/metrics_service.h
+++ b/chrome/browser/metrics_service.h
@@ -54,26 +54,19 @@ 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,
@@ -125,25 +118,57 @@ 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,
@@ -151,7 +176,7 @@ class MetricsService : public NotificationObserver,
                                   bool is_add);
 
   // Deletes pending_log_ and current_log_, and pushes their text into the
-  // appropriate unsent_log vectors.
+  // appropriate unsent_log vectors.  Called when Chrome shuts down.
   void PushPendingLogsToUnsentLists();
 
   // Save the pending_log_text_ persistently in a pref for transmission when we
@@ -163,8 +188,20 @@ class MetricsService : public NotificationObserver,
   // 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();
@@ -179,8 +216,12 @@ 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.
-  void PreparePendingLogForTransmission();
+  // Upon return, current_fetch_ should be reset with its upload data set to
+  // 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();
@@ -199,14 +240,50 @@ 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 <config> node.
-  void GetSettingsFromConfigNode(xmlNodePtr config_node);
+  // 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;
 
   // Records a window-related notification.
   void LogWindowChange(NotificationType type,
@@ -255,9 +332,10 @@ class MetricsService : public NotificationObserver,
   // buffered plugin stability statistics.
   void RecordCurrentState(PrefService* pref);
 
-  // Record complete list of histograms.  Called when we close a log.
+  // Record complete list of histograms into the current log.
+  // 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
@@ -280,8 +358,21 @@ 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);
 
-  bool recording_;
-  bool reporting_;  // if false, metrics logs are discarded rather than sent
+  // 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_;
 
   // The progession of states made by the browser are recorded in the following
   // state.
@@ -289,16 +380,25 @@ 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_;
 
@@ -340,16 +440,20 @@ 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 the log.  This is
-  // provided by the UMA server in the server response data.
-  int event_limit_;
+  // 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 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_;
+  // 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_;
 
-  // Indicate that a timer for sending the next log has already been queued,
-  // or that a URLFetch (i.e., log transmission) is in progress.
+  // Indicate that a timer for sending the next log has already been queued.
   bool timer_pending_;
 
   DISALLOW_EVIL_CONSTRUCTORS(MetricsService);
diff --git a/chrome/browser/metrics_service_uitest.cc b/chrome/browser/metrics_service_uitest.cc
index c62642d7..997de15 100644
--- a/chrome/browser/metrics_service_uitest.cc
+++ b/chrome/browser/metrics_service_uitest.cc
@@ -63,7 +63,9 @@ class MetricsServiceTest : public UITest {
   BrowserProxy* window_;
 };
 
-TEST_F(MetricsServiceTest, CloseRenderersNormally) {
+// This test expects the metrics recording to turn on regardless of whether
+// the user permits it.  Disabled for now, see bug 2522.
+TEST_F(MetricsServiceTest, DISABLED_CloseRenderersNormally) {
   OpenTabs();
   QuitBrowser();
 
diff --git a/chrome/browser/views/options/advanced_contents_view.cc b/chrome/browser/views/options/advanced_contents_view.cc
index c063e33..a4084f8 100644
--- a/chrome/browser/views/options/advanced_contents_view.cc
+++ b/chrome/browser/views/options/advanced_contents_view.cc
@@ -34,6 +34,7 @@
 #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"
@@ -323,6 +324,8 @@ class GeneralSection : public AdvancedSection,
   StringPrefMember auto_open_files_;
   BooleanPrefMember enable_metrics_recording_;
 
+  void ResolveMetricsReportingEnabled();
+
   DISALLOW_EVIL_CONSTRUCTORS(GeneralSection);
 };
 
@@ -342,24 +345,15 @@ void GeneralSection::ButtonPressed(ChromeViews::NativeButton* sender) {
                             profile()->GetPrefs());
   } else if (sender == reporting_enabled_checkbox_) {
     bool enabled = reporting_enabled_checkbox_->IsSelected();
-    // 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",
+    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);
   }
 }
 
@@ -444,15 +438,37 @@ void GeneralSection::NotifyPrefChanged(const std::wstring* pref_name) {
     reset_file_handlers_button_->SetEnabled(enabled);
   }
   if (!pref_name || *pref_name == prefs::kMetricsReportingEnabled) {
-    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);
+    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();
+  }
+
+  reporting_enabled_checkbox_->SetIsSelected(enabled);
 }
 
 ////////////////////////////////////////////////////////////////////////////////