Move MetricsService final log collection tasks to client.

chrome/browser/metrics/chrome_metrics_service_client.cc

Index: chrome/browser/metrics/chrome_metrics_service_client.cc
===================================================================
--- chrome/browser/metrics/chrome_metrics_service_client.cc	(revision 271878)
+++ chrome/browser/metrics/chrome_metrics_service_client.cc	(working copy)
@@ -4,15 +4,29 @@
 
 #include "chrome/browser/metrics/chrome_metrics_service_client.h"
 
+#include "base/bind.h"
 #include "base/logging.h"
+#include "base/time/time.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/google/google_util.h"
+#include "chrome/browser/memory_details.h"
 #include "chrome/browser/ui/browser_otr_state.h"
 #include "chrome/common/chrome_version_info.h"
 #include "chrome/common/crash_keys.h"
+#include "chrome/common/render_messages.h"
+#include "content/public/browser/histogram_fetcher.h"
+#include "content/public/browser/render_process_host.h"
 
+#if !defined(OS_ANDROID)
+#include "chrome/browser/service_process/service_process_control.h"
+#endif
+
 namespace {
 
+// This specifies the amount of time to wait for all renderers to send their
+// data.
+const int kMaxHistogramGatheringWaitDuration = 60000;  // 60 seconds.
+
 metrics::SystemProfileProto::Channel AsProtobufChannel(
     chrome::VersionInfo::Channel channel) {
   switch (channel) {
@@ -31,9 +45,31 @@
   return metrics::SystemProfileProto::CHANNEL_UNKNOWN;
 }
 
+// Handles asynchronous fetching of memory details.
+// Will run the provided task after finished.
+class MetricsMemoryDetails : public MemoryDetails {
+ public:
+  explicit MetricsMemoryDetails(const base::Closure& callback)
+      : callback_(callback) {}
+
+  virtual void OnDetailsAvailable() OVERRIDE {
+    base::MessageLoop::current()->PostTask(FROM_HERE, callback_);
+  }
+
+ private:
+  virtual ~MetricsMemoryDetails() {}
+
+  base::Closure callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(MetricsMemoryDetails);
+};
+
 }  // namespace
 
-ChromeMetricsServiceClient::ChromeMetricsServiceClient() {
+ChromeMetricsServiceClient::ChromeMetricsServiceClient()
+    : waiting_for_asynchronous_reporting_step_(false),

[Ilya Sherman, 2014/05/21 13:37:07]

nit: Perhaps we could give this a more descriptive name, like
waiting_for_final_log_collection_step_?

[Alexei Svitkine (slow), 2014/05/21 14:20:15]

Done.

+      num_async_histogram_fetches_in_progress_(0),
+      weak_ptr_factory_(this) {
 }
 
 ChromeMetricsServiceClient::~ChromeMetricsServiceClient() {
@@ -63,3 +99,84 @@
   // TODO(asvitkine): Move over from metrics_log.cc
   return std::string();
 }
+
+void ChromeMetricsServiceClient::OnFinalLogCollection(
+    const base::Closure& done_callback) {
+  final_log_collection_done_callback_ = done_callback;
+
+  // Begin the multi-step process of collecting memory usage histograms:
+  // First spawn a task to collect the memory details; when that task is
+  // finished, it will call OnMemoryDetailCollectionDone. That will in turn
+  // call HistogramSynchronization to collect histograms from all renderers and
+  // then call OnHistogramSynchronizationDone to continue processing.
+  DCHECK(!waiting_for_asynchronous_reporting_step_);
+  waiting_for_asynchronous_reporting_step_ = true;
+
+  base::Closure callback =
+      base::Bind(&ChromeMetricsServiceClient::OnMemoryDetailCollectionDone,
+                 weak_ptr_factory_.GetWeakPtr());
+
+  scoped_refptr<MetricsMemoryDetails> details(
+      new MetricsMemoryDetails(callback));
+  details->StartFetch(MemoryDetails::UPDATE_USER_METRICS);
+
+  // Collect WebCore cache information to put into a histogram.
+  for (content::RenderProcessHost::iterator i(
+          content::RenderProcessHost::AllHostsIterator());
+       !i.IsAtEnd(); i.Advance()) {
+    i.GetCurrentValue()->Send(new ChromeViewMsg_GetCacheResourceStats());
+  }
+}
+
+void ChromeMetricsServiceClient::OnMemoryDetailCollectionDone() {
+  // This function should only be called as the callback from an ansynchronous
+  // step.
+  DCHECK(waiting_for_asynchronous_reporting_step_);
+
+  // Create a callback_task for OnHistogramSynchronizationDone.
+  base::Closure callback = base::Bind(
+      &ChromeMetricsServiceClient::OnHistogramSynchronizationDone,
+      weak_ptr_factory_.GetWeakPtr());
+
+  base::TimeDelta timeout =
+      base::TimeDelta::FromMilliseconds(kMaxHistogramGatheringWaitDuration);
+
+  DCHECK_EQ(num_async_histogram_fetches_in_progress_, 0);
+
+#if defined(OS_ANDROID)
+  // Android has no service process.
+  num_async_histogram_fetches_in_progress_ = 1;
+#else  // OS_ANDROID
+  num_async_histogram_fetches_in_progress_ = 2;
+  // Run requests to service and content in parallel.
+  if (!ServiceProcessControl::GetInstance()->GetHistograms(callback, timeout)) {
+    // Assume |num_async_histogram_fetches_in_progress_| is not changed by
+    // |GetHistograms()|.
+    DCHECK_EQ(num_async_histogram_fetches_in_progress_, 2);
+    // Assign |num_async_histogram_fetches_in_progress_| above and decrement it
+    // here to make code work even if |GetHistograms()| fired |callback|.
+    --num_async_histogram_fetches_in_progress_;
+  }
+#endif  // OS_ANDROID
+
+  // Set up the callback to task to call after we receive histograms from all
+  // child processes. Wait time specifies how long to wait before absolutely

[Ilya Sherman, 2014/05/21 13:37:07]

nit: "Wait time" -> "timeout"

[Alexei Svitkine (slow), 2014/05/21 14:20:15]

Done.

+  // calling us back on the task.
+  content::FetchHistogramsAsynchronously(base::MessageLoop::current(), callback,
+                                         timeout);
+}
+
+void ChromeMetricsServiceClient::OnHistogramSynchronizationDone() {
+  // This function should only be called as the callback from an ansynchronous
+  // step.
+  DCHECK(waiting_for_asynchronous_reporting_step_);
+  DCHECK_GT(num_async_histogram_fetches_in_progress_, 0);
+
+  // Check if all expected requests finished.
+  if (--num_async_histogram_fetches_in_progress_ > 0)
+    return;
+
+  waiting_for_asynchronous_reporting_step_ = false;
+  final_log_collection_done_callback_.Run();
+}
+