Record document mode histogram in UMA on every upload.

* Refactors the code to allow adding metrics as part of metrics provider
  interface
* Implements a function in android_metrics_provider to record a new
  histogram which records the current document mode. This is needed for
  better analysis in Hera. I am adding a new histogram because the
  semantics are a bit different from DocumentActivity.RunningMode. The
  previous histogram is used only to record eligible devices that have
  manually opted in or out. The new histogram will record for all devices
  regardless of whether they could possibly be in document mode.

BUG=418642

Review URL: https://codereview.chromium.org/658903002

Cr-Commit-Position: refs/heads/master@{#300375}

chrome/android/java/src/org/chromium/chrome/browser/util/FeatureUtilities.java

@@ -84,4 +84,14 @@ public class FeatureUtilities {
         Bundle userRestrictions = manager.getUserRestrictions();
         return !userRestrictions.getBoolean(UserManager.DISALLOW_MODIFY_ACCOUNTS, false);
     }
+
+    /**
+     * Records the current document mode state with native-side feature utilities.
+     * @param enabled Whether the document mode is enabled.
+     */
+    public static void setDocumentModeEnabled(boolean enabled) {
+        nativeSetDocumentModeEnabled(enabled);
+    }
+
+    private static native void nativeSetDocumentModeEnabled(boolean enabled);
 }

chrome/browser/android/chrome_jni_registrar.cc

@@ -18,6 +18,7 @@
 #include "chrome/browser/android/dom_distiller/feedback_reporter_android.h"
 #include "chrome/browser/android/enhanced_bookmarks/enhanced_bookmarks_bridge.h"
 #include "chrome/browser/android/favicon_helper.h"
+#include "chrome/browser/android/feature_utilities.h"
 #include "chrome/browser/android/find_in_page/find_in_page_bridge.h"
 #include "chrome/browser/android/foreign_session_helper.h"
 #include "chrome/browser/android/intent_helper.h"
@@ -144,6 +145,7 @@ static base::android::RegistrationMethod kChromeRegisteredMethods[] = {
       prerender::ExternalPrerenderHandlerAndroid::
       RegisterExternalPrerenderHandlerAndroid },
   { "FaviconHelper", FaviconHelper::RegisterFaviconHelper },
+  { "FeatureUtilities", RegisterFeatureUtilities },
   { "FeedbackReporter", dom_distiller::android::RegisterFeedbackReporter },
   { "FindInPageBridge", FindInPageBridge::RegisterFindInPageBridge },
   { "FontSizePrefsAndroid", FontSizePrefsAndroid::Register },

chrome/browser/android/feature_utilities.cc

+// Copyright 2014 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 "chrome/browser/android/feature_utilities.h"
+
+#include "jni/FeatureUtilities_jni.h"
+
+namespace {
+bool document_mode_enabled = false;
+} // namespace
+
+namespace chrome {
+namespace android {
+
+RunningModeHistogram GetDocumentModeValue() {
+  return document_mode_enabled ? RUNNING_MODE_DOCUMENT_MODE :
+      RUNNING_MODE_TABBED_MODE;
+}
+
+} // namespace android
+} // namespace chrome
+
+
+static void SetDocumentModeEnabled(JNIEnv* env,
+                                   jclass clazz,
+                                   jboolean enabled) {
+  document_mode_enabled = enabled;
+}
+
+bool RegisterFeatureUtilities(JNIEnv* env) {
+  return RegisterNativesImpl(env);
+}

chrome/browser/android/feature_utilities.h

+// Copyright 2014 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.
+
+#ifndef CHROME_BROWSER_ANDROID_FEATURE_UTILITIES_H_
+#define CHROME_BROWSER_ANDROID_FEATURE_UTILITIES_H_
+
+#include <jni.h>
+
+namespace chrome {
+namespace android {
+
+enum RunningModeHistogram {
+  RUNNING_MODE_DOCUMENT_MODE,
+  RUNNING_MODE_TABBED_MODE,
+  RUNNING_MODE_MAX
+};
+
+RunningModeHistogram GetDocumentModeValue();
+
+} // namespace android
+} // namespace chrome
+
+bool RegisterFeatureUtilities(JNIEnv* env);
+
+#endif  // CHROME_BROWSER_ANDROID_FEATURE_UTILITIES_H_

chrome/browser/metrics/android_metrics_provider.cc

@@ -9,6 +9,7 @@
 #include "base/prefs/pref_service.h"
 #include "base/prefs/scoped_user_pref_update.h"
 #include "base/values.h"
+#include "chrome/browser/android/feature_utilities.h"
 #include "chrome/common/pref_names.h"
 
 namespace {
@@ -37,6 +38,13 @@ AndroidMetricsProvider::AndroidMetricsProvider(PrefService* local_state)
 AndroidMetricsProvider::~AndroidMetricsProvider() {
 }
 
+void AndroidMetricsProvider::ProvideGeneralMetrics(
+    metrics::ChromeUserMetricsExtension* uma_proto) {
+  UMA_HISTOGRAM_ENUMERATION(
+      "DocumentActivity.Enabled",
+      chrome::android::GetDocumentModeValue(),
+      chrome::android::RUNNING_MODE_MAX);
+}
 
 void AndroidMetricsProvider::ProvideStabilityMetrics(
     metrics::SystemProfileProto* system_profile_proto) {

chrome/browser/metrics/android_metrics_provider.h

@@ -12,6 +12,10 @@
 class PrefService;
 class PrefRegistrySimple;
 
+namespace metrics {
+class ChromeUserMetricsExtension;
+}
+
 // AndroidMetricsProvider provides Android-specific stability metrics.
 class AndroidMetricsProvider : public metrics::MetricsProvider {
  public:
@@ -19,11 +23,15 @@ class AndroidMetricsProvider : public metrics::MetricsProvider {
   explicit AndroidMetricsProvider(PrefService* local_state);
   virtual ~AndroidMetricsProvider();
 
+  // metrics::MetricsProvider:
+  virtual void ProvideGeneralMetrics(
+      metrics::ChromeUserMetricsExtension* uma_proto) override;
+
   // Called when the Activity that the user interacts with is swapped out.
   // TODO(asvitkine): Expose a way for Android code to actually invoke this.
   void OnForegroundActivityChanged(ActivityTypeIds::Type type);
 
-  // metrics::MetricsDataProvider:
+  // metrics::MetricsProvider:
   virtual void ProvideStabilityMetrics(
       metrics::SystemProfileProto* system_profile_proto) override;
 

chrome/browser/metrics/chrome_metrics_service_client.cc

@@ -239,11 +239,6 @@ void ChromeMetricsServiceClient::CollectFinalMetrics(
   DCHECK(!waiting_for_collect_final_metrics_step_);
   waiting_for_collect_final_metrics_step_ = true;
 
-#if !defined(OS_CHROMEOS) && !defined(OS_IOS)
-  // Record the signin status histogram value.
-  signin_status_metrics_provider_->RecordSigninStatusHistogram();
-#endif
-
   base::Closure callback =
       base::Bind(&ChromeMetricsServiceClient::OnMemoryDetailCollectionDone,
                  weak_ptr_factory_.GetWeakPtr());
@@ -339,10 +334,9 @@ void ChromeMetricsServiceClient::Initialize() {
 #endif  // defined(OS_CHROMEOS)
 
 #if !defined(OS_CHROMEOS) && !defined(OS_IOS)
-  signin_status_metrics_provider_ =
-      SigninStatusMetricsProvider::CreateInstance();
   metrics_service_->RegisterMetricsProvider(
-      scoped_ptr<metrics::MetricsProvider>(signin_status_metrics_provider_));
+      scoped_ptr<metrics::MetricsProvider>(
+          SigninStatusMetricsProvider::CreateInstance()));
 #endif
 }
 

chrome/browser/metrics/chrome_metrics_service_client.h

@@ -168,12 +168,6 @@ class ChromeMetricsServiceClient
   GoogleUpdateMetricsProviderWin* google_update_metrics_provider_;
 #endif
 
-#if !defined(OS_CHROMEOS) && !defined(OS_IOS)
-  // The SigninStatusMetricsProvider instance that was registered with
-  // MetricsService. Has the same lifetime as |metrics_service_|.
-  SigninStatusMetricsProvider* signin_status_metrics_provider_;
-#endif
-
   // Callback that is called when initial metrics gathering is complete.
   base::Closure finished_gathering_initial_metrics_callback_;
 

chrome/browser/metrics/signin_status_metrics_provider.cc

@@ -71,7 +71,8 @@ SigninStatusMetricsProvider::~SigninStatusMetricsProvider() {
     factory->RemoveObserver(this);
 }
 
-void SigninStatusMetricsProvider::RecordSigninStatusHistogram() {
+void SigninStatusMetricsProvider::ProvideGeneralMetrics(
+    metrics::ChromeUserMetricsExtension* uma_proto) {
   UMA_HISTOGRAM_ENUMERATION(
       "UMA.ProfileSignInStatus", signin_status_, SIGNIN_STATUS_MAX);
   // After a histogram value is recorded, a new UMA session will be started, so
@@ -201,7 +202,7 @@ void SigninStatusMetricsProvider::UpdateStatusWhenBrowserAdded(bool signed_in) {
       (signin_status_ == ALL_PROFILES_SIGNED_IN && !signed_in)) {
     SetSigninStatus(MIXED_SIGNIN_STATUS);
   } else if (signin_status_ == UNKNOWN_SIGNIN_STATUS) {
-    // If when function RecordSigninStatusHistogram() is called, Chrome is
+    // If when function ProvideGeneralMetrics() is called, Chrome is
     // running in the background with no browser window opened, |signin_status_|
     // will be reset to |UNKNOWN_SIGNIN_STATUS|. Then this newly added browser
     // is the only opened browser/profile and its signin status represents

chrome/browser/metrics/signin_status_metrics_provider.h

@@ -16,11 +16,16 @@
 #include "components/signin/core/browser/signin_manager_base.h"
 
 class Browser;
+class ChromeUserMetricsExtension;
 
 namespace base {
 class FilePath;
 }
 
+namespace metrics {
+class ChromeUserMetricsExtension;
+}
+
 // Collect login status of all opened profiles during one UMA session and record
 // the value into a histogram before UMA log is uploaded. It's currently not
 // supported on platform chromeos, Android or iOS.
@@ -31,10 +36,9 @@ class SigninStatusMetricsProvider : public metrics::MetricsProvider,
  public:
   virtual ~SigninStatusMetricsProvider();
 
-  // Record the collected sign-in status into a histogram and re-check current
-  // sign-in status to get prepared for the next UMA session. Called by
-  // MetricsServiceClient when it is collecting final metrics.
-  void RecordSigninStatusHistogram();
+  // metrics::MetricsProvider:
+  void ProvideGeneralMetrics(
+      metrics::ChromeUserMetricsExtension* uma_proto) override;
 
   // Factory method, creates a new instance of this class.
   static SigninStatusMetricsProvider* CreateInstance();

chrome/chrome_browser.gypi

@@ -51,6 +51,8 @@
       'browser/android/enhanced_bookmarks/enhanced_bookmarks_bridge.h',
       'browser/android/favicon_helper.cc',
       'browser/android/favicon_helper.h',
+      'browser/android/feature_utilities.cc',
+      'browser/android/feature_utilities.h',
       'browser/android/find_in_page/find_in_page_bridge.cc',
       'browser/android/find_in_page/find_in_page_bridge.h',
       'browser/android/foreign_session_helper.cc',
@@ -2762,6 +2764,7 @@
       'android/java/src/org/chromium/chrome/browser/UmaBridge.java',
       'android/java/src/org/chromium/chrome/browser/UmaUtils.java',
       'android/java/src/org/chromium/chrome/browser/UrlUtilities.java',
+      'android/java/src/org/chromium/chrome/browser/util/FeatureUtilities.java',
       'android/java/src/org/chromium/chrome/browser/VoiceSearchTabHelper.java',
       'android/java/src/org/chromium/chrome/browser/WebsiteSettingsPopup.java',
       'android/java/src/org/chromium/chrome/browser/WebsiteSettingsPopupLegacy.java',

components/metrics/metrics_provider.h

@@ -48,7 +48,8 @@ class MetricsProvider {
   virtual void ClearSavedStabilityMetrics();
 
   // Provides general metrics that are neither system profile nor stability
-  // metrics.
+  // metrics. May also be used to add histograms when final metrics are
+  // collected right before upload.
   virtual void ProvideGeneralMetrics(
       ChromeUserMetricsExtension* uma_proto);
 

components/metrics/metrics_service.cc

@@ -778,8 +778,8 @@ void MetricsService::CloseCurrentLog() {
   current_log->RecordStabilityMetrics(metrics_providers_.get(),
                                       incremental_uptime, uptime);
 
-  RecordCurrentHistograms();
   current_log->RecordGeneralMetrics(metrics_providers_.get());
+  RecordCurrentHistograms();
 
   log_manager_.FinishCurrentLog();
 }
@@ -1005,9 +1005,8 @@ void MetricsService::PrepareInitialMetricsLog() {
   MetricsLog* current_log = log_manager_.current_log();
   current_log->RecordStabilityMetrics(metrics_providers_.get(),
                                       base::TimeDelta(), base::TimeDelta());
-  RecordCurrentHistograms();
-
   current_log->RecordGeneralMetrics(metrics_providers_.get());
+  RecordCurrentHistograms();
 
   log_manager_.FinishCurrentLog();
   log_manager_.ResumePausedLog();

tools/metrics/histograms/histograms.xml

@@ -5071,6 +5071,14 @@ Therefore, the affected-histogram name has to have at least one dot in it.
   <summary>Result of DNS probes sent by the probe service.</summary>
 </histogram>
 
+<histogram name="DocumentActivity.Enabled" enum="RunningMode">
+  <owner>mariakhomenko@chromium.org</owner>
+  <summary>
+    Recorded only for Android. Records on every metrics upload whether document
+    mode is enabled.
+  </summary>
+</histogram>
+
 <histogram name="DomainBoundCerts.DBLoadedCount">
   <owner>mattm@chromium.org</owner>
   <summary>Number of certs loaded from domain bound cert database.</summary>
@@ -50958,6 +50966,11 @@ To add a new entry, add it with any value and run test to compute valid value.
   <int value="12" label="Import resource"/>
 </enum>
 
+<enum name="RunningMode" type="int">
+  <int value="0" label="Document Mode"/>
+  <int value="1" label="Tabbed Mode"/>
+</enum>
+
 <enum name="SavePasswordPromptResponseType" type="int">
   <int value="0" label="NO_RESPONSE"/>
   <int value="1" label="REMEMBER_PASSWORD"/>