Log operator code histogram on new metric log

chrome/browser/chrome_browser_metrics_log_observer.cc

+// Copyright (c) 2014 The Chromium Authors. All rights reserved.

[bengr, 2014/05/06 17:14:39]

Remove (c)

[bolian, 2014/05/06 18:00:08]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/chrome_browser_metrics_log_observer.h"
+
+#if defined(OS_ANDROID)
+#include "base/metrics/sparse_histogram.h"
+#include "base/strings/string_number_conversions.h"
+#include "net/android/network_library.h"
+#endif
+
+ChromeBrowserMetricsLogObserver::ChromeBrowserMetricsLogObserver() {
+#if defined(OS_ANDROID)
+  MetricsServiceHelper::AddMetricsLogObserver(this);
+#endif
+}
+
+ChromeBrowserMetricsLogObserver::~ChromeBrowserMetricsLogObserver() {
+#if defined(OS_ANDROID)
+  MetricsServiceHelper::RemoveMetricsLogObserver(this);
+#endif
+}
+
+void ChromeBrowserMetricsLogObserver::OnNewMetricsLog() {

[bengr, 2014/05/06 17:14:39]

Could you avoid the #ifdefs here by subclassing a
ChromeBrowserMetricsLogObserverAndroid?

[bolian, 2014/05/06 18:00:08]

I think I only need ifdefs in this one method and I have removed them in the
constructor and destructor. With that, using
ChromeBrowserMetricsLogObserverAndroid does not seem to improve things. What do
you think?

[Lei Zhang, 2014/05/08 05:40:19]

Why do you think ChromeBrowserMetricsLogObserverAndroid does not improve things?
As is, an empty observer gets called for no reason on non-Android builds.

[bolian, 2014/05/08 06:38:24]

ok. that makes sense. Will address this in the next patchset.

+#if defined(OS_ANDROID)
+  net::NetworkChangeNotifier::ConnectionType type =
+      net::NetworkChangeNotifier::GetConnectionType();
+  unsigned mcc_mnc = 0;
+  if (type == net::NetworkChangeNotifier::CONNECTION_2G ||

[bengr, 2014/05/06 17:14:39]

Are there no other mobile network types?

[bolian, 2014/05/06 18:00:08]

Yes, the three are on the only cellular network, for now.

+      type == net::NetworkChangeNotifier::CONNECTION_3G ||
+      type == net::NetworkChangeNotifier::CONNECTION_4G) {
+    // Log zero if not perfectly converted.
+    if (!base::StringToUint(
+        net::android::GetTelephonyNetworkOperator(), &mcc_mnc)) {
+      mcc_mnc = 0;

[bengr, 2014/05/06 17:14:39]

So 0 means unknown and ethernet and wifi?

[bolian, 2014/05/06 18:00:08]

Yes. Basically, it means non-cellular network, that we don't care for purpose of
this histogram.

+    }
+  }
+  UMA_HISTOGRAM_SPARSE_SLOWLY(
+      "NCN.NetworkOperatorMCCMNC_NewMetricsLog", mcc_mnc);
+#endif
+}