Send UMA recording state to TranslateRanker.

chrome/browser/translate/translate_ranker_metrics_provider.cc

Index: chrome/browser/translate/translate_ranker_metrics_provider.cc
diff --git a/chrome/browser/translate/translate_ranker_metrics_provider.cc b/chrome/browser/translate/translate_ranker_metrics_provider.cc
index ba98a639a5fa96c621120d1812414fecc52f5f08..58d57c6fef1d2e461053ff4e131042ecfc4b21d7 100644
--- a/chrome/browser/translate/translate_ranker_metrics_provider.cc
+++ b/chrome/browser/translate/translate_ranker_metrics_provider.cc
@@ -21,7 +21,7 @@ void TranslateRankerMetricsProvider::ProvideGeneralMetrics(
     TranslateRanker* ranker =
         TranslateRankerFactory::GetForBrowserContext(profile);
     if (!ranker)
-      return;
+      continue;
 
     std::vector<metrics::TranslateEventProto> translate_events;
     ranker->FlushTranslateEvents(&translate_events);
@@ -32,4 +32,23 @@ void TranslateRankerMetricsProvider::ProvideGeneralMetrics(
   }
 }
 
+void TranslateRankerMetricsProvider::SetLoggingState(bool logging_enabled) {
+  std::vector<Profile*> loaded_profiles =
+      g_browser_process->profile_manager()->GetLoadedProfiles();
+  for (Profile* profile : loaded_profiles) {

[Alexei Svitkine (slow), 2017/06/07 16:45:28]

So this will update the rankers for existing opened profiles. How about when a
new profile gets loaded?

I think you'd want to listen to profile creation events and update the ranking
state there.

Alternatively, it may be better to just have a global on/off switch - and then
you don't need to worry about profile lifecycles.

[hamelphi, 2017/06/08 20:27:58]

Who would own this switch? Ranker or UMA? I'd rather avoid using a global
variable in TranslateRanker if possible. If the metrics recording state
(https://cs.chromium.org/chromium/src/components/metrics_services_manager/metr...
maybe?) would be available globally, we could pass it as an argument to the
TranslateRanker constructor. However, my guess is that exposing this state
globally is probably not desirable.

How about we set the default to Disabled when we Initialize a Ranker object and
we only update the logging state on a profile once we receive OnRecordingEnabled
or ProvideGeneralMetrics. If I read the code correctly, OnRecordingEnabled is
called at startup, so we should not drop logging for profiles loaded at startup
(though I am not entirely sure if metrics is started after profiles are loaded).
We might lose logging on the first few TranslateEvents for profiles loaded
post-startup, but they should be enabled on the first call to
providegeneralmetrics. That sounds acceptable to me. What do you think?

+    TranslateRanker* ranker =
+        TranslateRankerFactory::GetForBrowserContext(profile);
+    if (ranker)
+      ranker->EnableLogging(logging_enabled);
+  }
+}
+
+void TranslateRankerMetricsProvider::OnRecordingEnabled() {
+  SetLoggingState(true);
+}
+
+void TranslateRankerMetricsProvider::OnRecordingDisabled() {
+  SetLoggingState(false);
+}
+
 }  // namespace translate