Automatically adapt to faster/slower uploads of renderer histograms

This replaces the current time based approach (chrome is given N seconds
to upload all renederer histograms) with an asynch callback approach
that waits until all renderers have responded (with their updates). It
uses a fall-back timer to ensure that a hung renderer won't delay things
forever as well.

This causes faster (and complete) updates in about:histograms as well
as generally assuring complete updates during UMA gatherings.

This code was contributed by Raman Tenneti in CL 42496
http://codereview.chromium.org/42496

bug=12850

r=raman
Review URL: http://codereview.chromium.org/113473

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

6 files changed, 84 insertions, 47 deletions

diff --git a/chrome/browser/browser_about_handler.cc b/chrome/browser/browser_about_handler.cc
index 6c46f18..f699b1e 100644
--- a/chrome/browser/browser_about_handler.cc
+++ b/chrome/browser/browser_about_handler.cc
@@ -24,6 +24,7 @@
 #include "chrome/browser/net/dns_global.h"
 #include "chrome/browser/renderer_host/render_process_host.h"
 #include "chrome/browser/renderer_host/render_view_host.h"
+#include "chrome/common/histogram_synchronizer.h"
 #include "chrome/common/jstemplate_builder.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/pref_service.h"
@@ -45,6 +46,9 @@
 #include "chrome/browser/views/about_network_dialog.h"
 #endif
 
+using base::Time;
+using base::TimeDelta;
+
 namespace {
 
 // The paths used for the about pages.
@@ -137,16 +141,14 @@ std::string AboutDns() {
 }
 
 std::string AboutHistograms(const std::string& query) {
-  std::string data;
-  for (RenderProcessHost::iterator it = RenderProcessHost::begin();
-       it != RenderProcessHost::end(); ++it) {
-    it->second->Send(new ViewMsg_GetRendererHistograms());
-  }
+  TimeDelta wait_time = TimeDelta::FromMilliseconds(10000);
 
-  // TODO(raman): Delay page layout until we get respnoses
-  // back from renderers, and not have to use a fixed size delay.
-  PlatformThread::Sleep(1000);
+  HistogramSynchronizer* current_synchronizer =
+      HistogramSynchronizer::CurrentSynchronizer();
+  DCHECK(current_synchronizer != NULL);
+  current_synchronizer->FetchRendererHistogramsSynchronously(wait_time);
 
+  std::string data;
   StatisticsRecorder::WriteHTMLGraph(query, &data);
   return data;
 }
diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc
index dd0e825..19b505a 100644
--- a/chrome/browser/browser_main.cc
+++ b/chrome/browser/browser_main.cc
@@ -41,6 +41,7 @@
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
+#include "chrome/common/histogram_synchronizer.h"
 #include "chrome/common/jstemplate_builder.h"
 #include "chrome/common/main_function_params.h"
 #include "chrome/common/pref_names.h"
@@ -427,6 +428,13 @@ int BrowserMain(const MainFunctionParams& parameters) {
   // Initialize histogram statistics gathering system.
   StatisticsRecorder statistics;
 
+  // Initialize histogram synchronizer system. This is a singleton and is used
+  // for posting tasks via NewRunnableMethod. Its deleted when it goes out of
+  // scope. Even though NewRunnableMethod does AddRef and Release, the object
+  // will not be deleted after the Task is executed.
+  scoped_refptr<HistogramSynchronizer> histogram_synchronizer =
+      new HistogramSynchronizer();
+
   // Initialize the shared instance of user data manager.
   scoped_ptr<UserDataManager> user_data_manager(UserDataManager::Create());
 
@@ -565,14 +573,18 @@ int BrowserMain(const MainFunctionParams& parameters) {
   // Set up a field trial to see what disabling DNS pre-resolution does to
   // latency of network transactions.
   FieldTrial::Probability kDIVISOR = 100;
-  FieldTrial::Probability k_PROBABILITY_PER_GROUP = 10;  // 10%.
+  FieldTrial::Probability k_PROBABILITY_PER_GROUP = 10;  // 10% probability.
+  // For options we don't (currently) wish to test, we use zero probability.
+  FieldTrial::Probability k_PROBABILITY_DISABLED = 0;
   scoped_refptr<FieldTrial> dns_trial = new FieldTrial("DnsImpact", kDIVISOR);
 
   dns_trial->AppendGroup("_disabled_prefetch", k_PROBABILITY_PER_GROUP);
+  // Don't discard names (erase these lines) yet, as we may use them, and we
+  // have histogram data named for these options.
   int disabled_plus_4_connections = dns_trial->AppendGroup(
-      "_disabled_prefetch_4_connections", k_PROBABILITY_PER_GROUP);
+      "_disabled_prefetch_4_connections", k_PROBABILITY_DISABLED);
   int enabled_plus_4_connections = dns_trial->AppendGroup(
-      "_enabled_prefetch_4_connections", k_PROBABILITY_PER_GROUP);
+      "_enabled_prefetch_4_connections", k_PROBABILITY_DISABLED);
 
   scoped_ptr<chrome_browser_net::DnsPrefetcherInit> dns_prefetch_init;
   if (dns_trial->group() == FieldTrial::kNotParticipating ||
diff --git a/chrome/browser/metrics/metrics_service.cc b/chrome/browser/metrics/metrics_service.cc
index 0289dd8..c822500 100644
--- a/chrome/browser/metrics/metrics_service.cc
+++ b/chrome/browser/metrics/metrics_service.cc
@@ -181,6 +181,7 @@
 #include "chrome/browser/search_engines/template_url_model.h"
 #include "chrome/common/child_process_info.h"
 #include "chrome/common/chrome_paths.h"
+#include "chrome/common/histogram_synchronizer.h"
 #include "chrome/common/libxml_utils.h"
 #include "chrome/common/notification_service.h"
 #include "chrome/common/pref_names.h"
@@ -209,6 +210,10 @@ static const char kMetricsType[] = "application/vnd.mozilla.metrics.bz2";
 // The delay, in seconds, after startup before sending the first log message.
 static const int kInitialInterlogDuration = 60;  // one minute
 
+// This specifies the amount of time to wait for all renderers to send their
+// data.
+static const int kMaxHistogramGatheringWaitDuration = 60000;  // 60 seconds.
+
 // The default maximum number of events in a log uploaded to the UMA server.
 static const int kInitialEventLimit = 2400;
 
@@ -881,15 +886,51 @@ void MetricsService::StartLogTransmissionTimer() {
 
   // 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.
+  // collect the memory details and when that task is finished, it will call
+  // OnMemoryDetailCollectionDone, which will call HistogramSynchronization to
+  // collect histograms from all renderers and then we will call
+  // OnHistogramSynchronizationDone to continue processing.
   MessageLoop::current()->PostDelayedTask(FROM_HERE,
       log_sender_factory_.
-          NewRunnableMethod(&MetricsService::CollectMemoryDetails),
+          NewRunnableMethod(&MetricsService::LogTransmissionTimerDone),
       static_cast<int>(interlog_duration_.InMilliseconds()));
 }
 
-void MetricsService::TryToStartTransmission() {
+void MetricsService::LogTransmissionTimerDone() {
+  Task* task = log_sender_factory_.
+      NewRunnableMethod(&MetricsService::OnMemoryDetailCollectionDone);
+
+  MetricsMemoryDetails* details = new MetricsMemoryDetails(task);
+  details->StartFetch();
+
+  // Collect WebCore cache information to put into a histogram.
+  for (RenderProcessHost::iterator it = RenderProcessHost::begin();
+       it != RenderProcessHost::end(); ++it) {
+    it->second->Send(new ViewMsg_GetCacheResourceStats());
+  }
+}
+
+void MetricsService::OnMemoryDetailCollectionDone() {
+  DCHECK(IsSingleThreaded());
+
+  // HistogramSynchronizer will Collect histograms from all renderers and it
+  // will call OnHistogramSynchronizationDone (if wait time elapses before it
+  // heard from all renderers, then also it will call
+  // OnHistogramSynchronizationDone).
+
+  // Create a callback_task for OnHistogramSynchronizationDone.
+  Task* callback_task = log_sender_factory_.NewRunnableMethod(
+      &MetricsService::OnHistogramSynchronizationDone);
+
+  // Set up the callback to task to call after we receive histograms from all
+  // renderer processes. Wait time specifies how long to wait before absolutely
+  // calling us back on the task.
+  HistogramSynchronizer::FetchRendererHistogramsAsynchronously(
+      MessageLoop::current(), callback_task,
+      kMaxHistogramGatheringWaitDuration);
+}
+
+void MetricsService::OnHistogramSynchronizationDone() {
   DCHECK(IsSingleThreaded());
 
   // This function should only be called via timer, so timer_pending_
@@ -1035,19 +1076,6 @@ bool MetricsService::TransmissionPermitted() const {
   }
 }
 
-void MetricsService::CollectMemoryDetails() {
-  Task* task = log_sender_factory_.
-      NewRunnableMethod(&MetricsService::TryToStartTransmission);
-  MetricsMemoryDetails* details = new MetricsMemoryDetails(task);
-  details->StartFetch();
-
-  // Collect WebCore cache information to put into a histogram.
-  for (RenderProcessHost::iterator it = RenderProcessHost::begin();
-       it != RenderProcessHost::end(); ++it) {
-    it->second->Send(new ViewMsg_GetCacheResourceStats());
-  }
-}
-
 void MetricsService::PrepareInitialLog() {
   DCHECK(state_ == PLUGIN_LIST_ARRIVED);
   std::vector<WebPluginInfo> plugins;
@@ -1781,21 +1809,9 @@ void MetricsService::RecordCurrentState(PrefService* pref) {
   RecordPluginChanges(pref);
 }
 
-void MetricsService::CollectRendererHistograms() {
-  for (RenderProcessHost::iterator it = RenderProcessHost::begin();
-       it != RenderProcessHost::end(); ++it) {
-    it->second->Send(new ViewMsg_GetRendererHistograms());
-  }
-}
-
 void MetricsService::RecordCurrentHistograms() {
   DCHECK(current_log_);
 
-  CollectRendererHistograms();
-
-  // TODO(raman): Delay the metrics collection activities until we get all the
-  // updates from the renderers, or we time out (1 second?  3 seconds?).
-
   StatisticsRecorder::Histograms histograms;
   StatisticsRecorder::GetHistograms(&histograms);
   for (StatisticsRecorder::Histograms::iterator it = histograms.begin();
diff --git a/chrome/browser/metrics/metrics_service.h b/chrome/browser/metrics/metrics_service.h
index 199e1b1..e78a66a 100644
--- a/chrome/browser/metrics/metrics_service.h
+++ b/chrome/browser/metrics/metrics_service.h
@@ -25,6 +25,7 @@
 
 class BookmarkModel;
 class BookmarkNode;
+class HistogramSynchronizer;
 class PrefService;
 class Profile;
 class TemplateURLModel;
@@ -187,9 +188,15 @@ class MetricsService : public NotificationObserver,
 
   // Start timer for next log transmission.
   void StartLogTransmissionTimer();
-  // Do not call TryToStartTransmission() directly.
+
+  // Internal function to collect process memory information.
+  void LogTransmissionTimerDone();
+
+  // Do not call OnMemoryDetailCollectionDone() or
+  // OnHistogramSynchronizationDone() directly.
   // Use StartLogTransmissionTimer() to schedule a call.
-  void TryToStartTransmission();
+  void OnMemoryDetailCollectionDone();
+  void OnHistogramSynchronizationDone();
 
   // Takes whatever log should be uploaded next (according to the state_)
   // and makes it the pending log.  If pending_log_ is not NULL,
@@ -201,9 +208,6 @@ class MetricsService : public NotificationObserver,
   // 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();
diff --git a/chrome/browser/renderer_host/resource_message_filter.cc b/chrome/browser/renderer_host/resource_message_filter.cc
index 4749064..8cfaa61 100644
--- a/chrome/browser/renderer_host/resource_message_filter.cc
+++ b/chrome/browser/renderer_host/resource_message_filter.cc
@@ -23,6 +23,7 @@
 #include "chrome/common/app_cache/app_cache_dispatcher_host.h"
 #include "chrome/common/chrome_plugin_lib.h"
 #include "chrome/common/chrome_plugin_util.h"
+#include "chrome/common/histogram_synchronizer.h"
 #include "chrome/common/notification_service.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/pref_service.h"
@@ -832,8 +833,9 @@ void ResourceMessageFilter::OnDnsPrefetch(
 }
 
 void ResourceMessageFilter::OnRendererHistograms(
+    int sequence_number,
     const std::vector<std::string>& histograms) {
-  Histogram::DeserializeHistogramList(histograms);
+  HistogramSynchronizer::DeserializeHistogramList(sequence_number, histograms);
 }
 
 #if defined(OS_MACOSX)
diff --git a/chrome/browser/renderer_host/resource_message_filter.h b/chrome/browser/renderer_host/resource_message_filter.h
index bc156c7..9e23079 100644
--- a/chrome/browser/renderer_host/resource_message_filter.h
+++ b/chrome/browser/renderer_host/resource_message_filter.h
@@ -152,7 +152,8 @@ class ResourceMessageFilter : public IPC::ChannelProxy::MessageFilter,
   void OnGetAutoCorrectWord(const std::wstring& word,
                             IPC::Message* reply_msg);
   void OnDnsPrefetch(const std::vector<std::string>& hostnames);
-  void OnRendererHistograms(const std::vector<std::string>& histogram_info);
+  void OnRendererHistograms(int sequence_number,
+                            const std::vector<std::string>& histogram_info);
   void OnReceiveContextMenuMsg(const IPC::Message& msg);
   // Clipboard messages
   void OnClipboardWriteObjects(const Clipboard::ObjectMap& objects);