Auto-always/never can at most triggered twice (new translate infobar)

chrome/browser/ui/android/infobars/translate_compact_infobar.cc

 // Copyright 2017 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 "chrome/browser/ui/android/infobars/translate_compact_infobar.h"
 
 #include <stddef.h>
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_array.h"
@@ skipping 53 matching lines @@
     return;  // We're closing; don't call anything, it might access the owner.
 
   translate::TranslateInfoBarDelegate* delegate = GetDelegate();
   if (action == InfoBarAndroid::ACTION_TRANSLATE) {
     action_flags_ |= FLAG_TRANSLATE;
     delegate->Translate();
     if (!delegate->ShouldAlwaysTranslate() && ShouldAutoAlwaysTranslate()) {
       JNIEnv* env = base::android::AttachCurrentThread();
       Java_TranslateCompactInfoBar_setAutoAlwaysTranslate(env,
                                                           GetJavaInfoBar());
+
+      // Auto-always is triggered by the line above.  Need to increment the
+      // auto-always counter.
+      delegate->IncrementTranslationAutoAlwaysCount();
+      // Reset translateAcceptedCount so that auto-always could be triggered
+      // again.
+      delegate->ResetTranslationAcceptedCount();
     }
   } else if (action == InfoBarAndroid::ACTION_TRANSLATE_SHOW_ORIGINAL) {
     action_flags_ |= FLAG_REVERT;
     delegate->RevertWithoutClosingInfobar();
   } else {
     DCHECK_EQ(InfoBarAndroid::ACTION_NONE, action);
   }
 }
 
 void TranslateCompactInfoBar::SetJavaInfoBar(
@@ skipping 46 matching lines @@
       action_flags_ |= FLAG_NEVER_SITE;
       delegate->ToggleSiteBlacklist();
     }
   } else {
     DCHECK(false);
   }
 }
 
 bool TranslateCompactInfoBar::ShouldAutoAlwaysTranslate() {
   translate::TranslateInfoBarDelegate* delegate = GetDelegate();
-  return (delegate->GetTranslationAcceptedCount() == kAcceptCountThreshold);
+  return (delegate->GetTranslationAcceptedCount() == kAcceptCountThreshold &&
+          delegate->GetTranslationAutoAlwaysCount() < kMaxNumberOfAutoAlways);
 }
 
 jboolean TranslateCompactInfoBar::ShouldAutoNeverTranslate(
     JNIEnv* env,
     const base::android::JavaParamRef<jobject>& obj,
     jboolean menu_expanded) {
   // Flip menu expanded bit.
   if (menu_expanded)
     action_flags_ |= FLAG_EXPAND_MENU;
 
   if (!IsDeclinedByUser())
     return false;
 
   translate::TranslateInfoBarDelegate* delegate = GetDelegate();
   // Don't trigger if it's off the record or already blocked.
   if (delegate->is_off_the_record() ||
       !delegate->IsTranslatableLanguageByPrefs())
     return false;
 
-  return (delegate->GetTranslationDeniedCount() == kDeniedCountThreshold);
+  int auto_never_count = delegate->GetTranslationAutoNeverCount();
+
+  // Because this checking is done before deniedCount is incremented,
+  // deniedCount will be off by 1 when auto-never hasn't ever been triggered.
+  int off_by_one = auto_never_count == 0 ? 1 : 0;

[mdjones, 2017/06/09 17:43:37]

Just out of curiosity, why is this only true if auto_never_count is 0? Wouldn't
it be off by one all the time?

[Marti Wong, 2017/06/13 06:28:40]

At the beginning, DeniedCount starts at 0. And there is off-by-one.

But after auto-never is triggered once, DeniedCount starts at 1.  
(DeniedCount is incremented after auto-never is triggered.)
So there is no off-by-one by then.

I modified the comment to explain more clearly.

+
+  bool return_value = (delegate->GetTranslationDeniedCount() + off_by_one ==

[mdjones, 2017/06/09 17:43:36]

nit: "return_value" could probably be a bit more descriptive, maybe
"never_translate".

[Marti Wong, 2017/06/13 06:28:40]

Done.

+                           kDeniedCountThreshold &&
+                       auto_never_count < kMaxNumberOfAutoNever);
+  if (return_value) {
+    // Auto-never will be triggered.  Need to increment the auto-never counter.
+    delegate->IncrementTranslationAutoNeverCount();
+    // Reset translateDeniedCount so that auto-never could be triggered again.
+    delegate->ResetTranslationDeniedCount();
+  }
+  return return_value;
 }
 
 translate::TranslateInfoBarDelegate* TranslateCompactInfoBar::GetDelegate() {
   return delegate()->AsTranslateInfoBarDelegate();
 }
 
 void TranslateCompactInfoBar::OnTranslateStepChanged(
     translate::TranslateStep step,
     translate::TranslateErrors::Type error_type) {
   if (!owner())
@@ skipping 11 matching lines @@
   // Whether there is any affirmative action bit.
   return action_flags_ == FLAG_NONE;
 }
 
 // Native JNI methods ---------------------------------------------------------
 
 // static
 bool RegisterTranslateCompactInfoBar(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }