 Chromium Code Reviews
 Chromium Code Reviews Issue 2913593002:
  Implementation of translation event logging.  (Closed)
    
  
    Issue 2913593002:
  Implementation of translation event logging.  (Closed) 
  | 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 85eed92770a043ee3b1ae9246f4b6b31153f6d27..1c1a3976a8c45ef85a0600543815ea3e5ae4ffac 100644 | 
| --- a/chrome/browser/translate/chrome_translate_client.cc | 
| +++ b/chrome/browser/translate/chrome_translate_client.cc | 
| @@ -50,26 +50,29 @@ | 
| #include "url/gurl.h" | 
| namespace { | 
| +using metrics::TranslateEventProto; | 
| -metrics::TranslateEventProto::EventType BubbleResultToTranslateEvent( | 
| +TranslateEventProto::EventType BubbleResultToTranslateEvent( | 
| ShowTranslateBubbleResult result) { | 
| switch (result) { | 
| case ShowTranslateBubbleResult::BROWSER_WINDOW_NOT_VALID: | 
| - return metrics::TranslateEventProto::BROWSER_WINDOW_IS_INVALID; | 
| + return TranslateEventProto::BROWSER_WINDOW_IS_INVALID; | 
| case ShowTranslateBubbleResult::BROWSER_WINDOW_MINIMIZED: | 
| - return metrics::TranslateEventProto::BROWSER_WINDOW_IS_MINIMIZED; | 
| + return TranslateEventProto::BROWSER_WINDOW_IS_MINIMIZED; | 
| case ShowTranslateBubbleResult::BROWSER_WINDOW_NOT_ACTIVE: | 
| - return metrics::TranslateEventProto::BROWSER_WINDOW_NOT_ACTIVE; | 
| + return TranslateEventProto::BROWSER_WINDOW_NOT_ACTIVE; | 
| case ShowTranslateBubbleResult::WEB_CONTENTS_NOT_ACTIVE: | 
| - return metrics::TranslateEventProto::WEB_CONTENTS_NOT_ACTIVE; | 
| + return TranslateEventProto::WEB_CONTENTS_NOT_ACTIVE; | 
| case ShowTranslateBubbleResult::EDITABLE_FIELD_IS_ACTIVE: | 
| - return metrics::TranslateEventProto::EDITABLE_FIELD_IS_ACTIVE; | 
| + return TranslateEventProto::EDITABLE_FIELD_IS_ACTIVE; | 
| default: | 
| NOTREACHED(); | 
| return metrics::TranslateEventProto::UNKNOWN; | 
| } | 
| } | 
| +// ========== LOG LANGUAGE DETECTION EVENT ============== | 
| + | 
| std::unique_ptr<sync_pb::UserEventSpecifics> ConstructLanguageDetectionEvent( | 
| const int entry_id, | 
| const translate::LanguageDetectionDetails& details) { | 
| @@ -113,6 +116,85 @@ void LogLanguageDetectionEvent( | 
| } | 
| } | 
| +// ========== LOG TRANSLATE EVENT ============== | 
| + | 
| +bool ConstructTranslateEvent(const int entry_id, | 
| 
amoylan
2017/05/30 03:10:56
Can we add a comment atop this function? particula
 
renjieliu1
2017/05/30 03:33:24
Done.
 | 
| + const TranslateEventProto& translate_event, | 
| + sync_pb::UserEventSpecifics* const specifics) { | 
| + specifics->set_event_time_usec(base::Time::Now().ToInternalValue()); | 
| + | 
| + // TODO(renjieliu): Revisit this field when the best way to identify | 
| + // navigations is determined. | 
| + specifics->set_navigation_id(base::Time::Now().ToInternalValue()); | 
| + auto* const translation = specifics->mutable_translation(); | 
| + translation->set_from_language_code(translate_event.source_language()); | 
| + translation->set_to_language_code(translate_event.target_language()); | 
| + switch (translate_event.event_type()) { | 
| + case TranslateEventProto::UNKNOWN: | 
| + translation->set_interaction(sync_pb::Translation::UNKNOWN); | 
| + break; | 
| + case TranslateEventProto::USER_ACCEPT: | 
| + if (translate_event.has_modified_source_language() || | 
| + translate_event.has_modified_target_language()) { | 
| + // Special case, since we don't have event enum telling us it's actually | 
| + // modified by user, we do this by check whether this event has modified | 
| + // source or target language. | 
| + translation->set_from_language_code( | 
| + translate_event.modified_source_language()); | 
| + translation->set_to_language_code( | 
| + translate_event.modified_target_language()); | 
| + translation->set_interaction(sync_pb::Translation::MANUAL); | 
| + } else { | 
| + translation->set_interaction(sync_pb::Translation::ACCEPTED); | 
| + } | 
| + break; | 
| + case TranslateEventProto::USER_DECLINE: | 
| 
napper
2017/05/30 02:56:26
I think we should keep the naming consistent (i.e.
 
renjieliu1
2017/05/30 03:33:24
Done. Changed the proto as well.
 | 
| + translation->set_interaction(sync_pb::Translation::REJECTED); | 
| + break; | 
| + case TranslateEventProto::USER_IGNORE: | 
| + translation->set_interaction(sync_pb::Translation::IGNORED); | 
| + break; | 
| + case TranslateEventProto::USER_DISMISS: | 
| + translation->set_interaction(sync_pb::Translation::DISMISSED); | 
| + break; | 
| + case TranslateEventProto::USER_REVERT: | 
| + translation->set_interaction(sync_pb::Translation::TRANSLATION_REVERTED); | 
| + break; | 
| + case TranslateEventProto::AUTOMATICALLY_TRANSLATED: | 
| + translation->set_interaction(sync_pb::Translation::AUTOMATIC_TRANSLATION); | 
| + break; | 
| + default: // We don't care about other events. | 
| + return false; | 
| + } | 
| + return true; | 
| +} | 
| + | 
| +void LogTranslateEvent(const content::WebContents* const web_contents, | 
| + const metrics::TranslateEventProto& translate_event) { | 
| + auto* const profile = | 
| 
amoylan
2017/05/30 03:10:56
Do you want to DCHECK web_contents != nullptr?
Act
 
renjieliu1
2017/05/30 03:33:24
Done.
 | 
| + Profile::FromBrowserContext(web_contents->GetBrowserContext()); | 
| + | 
| + 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. | 
| + // The navigation entry can be null in situations like download or initial | 
| + // blank page. | 
| + if (entry == nullptr) | 
| + return; | 
| + | 
| + auto specifics = base::MakeUnique<sync_pb::UserEventSpecifics>(); | 
| + // We only log the event we care about. | 
| + const bool needs_logging = ConstructTranslateEvent( | 
| + entry->GetUniqueID(), translate_event, specifics.get()); | 
| + if (needs_logging) { | 
| + user_event_service->RecordUserEvent(std::move(specifics)); | 
| + } | 
| +} | 
| + | 
| } // namespace | 
| DEFINE_WEB_CONTENTS_USER_DATA_KEY(ChromeTranslateClient); | 
| @@ -217,6 +299,11 @@ void ChromeTranslateClient::GetTranslateLanguages( | 
| translate::TranslateManager::GetTargetLanguage(translate_prefs.get()); | 
| } | 
| +void ChromeTranslateClient::RecordTranslateEvent( | 
| + const TranslateEventProto& translate_event) { | 
| + LogTranslateEvent(web_contents(), translate_event); | 
| +} | 
| + | 
| // static | 
| void ChromeTranslateClient::BindContentTranslateDriver( | 
| content::RenderFrameHost* render_frame_host, |