Create and integrate a metrics service client into Blimp engine.

blimp/engine/app/blimp_metrics_service_client.cc

Index: blimp/engine/app/blimp_metrics_service_client.cc
diff --git a/blimp/engine/app/blimp_metrics_service_client.cc b/blimp/engine/app/blimp_metrics_service_client.cc
new file mode 100644
index 0000000000000000000000000000000000000000..ff3674b5221f7aef90a601a3b97eb9d8a6ca4460
--- /dev/null
+++ b/blimp/engine/app/blimp_metrics_service_client.cc
@@ -0,0 +1,180 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "blimp/engine/app/blimp_metrics_service_client.h"
+
+#include "base/bind.h"
+#include "base/i18n/rtl.h"
+#include "components/metrics/call_stack_profile_metrics_provider.h"
+#include "components/metrics/gpu/gpu_metrics_provider.h"
+#include "components/metrics/metrics_service.h"
+#include "components/metrics/metrics_state_manager.h"
+#include "components/metrics/net/net_metrics_log_uploader.h"
+#include "components/metrics/profiler/profiler_metrics_provider.h"
+#include "components/metrics/ui/screen_info_metrics_provider.h"
+#include "components/metrics/url_constants.h"
+#include "components/prefs/pref_service.h"
+#include "content/public/browser/browser_thread.h"
+
+namespace blimp {
+namespace engine {
+
+base::LazyInstance<BlimpMetricsServiceClient>::Leaky g_lazy_instance_;
+
+namespace {

[Bernhard Bauer, 2016/04/25 08:21:36]

Nit: I find it more readable to add empty lines after the opening and before the
closing of a namespace (or a group thereof).

[Jess, 2016/04/26 00:01:24]

Done.

+// Standard interval between log uploads, in minutes
+const int kStandardUploadIntervalMinutes = 30;  // Thirty minutes.
+
+
+// Callbacks for metrics::MetricsStateManager::Create. Store/LoadClientInfo
+// allows Windows Chrome to back up ClientInfo. They're no-ops for Blimp.
+void StoreClientInfo(const metrics::ClientInfo& client_info) {}
+
+scoped_ptr<metrics::ClientInfo> LoadClientInfo() {
+  scoped_ptr<metrics::ClientInfo> client_info;
+  return client_info;

[Bernhard Bauer, 2016/04/25 08:21:36]

You can just return nullptr here.

[Jess, 2016/04/26 00:01:24]

Done.

+}
+}  // namespace
+
+// static
+BlimpMetricsServiceClient* BlimpMetricsServiceClient::GetInstance() {
+  return g_lazy_instance_.Pointer();
+}
+
+// static
+void BlimpMetricsServiceClient::Initialize(
+    PrefService* pref_service,
+    net::URLRequestContextGetter* request_context) {
+  DCHECK(!is_initialized_);
+  pref_service_ = pref_service;
+  request_context_ = request_context;
+
+  metrics_state_manager_ = metrics::MetricsStateManager::Create(
+      pref_service_,
+      base::Bind(&BlimpMetricsServiceClient::is_reporting_enabled,
+                 base::Unretained(this)),
+      base::Bind(&StoreClientInfo), base::Bind(&LoadClientInfo));

[Alexei Svitkine (slow), 2016/04/25 20:44:25]

Hmm, I wonder if we should just change MetricsStateManager to take a
MetricsServiceClient and remove these three callback params in favor of just the
client param? Anyway, that should be done in a separate CL. I'm okay if you just
add a TODO about it if you'd rather not do that.

[Jess, 2016/04/26 00:01:24]

A TODO or a crbug? I am thinking in terms of just Blimp or more broadly clients.

+
+  metrics_service_.reset(new ::metrics::MetricsService(
+      metrics_state_manager_.get(), this, pref_service_));
+
+  metrics_service_->RegisterMetricsProvider(
+      scoped_ptr<metrics::MetricsProvider>(new metrics::NetworkMetricsProvider(
+          content::BrowserThread::GetBlockingPool())));
+
+  metrics_service_->RegisterMetricsProvider(
+      scoped_ptr<metrics::MetricsProvider>(new metrics::GPUMetricsProvider));
+  metrics_service_->RegisterMetricsProvider(
+      scoped_ptr<metrics::MetricsProvider>(
+          new metrics::ScreenInfoMetricsProvider));
+
+  metrics_service_->RegisterMetricsProvider(
+      scoped_ptr<metrics::MetricsProvider>(
+          new metrics::ProfilerMetricsProvider()));
+
+  metrics_service_->RegisterMetricsProvider(
+      scoped_ptr<metrics::MetricsProvider>(
+          new metrics::CallStackProfileMetricsProvider));
+
+  metrics_service_->InitializeMetricsRecordingState();
+
+  is_initialized_ = true;
+
+  if (is_reporting_enabled()) {

[Bernhard Bauer, 2016/04/25 08:21:36]

Nit: Braces are optional for single-line bodies. (I don't know whether Blimp
prefers one way or the other; I would look around the code to see).

[Jess, 2016/04/26 00:01:24]

Acknowledged.

Wez, please let me know if there is a preference on engine.

+    metrics_service_->Start();
+  }
+}
+
+void BlimpMetricsServiceClient::Finalize() {
+  DCHECK(is_initialized_);
+  metrics_service_->Stop();
+}
+
+metrics::MetricsService* BlimpMetricsServiceClient::GetMetricsService() {
+  return metrics_service_.get();
+}
+
+// In Chrome, UMA and Breakpad are enabled/disabled together by the same
+// checkbox and they share the same client ID (a.k.a. GUID). SetMetricsClientId
+// and OnRecordingDisabled are intended to provide the ID to Breakpad.
+// This is not used on required by Blimp, so these are no-ops.
+void BlimpMetricsServiceClient::SetMetricsClientId(
+    const std::string& client_id) {}
+
+void BlimpMetricsServiceClient::OnRecordingDisabled() {}
+
+bool BlimpMetricsServiceClient::IsOffTheRecordSessionActive() {
+  // Blimp does not have incognito mode.
+  return false;
+}
+
+int32_t BlimpMetricsServiceClient::GetProduct() {
+  // Indicates product family, not reported platform.
+  return metrics::ChromeUserMetricsExtension::CHROME;

[Alexei Svitkine (slow), 2016/04/25 20:44:25]

Change this to BLIMP.

[Jess, 2016/04/26 00:01:24]

There is no such enum option.

I believe we discussed using OS instead of product to separate Blimp from other
Chrome products.  Please feel free to reach out to me offline if I misunderstand
the usage here.

[Alexei Svitkine (slow), 2016/04/26 15:20:38]

Ah right. I suggest mentioning this in the comment above. i.e.:

// Note: Blimp still reports Chrome as the product, but uses a different OS
string to differentiate itself from normal Chrome reports.

I would also see if we can have the Blimp os name string be provided through
this client interface, rather than what we currently have in place as an ifdef
in the component. Maybe something like GetOsNameOverride() API on the client
interface?

+}
+
+std::string BlimpMetricsServiceClient::GetApplicationLocale() {
+  return base::i18n::GetConfiguredLocale();
+}
+
+bool BlimpMetricsServiceClient::GetBrand(std::string* brand_code) {
+  // Blimp doesn't use brand codes.
+  return false;
+}
+
+metrics::SystemProfileProto::Channel BlimpMetricsServiceClient::GetChannel() {
+  // Blimp engine does not have channel info yet.
+  return metrics::SystemProfileProto::CHANNEL_UNKNOWN;
+}
+
+std::string BlimpMetricsServiceClient::GetVersionString() {
+  // TODO(jessicag): Add in a meaningful version string.
+  return "";

[Alexei Svitkine (slow), 2016/04/25 20:44:25]

std::string()

[Jess, 2016/04/26 00:01:24]

Done.

+}
+
+void BlimpMetricsServiceClient::OnLogUploadComplete() {}
+
+void BlimpMetricsServiceClient::InitializeSystemProfileMetrics(
+    const base::Closure& done_callback) {
+  done_callback.Run();
+}
+
+void BlimpMetricsServiceClient::CollectFinalMetricsForLog(
+    const base::Closure& done_callback) {
+  done_callback.Run();
+}
+
+scoped_ptr<metrics::MetricsLogUploader>
+BlimpMetricsServiceClient::CreateUploader(
+    const base::Callback<void(int)>& on_upload_complete) {
+  return scoped_ptr<metrics::MetricsLogUploader>(

[Bernhard Bauer, 2016/04/25 08:21:36]

You can use base::WrapUnique for this; it's a bit shorter.

[Jess, 2016/04/26 00:01:24]

Done.

+      new metrics::NetMetricsLogUploader(
+          // URLRequestContextGetter* that remains valid for class lifetime

[Alexei Svitkine (slow), 2016/04/25 20:44:25]

Sounds like this comment should be on the field in the class, rather than here.

[Jess, 2016/04/26 00:01:24]

Done.

+          // In place of g_browser_process->system_request_context()
+          // In engine use subclass BlimpURLRequestContextGetter.
+          // Member of g/e/common/blimp_browser_context.
+          request_context_,
+          metrics::kDefaultMetricsServerUrl,
+          metrics::kDefaultMetricsMimeType,
+          on_upload_complete));
+}
+
+base::TimeDelta BlimpMetricsServiceClient::GetStandardUploadInterval() {
+  return base::TimeDelta::FromMinutes(kStandardUploadIntervalMinutes);
+}
+
+BlimpMetricsServiceClient::BlimpMetricsServiceClient()
+    : is_initialized_(false),
+      pref_service_(nullptr),
+      request_context_(nullptr) {}
+
+BlimpMetricsServiceClient::~BlimpMetricsServiceClient() {}
+
+bool BlimpMetricsServiceClient::is_reporting_enabled() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  return true;
+}
+
+}  // namespace engine
+}  // namespace blimp