fix "Always do this" checkbox

chrome/browser/ui/views/translate/translate_bubble_view.cc

Index: chrome/browser/ui/views/translate/translate_bubble_view.cc
diff --git a/chrome/browser/ui/views/translate/translate_bubble_view.cc b/chrome/browser/ui/views/translate/translate_bubble_view.cc
index 9726e774b7812db4bce2278573591ca0e38b55ab..989842bf716417857939867bd3c84077ee559d96 100644
--- a/chrome/browser/ui/views/translate/translate_bubble_view.cc
+++ b/chrome/browser/ui/views/translate/translate_bubble_view.cc
@@ -179,6 +179,12 @@ void TranslateBubbleView::CloseBubble() {
 void TranslateBubbleView::Init() {
   SetLayoutManager(new views::BoxLayout(views::BoxLayout::kVertical, 0, 0, 0));
 
+  should_always_translate_ = model_->ShouldAlwaysTranslate();
+  if (Use2016Q2UI() && !is_in_incognito_window_) {
+    should_always_translate_ =
+        model_->ShouldAlwaysTranslateBeCheckedByDefault();
+  }
+
   before_translate_view_ = CreateViewBeforeTranslate();
   translating_view_ = CreateViewTranslating();
   after_translate_view_ = CreateViewAfterTranslate();
@@ -371,7 +377,8 @@ TranslateBubbleView::TranslateBubbleView(
       denial_combobox_(NULL),
       source_language_combobox_(NULL),
       target_language_combobox_(NULL),
-      always_translate_checkbox_(NULL),
+      before_always_translate_checkbox_(NULL),
+      advanced_always_translate_checkbox_(NULL),
       advanced_cancel_button_(NULL),
       advanced_done_button_(NULL),
       denial_menu_button_(NULL),
@@ -405,15 +412,17 @@ void TranslateBubbleView::HandleButtonPressed(
     TranslateBubbleView::ButtonID sender_id) {
   switch (sender_id) {
     case BUTTON_ID_TRANSLATE: {
-      if (always_translate_checkbox_)
-        model_->SetAlwaysTranslate(always_translate_checkbox_->checked());
+      views::Checkbox* always_checkbox = GetAlwaysTranslateCheckbox();

[groby-ooo-7-16, 2016/06/20 19:03:09]

I'd suggest just using |should_always_translate_| here, but that's probably a
personal preference.

[ftang, 2016/06/20 21:30:39]

Done.

+      if (always_checkbox)
+        model_->SetAlwaysTranslate(always_checkbox->checked());
       model_->Translate();
       translate::ReportUiAction(translate::TRANSLATE_BUTTON_CLICKED);
       break;
     }
     case BUTTON_ID_DONE: {
-      if (always_translate_checkbox_)
-        model_->SetAlwaysTranslate(always_translate_checkbox_->checked());
+      views::Checkbox* always_checkbox = GetAlwaysTranslateCheckbox();
+      if (always_checkbox)
+        model_->SetAlwaysTranslate(always_checkbox->checked());
       if (model_->IsPageTranslatedInCurrentLanguages()) {
         model_->GoBackFromAdvanced();
         UpdateChildVisibilities();
@@ -444,11 +453,13 @@ void TranslateBubbleView::HandleButtonPressed(
       break;
     }
     case BUTTON_ID_ALWAYS_TRANSLATE: {
-      // Do nothing. The state of the checkbox affects only when the 'Done'
-      // button is pressed.
-      translate::ReportUiAction(always_translate_checkbox_->checked()
-                                    ? translate::ALWAYS_TRANSLATE_CHECKED
-                                    : translate::ALWAYS_TRANSLATE_UNCHECKED);
+      views::Checkbox* always_checkbox = GetAlwaysTranslateCheckbox();
+      if (always_checkbox) {

[groby-ooo-7-16, 2016/06/20 19:03:10]

You might want to DCHECK here, instead of if(...) - it's technically invalid for
us to get a button press, but without a checkbox that generated it.

[ftang, 2016/06/20 21:30:39]

Done.

+        should_always_translate_ = always_checkbox->checked();
+        translate::ReportUiAction(should_always_translate_
+                                      ? translate::ALWAYS_TRANSLATE_CHECKED
+                                      : translate::ALWAYS_TRANSLATE_UNCHECKED);
+      }
       break;
     }
   }
@@ -527,6 +538,11 @@ void TranslateBubbleView::HandleComboboxPerformAction(
 }
 
 void TranslateBubbleView::UpdateChildVisibilities() {
+  // Update the statew of the always translate checkbox
+  if (advanced_always_translate_checkbox_)
+    advanced_always_translate_checkbox_->SetChecked(should_always_translate_);
+  if (before_always_translate_checkbox_)
+    before_always_translate_checkbox_->SetChecked(should_always_translate_);
   for (int i = 0; i < child_count(); i++) {
     views::View* view = child_at(i);
     view->SetVisible(view == GetCurrentView());
@@ -608,13 +624,11 @@ views::View* TranslateBubbleView::CreateViewBeforeTranslate() {
     layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
     layout->StartRow(0, COLUMN_SET_ID_MESSAGE);
     layout->SkipColumns(1);
-    always_translate_checkbox_ = new views::Checkbox(
+    before_always_translate_checkbox_ = new views::Checkbox(
         l10n_util::GetStringUTF16(IDS_TRANSLATE_BUBBLE_ALWAYS_DO_THIS));
-    always_translate_checkbox_->SetChecked(
-        model_->ShouldAlwaysTranslateBeCheckedByDefault());
-    always_translate_checkbox_->set_id(BUTTON_ID_ALWAYS_TRANSLATE);
-    always_translate_checkbox_->set_listener(this);
-    layout->AddView(always_translate_checkbox_);
+    before_always_translate_checkbox_->set_id(BUTTON_ID_ALWAYS_TRANSLATE);
+    before_always_translate_checkbox_->set_listener(this);
+    layout->AddView(before_always_translate_checkbox_);
   }
   if (Use2016Q2UI()) {
     layout->AddPaddingRow(0, views::kPanelSubVerticalSpacing);
@@ -831,9 +845,9 @@ views::View* TranslateBubbleView::CreateViewAdvanced() {
 
   // In an incognito window, "Always translate" checkbox shouldn't be shown.
   if (!is_in_incognito_window_) {
-    always_translate_checkbox_ = new views::Checkbox(base::string16());
-    always_translate_checkbox_->set_id(BUTTON_ID_ALWAYS_TRANSLATE);
-    always_translate_checkbox_->set_listener(this);
+    advanced_always_translate_checkbox_ = new views::Checkbox(base::string16());
+    advanced_always_translate_checkbox_->set_id(BUTTON_ID_ALWAYS_TRANSLATE);
+    advanced_always_translate_checkbox_->set_listener(this);
   }
 
   views::View* view = new views::View();
@@ -879,7 +893,7 @@ views::View* TranslateBubbleView::CreateViewAdvanced() {
     layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
     layout->StartRow(0, COLUMN_SET_ID_LANGUAGES);
     layout->SkipColumns(1);
-    layout->AddView(always_translate_checkbox_);
+    layout->AddView(advanced_always_translate_checkbox_);
   }
 
   layout->AddPaddingRow(0, views::kUnrelatedControlVerticalSpacing);
@@ -904,6 +918,18 @@ views::View* TranslateBubbleView::CreateViewAdvanced() {
   return view;
 }
 
+views::Checkbox* TranslateBubbleView::GetAlwaysTranslateCheckbox() {
+  if (model_->GetViewState() == TranslateBubbleModel::VIEW_STATE_ADVANCED) {
+    return advanced_always_translate_checkbox_;
+  } else if (model_->GetViewState() ==
+             TranslateBubbleModel::VIEW_STATE_BEFORE_TRANSLATE) {
+    return before_always_translate_checkbox_;
+  } else {
+    NOTREACHED();
+    return nullptr;
+  }
+}
+
 void TranslateBubbleView::SwitchView(
     TranslateBubbleModel::ViewState view_state) {
   if (model_->GetViewState() == view_state)
@@ -934,12 +960,9 @@ void TranslateBubbleView::UpdateAdvancedView() {
       model_->GetLanguageNameAt(model_->GetTargetLanguageIndex());
 
   // "Always translate" checkbox doesn't exist in an incognito window.
-  if (always_translate_checkbox_) {
-    always_translate_checkbox_->SetText(
+  if (advanced_always_translate_checkbox_) {
+    advanced_always_translate_checkbox_->SetText(
         l10n_util::GetStringUTF16(IDS_TRANSLATE_BUBBLE_ALWAYS));
-    always_translate_checkbox_->SetChecked(
-        Use2016Q2UI() ? model_->ShouldAlwaysTranslateBeCheckedByDefault()
-                      : model_->ShouldAlwaysTranslate());
   }
 
   base::string16 label;