Change the TranslateDeclined() to only count decline if the explicit_closed is set to true

components/translate/core/browser/translate_ui_delegate.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 "components/translate/core/browser/translate_ui_delegate.h"
 
 #include "base/i18n/string_compare.h"
 #include "base/metrics/histogram_macros.h"
 #include "components/translate/core/browser/language_state.h"
 #include "components/translate/core/browser/translate_client.h"
@@ skipping 204 matching lines @@
 }
 
 void TranslateUIDelegate::RevertTranslation() {
   if (translate_manager_) {
     translate_manager_->RevertTranslation();
     UMA_HISTOGRAM_BOOLEAN(kRevertTranslation, true);
   }
 }
 
 void TranslateUIDelegate::TranslationDeclined(bool explicitly_closed) {
-  if (!translate_driver_->IsOffTheRecord()) {

[groby-ooo-7-16, 2016/01/28 21:17:28]

Overall, I'd prefer it if you'd just add && explicitly_closed here - large if
blocks decrease readability, IMHO

[ftang, 2016/01/28 22:37:21]

Done.

[ftang, 2016/01/28 22:37:21]

Acknowledged.

-    const std::string& language = GetOriginalLanguageCode();
-    prefs_->ResetTranslationAcceptedCount(language);
-    prefs_->IncrementTranslationDeniedCount(language);
-    prefs_->UpdateLastDeniedTime(language);
+  if (explicitly_closed) {
+    if (!translate_driver_->IsOffTheRecord()) {
+      const std::string& language = GetOriginalLanguageCode();
+      prefs_->ResetTranslationAcceptedCount(language);
+      prefs_->IncrementTranslationDeniedCount(language);
+      prefs_->UpdateLastDeniedTime(language);
+    }
+
+    // Remember that the user declined the translation so as to prevent showing
+    // a translate UI for that page again.  (TranslateManager initiates
+    // translations when getting a LANGUAGE_DETERMINED from the page, which
+    // happens when a load stops. That could happen multiple times, including
+    // after the user already declined the translation.)
+    if (explicitly_closed && translate_manager_) {

[groby-ooo-7-16, 2016/01/28 21:17:28]

If you keep this structure, no need to check |explicitly_closed| here - see
larger if block

[ftang, 2016/01/28 22:37:21]

Done.

+      translate_manager_->GetLanguageState().set_translation_declined(true);
+      UMA_HISTOGRAM_BOOLEAN(kDeclineTranslate, true);
+    }
+  } else {
+    UMA_HISTOGRAM_BOOLEAN(kDeclineTranslateDismissUI, true);
   }
-
-  // Remember that the user declined the translation so as to prevent showing a
-  // translate UI for that page again.  (TranslateManager initiates translations
-  // when getting a LANGUAGE_DETERMINED from the page, which happens when a load
-  // stops. That could happen multiple times, including after the user already
-  // declined the translation.)
-  if (translate_manager_) {
-    translate_manager_->GetLanguageState().set_translation_declined(true);
-    UMA_HISTOGRAM_BOOLEAN(kDeclineTranslate, true);
-  }
-
-  if (!explicitly_closed)
-    UMA_HISTOGRAM_BOOLEAN(kDeclineTranslateDismissUI, true);
 }
 
 bool TranslateUIDelegate::IsLanguageBlocked() {
   return prefs_->IsBlockedLanguage(GetOriginalLanguageCode());
 }
 
 void TranslateUIDelegate::SetLanguageBlocked(bool value) {
   if (value) {
     prefs_->BlockLanguage(GetOriginalLanguageCode());
     if (translate_manager_) {
@@ skipping 44 matching lines @@
   UMA_HISTOGRAM_BOOLEAN(kAlwaysTranslateLang, true);
 }
 
 std::string TranslateUIDelegate::GetPageHost() {
   if (!translate_driver_->HasCurrentPage())
     return std::string();
   return translate_driver_->GetLastCommittedURL().HostNoBrackets();
 }
 
 }  // namespace translate