Implement client side logging for language detection.

chrome/browser/translate/chrome_translate_client.cc

Index: chrome/browser/translate/chrome_translate_client.cc
diff --git a/chrome/browser/translate/chrome_translate_client.cc b/chrome/browser/translate/chrome_translate_client.cc
index 830d647c55473048a8813db47a967be9b8bbf6bb..07ee65a68ca95de1886c53aa683a05cdbcadb1ea 100644
--- a/chrome/browser/translate/chrome_translate_client.cc
+++ b/chrome/browser/translate/chrome_translate_client.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/translate/chrome_translate_client.h"
 
+#include <memory>
 #include <vector>
 
 #include "base/logging.h"
@@ -14,6 +15,7 @@
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/infobars/infobar_service.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/sync/user_event_service_factory.h"
 #include "chrome/browser/translate/language_model_factory.h"
 #include "chrome/browser/translate/translate_accept_languages_factory.h"
 #include "chrome/browser/translate/translate_ranker_factory.h"
@@ -29,6 +31,8 @@
 #include "chrome/grit/theme_resources.h"
 #include "components/metrics/proto/translate_event.pb.h"
 #include "components/prefs/pref_service.h"
+#include "components/sync/protocol/user_event_specifics.pb.h"
+#include "components/sync/user_events/user_event_service.h"
 #include "components/translate/core/browser/language_model.h"
 #include "components/translate/core/browser/language_state.h"
 #include "components/translate/core/browser/page_translated_details.h"
@@ -39,6 +43,7 @@
 #include "components/translate/core/browser/translate_prefs.h"
 #include "components/translate/core/common/language_detection_details.h"
 #include "components/variations/service/variations_service.h"
+#include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
@@ -65,6 +70,52 @@ metrics::TranslateEventProto::EventType BubbleResultToTranslateEvent(
   }
 }
 
+std::unique_ptr<sync_pb::UserEventSpecifics> ConstructLanguageDetectionEvent(
+    const int entry_id,
+    const translate::LanguageDetectionDetails& details) {
+  auto specifics = base::MakeUnique<sync_pb::UserEventSpecifics>();
+  specifics->set_event_time_usec(base::Time::Now().ToInternalValue());
+
+  specifics->set_navigation_id(entry_id);

[skym, 2017/05/23 22:45:42]

So, this is kind of an open question, what's the best way to identify
navigations. For the time being, you need to use the timestamp of the navigation

entry->GetTimestamp().ToInternalValue()

At the very least, the field UserEventSpecifics::navigation_id is poorly named.
I don't think you're the only one that's going to jump to this assumption.

You can read more ramblings about identifying navigations at
https://docs.google.com/document/d/1ZeMUbFCxUkAlIOzOv4r9eXhLemZH2VR8npfiyXigL...
, we haven't agreed on what is the best approach going forwards, and nothing has
been implemented to really solve things yet.

We might eventually go back to using NavigationEntry::GetUniqueID(), but we're
not there yet.

[renjieliu1, 2017/05/24 00:33:17]

thank you for the clarification!

+
+  sync_pb::LanguageDetection lang_detection;
+  auto* const lang = lang_detection.add_detected_languages();

[skym, 2017/05/23 22:45:42]

I'm not a huge fan of this auto, but I suppose this is largely stylistic.

[renjieliu1, 2017/05/24 00:33:17]

Acknowledged.

+  lang->set_language_code(details.cld_language);
+  lang->set_is_reliable(details.is_cld_reliable);
+  // Only set adopted_language when it's different from cld_language.
+  if (details.adopted_language != details.cld_language) {
+    lang_detection.set_adopted_language(details.adopted_language);

[skym, 2017/05/23 22:45:42]

I'm sorry, I should have commented about this when you made the proto, but what
do you think of renaming "adopted_language" to "adopted_language_code"? It seems
confusing given there's a concept of a sync_pb::Langauge, of which this field is
not.

[renjieliu1, 2017/05/24 00:33:17]

That's a good suggestion! Changed the proto.

+  }
+  *specifics->mutable_language_detection() = lang_detection;
+  return specifics;
+}
+
+void LogLanguageDetectionEvent(
+    const content::WebContents* const web_contents,
+    const translate::LanguageDetectionDetails& details) {
+  auto* const profile =
+      Profile::FromBrowserContext(web_contents->GetBrowserContext());
+
+  if (profile == nullptr) {

[skym, 2017/05/23 22:45:41]

Can this happen? Just convenience for tests?

[renjieliu1, 2017/05/24 00:33:17]

you're right, previously I thought if user is not logged in the profile would be
null, but apparently if user's not logged in, a default profile will be
returned.

+    return;
+  }
+
+  // Get user event service.

[skym, 2017/05/23 22:45:42]

I'd omit this comment.

[renjieliu1, 2017/05/24 00:33:17]

Done.

+  syncer::UserEventService* const user_event_service =
+      browser_sync::UserEventServiceFactory::GetForProfile(profile);
+
+  const auto* const entry =
+      web_contents->GetController().GetLastCommittedEntry();
+
+  // If entry is null, we don't record the page.
+  if (entry == nullptr) {
+    return;

[skym, 2017/05/23 22:45:42]

Early return to avoid a single expression below seems a bit drastic to me.

[renjieliu1, 2017/05/24 00:33:17]

Done.

+  }
+
+  user_event_service->RecordUserEvent(
+      ConstructLanguageDetectionEvent(entry->GetUniqueID(), details));
+}
+
 }  // namespace
 
 DEFINE_WEB_CONTENTS_USER_DATA_KEY(ChromeTranslateClient);
@@ -309,6 +360,7 @@ void ChromeTranslateClient::OnLanguageDetermined(
       content::Source<content::WebContents>(web_contents()),
       content::Details<const translate::LanguageDetectionDetails>(&details));
 
+  LogLanguageDetectionEvent(web_contents(), details);
   // Unless we have no language model (e.g., in incognito), notify the model
   // about detected language of every page visited.
   if (language_model_ && details.is_cld_reliable)