fix "Always do this" checkbox

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

 // Copyright 2013 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/views/translate/translate_bubble_view.h"
 
 #include <stddef.h>
 #include <algorithm>
 #include <string>
 #include <utility>
@@ skipping 161 matching lines @@
 }
 
 void TranslateBubbleView::CloseBubble() {
   mouse_handler_.reset();
   LocationBarBubbleDelegateView::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();
   error_view_ = CreateViewError();
   advanced_view_ = CreateViewAdvanced();
 
   AddChildView(before_translate_view_);
   AddChildView(translating_view_);
   AddChildView(after_translate_view_);
   AddChildView(error_view_);
@@ skipping 161 matching lines @@
 }
 
 TranslateBubbleModel::ViewState TranslateBubbleView::GetViewState() const {
   return model_->GetViewState();
 }
 
 TranslateBubbleView::TranslateBubbleView(
     views::View* anchor_view,
     std::unique_ptr<TranslateBubbleModel> model,
     translate::TranslateErrors::Type error_type,
     content::WebContents* web_contents)

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

Please initialize |should_always_translate_| here

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

Done.

     : LocationBarBubbleDelegateView(anchor_view, web_contents),
       WebContentsObserver(web_contents),
       before_translate_view_(NULL),
       translating_view_(NULL),
       after_translate_view_(NULL),
       error_view_(NULL),
       advanced_view_(NULL),
       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),
       model_(std::move(model)),
       error_type_(error_type),
       is_in_incognito_window_(
           web_contents && web_contents->GetBrowserContext()->IsOffTheRecord()) {
   translate_bubble_view_ = this;
   if (web_contents)  // web_contents can be null in unit_tests.
     mouse_handler_.reset(new WebContentMouseHandler(this, web_contents));
@@ skipping 13 matching lines @@
       return advanced_view_;
   }
   NOTREACHED();
   return NULL;
 }
 
 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();
         SizeToContents();
       } else {
         model_->Translate();
         SwitchView(TranslateBubbleModel::VIEW_STATE_TRANSLATING);
       }
       translate::ReportUiAction(translate::DONE_BUTTON_CLICKED);
       break;
@@ skipping 10 matching lines @@
       translate::ReportUiAction(translate::TRY_AGAIN_BUTTON_CLICKED);
       break;
     }
     case BUTTON_ID_SHOW_ORIGINAL: {
       model_->RevertTranslation();
       GetWidget()->Close();
       translate::ReportUiAction(translate::SHOW_ORIGINAL_BUTTON_CLICKED);
       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;
     }
   }
 }
 
 void TranslateBubbleView::HandleLinkClicked(
     TranslateBubbleView::LinkID sender_id) {
   switch (sender_id) {
     case LINK_ID_ADVANCED: {
       SwitchView(TranslateBubbleModel::VIEW_STATE_ADVANCED);
@@ skipping 58 matching lines @@
       model_->UpdateTargetLanguageIndex(
           target_language_combobox_->selected_index());
       UpdateAdvancedView();
       translate::ReportUiAction(translate::TARGET_LANGUAGE_MENU_CLICKED);
       break;
     }
   }
 }
 
 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());
   }
 }
 
 views::View* TranslateBubbleView::CreateViewBeforeTranslate() {
   const int kQuestionWidth = 200;
   base::string16 original_language_name =
       model_->GetLanguageNameAt(model_->GetOriginalLanguageIndex());
@@ skipping 61 matching lines @@
         l10n_util::GetStringUTF16(IDS_TRANSLATE_BUBBLE_BEFORE_TRANSLATE)));
     layout->AddView(
         CreateLink(this, IDS_TRANSLATE_BUBBLE_ADVANCED, LINK_ID_ADVANCED));
   }
 
   // In an incognito window, the "Always translate" checkbox shouldn't be shown.
   if (Use2016Q2UI() && !is_in_incognito_window_) {
     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);
   } else {
     layout->AddPaddingRow(0, views::kUnrelatedControlVerticalSpacing);
   }
 
   layout->StartRow(0, COLUMN_SET_ID_CONTENT);
   views::LabelButton* accept_button =
       Use2016Q2UI()
@@ skipping 196 matching lines @@
   target_language_combobox_model_.reset(
       new LanguageComboboxModel(target_default_index, model_.get()));
   target_language_combobox_ =
       new views::Combobox(target_language_combobox_model_.get());
 
   target_language_combobox_->set_id(COMBOBOX_ID_TARGET_LANGUAGE);
   target_language_combobox_->set_listener(this);
 
   // 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();
   views::GridLayout* layout = new views::GridLayout(view);
   view->SetLayoutManager(layout);
 
   using views::GridLayout;
 
   enum {
     COLUMN_SET_ID_LANGUAGES,
@@ skipping 25 matching lines @@
   layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
 
   layout->StartRow(0, COLUMN_SET_ID_LANGUAGES);
   layout->AddView(target_language_label);
   layout->AddView(target_language_combobox_);
 
   if (!is_in_incognito_window_) {
     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);
 
   layout->StartRow(0, COLUMN_SET_ID_BUTTONS);
   layout->AddView(CreateLink(this, IDS_TRANSLATE_BUBBLE_LANGUAGE_SETTINGS,
                              LINK_ID_LANGUAGE_SETTINGS));
   advanced_done_button_ =
       Use2016Q2UI()
           ? CreateBlueButton(this, l10n_util::GetStringUTF16(IDS_DONE),
                              BUTTON_ID_DONE)
           : CreateLabelButton(this, l10n_util::GetStringUTF16(IDS_DONE),
                               BUTTON_ID_DONE);
   advanced_done_button_->SetIsDefault(true);
   advanced_cancel_button_ = CreateLabelButton(
       this, l10n_util::GetStringUTF16(IDS_CANCEL), BUTTON_ID_CANCEL);
   layout->AddView(advanced_done_button_);
   layout->AddView(advanced_cancel_button_);
 
   UpdateAdvancedView();
 
   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)
     return;
 
   model_->SetViewState(view_state);
   UpdateChildVisibilities();
   if (view_state == TranslateBubbleModel::VIEW_STATE_ADVANCED)
     UpdateAdvancedView();
   SizeToContents();
@@ skipping 10 matching lines @@
   DCHECK(source_language_combobox_);
   DCHECK(target_language_combobox_);
   DCHECK(advanced_done_button_);
 
   base::string16 source_language_name =
       model_->GetLanguageNameAt(model_->GetOriginalLanguageIndex());
   base::string16 target_language_name =
       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;
   if (model_->IsPageTranslatedInCurrentLanguages())
     label = l10n_util::GetStringUTF16(IDS_DONE);
   else
     label = l10n_util::GetStringUTF16(IDS_TRANSLATE_BUBBLE_ACCEPT);
   advanced_done_button_->SetText(label);
   advanced_done_button_->SizeToPreferredSize();
   if (advanced_view_)
     advanced_view_->Layout();
 }