Don't gate UKM recording on UMA reporting.

Also make sure Translate's UMA reporting state is up to date. Because
profiles are often loaded after metrics reporting is enabled during
startup, the translate ranker (which is tied to the user profile) used
not to get notified that reporting is enabled.

Bug: 1087519
Change-Id: I205691d780e9f6390f8f210caf4540a5d2ca878b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2250222Reviewed-by: Megan Jablonski <megjablon@chromium.org>
Commit-Queue: Simon Pelchat <spelchat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#779496}

chrome/browser/translate/translate_ranker_metrics_provider.cc

@@ -13,6 +13,15 @@
 
 namespace translate {
 
+TranslateRankerMetricsProvider::TranslateRankerMetricsProvider()
+    : logging_enabled_(false) {
+  g_browser_process->profile_manager()->AddObserver(this);
+}
+
+TranslateRankerMetricsProvider::~TranslateRankerMetricsProvider() {
+  g_browser_process->profile_manager()->RemoveObserver(this);
+}
+
 void TranslateRankerMetricsProvider::ProvideCurrentSessionData(
     metrics::ChromeUserMetricsExtension* uma_proto) {
   std::vector<Profile*> loaded_profiles =
@@ -54,4 +63,8 @@ void TranslateRankerMetricsProvider::OnRecordingDisabled() {
   UpdateLoggingState();
 }
 
+void TranslateRankerMetricsProvider::OnProfileAdded(Profile* profile) {
+  UpdateLoggingState();
+}
+
 }  // namespace translate

chrome/browser/translate/translate_ranker_metrics_provider.h

@@ -5,23 +5,27 @@
 #ifndef CHROME_BROWSER_TRANSLATE_TRANSLATE_RANKER_METRICS_PROVIDER_H_
 #define CHROME_BROWSER_TRANSLATE_TRANSLATE_RANKER_METRICS_PROVIDER_H_
 
+#include "chrome/browser/profiles/profile_manager_observer.h"
 #include "components/metrics/metrics_provider.h"
 
 namespace translate {
 
 // Provides metrics related to the translate ranker.
-class TranslateRankerMetricsProvider : public metrics::MetricsProvider {
+class TranslateRankerMetricsProvider : public metrics::MetricsProvider,
+                                       public ProfileManagerObserver {
  public:
-  TranslateRankerMetricsProvider() : logging_enabled_(false) {}
+  TranslateRankerMetricsProvider();
+  ~TranslateRankerMetricsProvider() override;
 
-  ~TranslateRankerMetricsProvider() override {}
-
-  // From metrics::MetricsProvider...
+  // From metrics::MetricsProvider.
   void ProvideCurrentSessionData(
       metrics::ChromeUserMetricsExtension* uma_proto) override;
   void OnRecordingEnabled() override;
   void OnRecordingDisabled() override;
 
+  // From ProfileManagerObserver.
+  void OnProfileAdded(Profile* profile) override;
+
  private:
   // Updates the logging state of all ranker instances.
   void UpdateLoggingState();

components/translate/core/browser/translate_ranker_impl.cc

@@ -156,7 +156,7 @@ TranslateRankerImpl::TranslateRankerImpl(const base::FilePath& model_path,
                                          const GURL& model_url,
                                          ukm::UkmRecorder* ukm_recorder)
     : ukm_recorder_(ukm_recorder),
-      is_logging_enabled_(false),
+      is_uma_logging_enabled_(false),
       is_query_enabled_(base::FeatureList::IsEnabled(kTranslateRankerQuery)),
       is_enforcement_enabled_(
           base::FeatureList::IsEnabled(kTranslateRankerEnforcement)),
@@ -216,11 +216,11 @@ GURL TranslateRankerImpl::GetModelURL() {
 }
 
 void TranslateRankerImpl::EnableLogging(bool value) {
-  if (value != is_logging_enabled_) {
+  if (value != is_uma_logging_enabled_) {
     DVLOG(3) << "Cleared translate events cache.";
     event_cache_.clear();
   }
-  is_logging_enabled_ = value;
+  is_uma_logging_enabled_ = value;
 }
 
 uint32_t TranslateRankerImpl::GetModelVersion() const {
@@ -344,11 +344,11 @@ void TranslateRankerImpl::AddTranslateEvent(
     const metrics::TranslateEventProto& event,
     ukm::SourceId ukm_source_id) {
   DCHECK(sequence_checker_.CalledOnValidSequence());
-  if (is_logging_enabled_) {
+  if (ukm_source_id != ukm::kInvalidSourceId) {
+    SendEventToUKM(event, ukm_source_id);
+  }
+  if (is_uma_logging_enabled_) {
     DVLOG(3) << "Adding translate ranker event.";
-    if (ukm_source_id != ukm::kInvalidSourceId) {
-      SendEventToUKM(event, ukm_source_id);
-    }
     event_cache_.push_back(event);
   }
 }

components/translate/core/browser/translate_ranker_impl.h

@@ -142,7 +142,7 @@ class TranslateRankerImpl : public TranslateRanker {
   std::unique_ptr<assist_ranker::RankerModel> model_;
 
   // Tracks whether or not translate event logging is enabled.
-  bool is_logging_enabled_ = true;
+  bool is_uma_logging_enabled_ = true;
 
   // Tracks whether or not translate ranker querying is enabled.
   bool is_query_enabled_ = true;

components/translate/core/browser/translate_ranker_impl_unittest.cc

@@ -387,13 +387,18 @@ TEST_F(TranslateRankerImplTest, EnableLogging) {
       GetRankerForTest(0.0f);
   std::vector<metrics::TranslateEventProto> flushed_events;
 
-  // Logging is disabled by default. No events will be cached.
+  // UMA logging is disabled by default. No events will be cached.
   ranker->RecordTranslateEvent(0, kUkmSourceId0, &translate_event1_);
   ranker->RecordTranslateEvent(1, kUkmSourceId0, &translate_event2_);
 
   ranker->FlushTranslateEvents(&flushed_events);
   EXPECT_EQ(0U, flushed_events.size());
 
+  // Make sure UKM logging still works.
+  auto entries = GetTestUkmRecorder()->GetEntriesByName(
+      ukm::builders::Translate::kEntryName);
+  EXPECT_EQ(2u, entries.size());
+
   // Once we enable logging, events will be cached.
   ranker->EnableLogging(true);
   ranker->RecordTranslateEvent(0, kUkmSourceId0, &translate_event1_);
@@ -403,6 +408,10 @@ TEST_F(TranslateRankerImplTest, EnableLogging) {
   EXPECT_EQ(2U, flushed_events.size());
   flushed_events.clear();
 
+  entries = GetTestUkmRecorder()->GetEntriesByName(
+      ukm::builders::Translate::kEntryName);
+  EXPECT_EQ(4u, entries.size());
+
   // Turning logging back off, caching is disabled once again.
   ranker->EnableLogging(false);
   ranker->RecordTranslateEvent(0, kUkmSourceId0, &translate_event1_);
@@ -411,6 +420,10 @@ TEST_F(TranslateRankerImplTest, EnableLogging) {
   // Logging is disabled, so no events should be cached.
   ranker->FlushTranslateEvents(&flushed_events);
   EXPECT_EQ(0U, flushed_events.size());
+
+  entries = GetTestUkmRecorder()->GetEntriesByName(
+      ukm::builders::Translate::kEntryName);
+  EXPECT_EQ(6u, entries.size());
 }
 
 TEST_F(TranslateRankerImplTest, EnableLoggingClearsCache) {