Implement the 2016Q2 Translate UI designe spec out in

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

Index: chrome/browser/ui/views/translate/translate_bubble_view_unittest.cc
diff --git a/chrome/browser/ui/views/translate/translate_bubble_view_unittest.cc b/chrome/browser/ui/views/translate/translate_bubble_view_unittest.cc
index d9417ae413f6d4cb379f3ff0e1f39701853d3a64..549b4c62579ce765eeb6c32b2b765aa99109d76f 100644
--- a/chrome/browser/ui/views/translate/translate_bubble_view_unittest.cc
+++ b/chrome/browser/ui/views/translate/translate_bubble_view_unittest.cc
@@ -7,11 +7,17 @@
 #include <memory>
 #include <utility>
 
+#include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/ui/translate/translate_bubble_model.h"
 #include "chrome/browser/ui/translate/translate_bubble_view_state_transition.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "ui/events/event_constants.h"
+#include "ui/events/event_utils.h"
+#include "ui/gfx/range/range.h"
+#include "ui/views/bubble/bubble_frame_view.h"
 #include "ui/views/controls/button/checkbox.h"
 #include "ui/views/controls/combobox/combobox.h"
+#include "ui/views/controls/styled_label.h"
 #include "ui/views/test/combobox_test_api.h"
 #include "ui/views/test/views_test_base.h"
 #include "ui/views/widget/widget.h"
@@ -28,6 +34,7 @@ class MockTranslateBubbleModel : public TranslateBubbleModel {
         never_translate_language_(false),
         never_translate_site_(false),
         should_always_translate_(false),
+        always_translate_checked_(false),
         set_always_translate_called_count_(0),
         translate_called_(false),
         revert_translation_called_(false),
@@ -54,7 +61,7 @@ class MockTranslateBubbleModel : public TranslateBubbleModel {
   int GetNumberOfLanguages() const override { return 1000; }
 
   base::string16 GetLanguageNameAt(int index) const override {
-    return base::string16();
+    return base::ASCIIToUTF16("English");
   }
 
   int GetOriginalLanguageIndex() const override {
@@ -81,6 +88,9 @@ class MockTranslateBubbleModel : public TranslateBubbleModel {
     never_translate_site_ = value;
   }
 
+  bool GetAlwaysTranslateChecked() const override {
+    return always_translate_checked_;
+  }
   bool ShouldAlwaysTranslate() const override {

[msw, 2016/04/29 20:00:02]

nit: blank line above

[ftang, 2016/04/29 22:37:02]

Done.

     return should_always_translate_;
   }
@@ -113,6 +123,7 @@ class MockTranslateBubbleModel : public TranslateBubbleModel {
   bool never_translate_language_;
   bool never_translate_site_;
   bool should_always_translate_;
+  bool always_translate_checked_;
   int set_always_translate_called_count_;
   bool translate_called_;
   bool revert_translation_called_;
@@ -143,6 +154,20 @@ class TranslateBubbleViewTest : public views::ViewsTestBase {
 
     mock_model_ = new MockTranslateBubbleModel(
         TranslateBubbleModel::VIEW_STATE_BEFORE_TRANSLATE);
+
+    base::FeatureList::ClearInstanceForTesting();
+    base::FeatureList::SetInstance(base::WrapUnique(new base::FeatureList));
+  }
+
+  void TurnOnTranslate2016Q2UIFlag() {
+    base::FeatureList::ClearInstanceForTesting();
+    std::unique_ptr<base::FeatureList> feature_list(new base::FeatureList);
+    feature_list->InitializeFromCommandLine(
+        translate::kTranslateUI2016Q2.name, "");

[msw, 2016/04/29 20:00:02]

nit: s/""/std::string()/

[ftang, 2016/04/29 22:37:02]

Done.

+    base::FeatureList::SetInstance(std::move(feature_list));
+  }
+
+  void CreateAndShowBubble() {
     std::unique_ptr<TranslateBubbleModel> model(mock_model_);
     bubble_ = new TranslateBubbleView(anchor_widget_->GetContentsView(),
                                       std::move(model),
@@ -166,6 +191,17 @@ class TranslateBubbleViewTest : public views::ViewsTestBase {
 };
 
 TEST_F(TranslateBubbleViewTest, TranslateButton) {
+  CreateAndShowBubble();
+  EXPECT_FALSE(mock_model_->translate_called_);
+
+  // Press the "Translate" button.
+  bubble_->HandleButtonPressed(TranslateBubbleView::BUTTON_ID_TRANSLATE);
+  EXPECT_TRUE(mock_model_->translate_called_);
+}
+
+TEST_F(TranslateBubbleViewTest, TranslateButtonIn2016Q2UI) {
+  TurnOnTranslate2016Q2UIFlag();
+  CreateAndShowBubble();
   EXPECT_FALSE(mock_model_->translate_called_);
 
   // Press the "Translate" button.
@@ -173,7 +209,25 @@ TEST_F(TranslateBubbleViewTest, TranslateButton) {
   EXPECT_TRUE(mock_model_->translate_called_);
 }
 
+TEST_F(TranslateBubbleViewTest, CloseButtonIn2016Q2UI) {
+  TurnOnTranslate2016Q2UIFlag();
+  CreateAndShowBubble();
+  EXPECT_FALSE(mock_model_->translate_called_);
+  EXPECT_FALSE(mock_model_->translation_declined_);
+  EXPECT_FALSE(bubble_->GetWidget()->IsClosed());
+
+  // Press the "Close" button.
+  bubble_->GetBubbleFrameView()->ButtonPressed(

[msw, 2016/04/29 20:00:02]

nit: maybe use EventGenerator.

[ftang, 2016/04/29 22:37:02]

Acknowledged.

+      bubble_->GetBubbleFrameView()->close_,
+      ui::MouseEvent(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
+                     ui::EventTimeForNow(), ui::EF_NONE, ui::EF_NONE));
+
+  EXPECT_FALSE(mock_model_->translate_called_);
+  EXPECT_TRUE(mock_model_->translation_declined_);
+}
+
 TEST_F(TranslateBubbleViewTest, ComboboxNope) {
+  CreateAndShowBubble();
   views::test::ComboboxTestApi test_api(denial_combobox());
   EXPECT_FALSE(denial_button_clicked());
   EXPECT_FALSE(bubble_->GetWidget()->IsClosed());
@@ -184,7 +238,38 @@ TEST_F(TranslateBubbleViewTest, ComboboxNope) {
   EXPECT_TRUE(bubble_->GetWidget()->IsClosed());
 }
 
+TEST_F(TranslateBubbleViewTest, ComboboxNopeIn2016Q2UI) {
+  TurnOnTranslate2016Q2UIFlag();
+  CreateAndShowBubble();
+  views::test::ComboboxTestApi test_api(denial_combobox());
+  EXPECT_FALSE(denial_button_clicked());
+  EXPECT_FALSE(bubble_->GetWidget()->IsClosed());
+
+  test_api.PerformActionAt(static_cast<int>(
+      TranslateBubbleView::DenialComboboxIndex::DONT_TRANSLATE));
+
+  // In the 2016Q2 UI, we should not close nor take action.

[msw, 2016/04/29 20:00:02]

Isn't the 'nope' button simply not available in the new UI? I'm not sure it
makes sense to test your workaround to no-op when the non-existent button is
pressed...

[ftang, 2016/04/29 22:37:02]

no, that is not true. the menu item will be there but the clicking should have
no effect. User will still see it.

[msw, 2016/04/29 23:43:39]

Sorry, in the mocks it doesn't show a 'Nope' button. It shows [Translate],
[Never (site)], [Never (lang)] under the [Options] button, a close (x), and an
[Always] checkbox; where is the [Nope] button? Can you post a screenshot with
this button?
https://docs.google.com/presentation/d/1QECg9ZPANcOISaiS3tEDgOj4yJNSFnPhq2VWE...

It sounds like you saying that there is a [Nope] button that won't even close
the bubble, so users can repeatedly click [Nope] and the bubble will do
*nothing*, even though users (myself included) would expect it to close the
bubble. I hope I'm just misunderstanding your meaning... can you clarify?

+  EXPECT_FALSE(denial_button_clicked());
+  EXPECT_FALSE(bubble_->GetWidget()->IsClosed());
+}
+
 TEST_F(TranslateBubbleViewTest, ComboboxNeverTranslateLanguage) {
+  CreateAndShowBubble();
+  views::test::ComboboxTestApi test_api(denial_combobox());
+  EXPECT_FALSE(bubble_->GetWidget()->IsClosed());
+  EXPECT_FALSE(mock_model_->never_translate_language_);
+  EXPECT_FALSE(denial_button_clicked());
+
+  test_api.PerformActionAt(static_cast<int>(
+      TranslateBubbleView::DenialComboboxIndex::NEVER_TRANSLATE_LANGUAGE));
+  EXPECT_TRUE(denial_button_clicked());
+  EXPECT_TRUE(mock_model_->never_translate_language_);
+  EXPECT_TRUE(bubble_->GetWidget()->IsClosed());
+}
+
+TEST_F(TranslateBubbleViewTest, ComboboxNeverTranslateLanguageIn2016Q2UI) {
+  TurnOnTranslate2016Q2UIFlag();
+  CreateAndShowBubble();
   views::test::ComboboxTestApi test_api(denial_combobox());
   EXPECT_FALSE(bubble_->GetWidget()->IsClosed());
   EXPECT_FALSE(mock_model_->never_translate_language_);
@@ -198,6 +283,22 @@ TEST_F(TranslateBubbleViewTest, ComboboxNeverTranslateLanguage) {
 }
 
 TEST_F(TranslateBubbleViewTest, ComboboxNeverTranslateSite) {
+  CreateAndShowBubble();
+  views::test::ComboboxTestApi test_api(denial_combobox());
+  EXPECT_FALSE(mock_model_->never_translate_site_);
+  EXPECT_FALSE(denial_button_clicked());
+  EXPECT_FALSE(bubble_->GetWidget()->IsClosed());
+
+  test_api.PerformActionAt(static_cast<int>(
+      TranslateBubbleView::DenialComboboxIndex::NEVER_TRANSLATE_SITE));
+  EXPECT_TRUE(denial_button_clicked());
+  EXPECT_TRUE(mock_model_->never_translate_site_);
+  EXPECT_TRUE(bubble_->GetWidget()->IsClosed());
+}
+
+TEST_F(TranslateBubbleViewTest, ComboboxNeverTranslateSiteIn2016Q2UI) {
+  TurnOnTranslate2016Q2UIFlag();
+  CreateAndShowBubble();
   views::test::ComboboxTestApi test_api(denial_combobox());
   EXPECT_FALSE(mock_model_->never_translate_site_);
   EXPECT_FALSE(denial_button_clicked());
@@ -211,6 +312,7 @@ TEST_F(TranslateBubbleViewTest, ComboboxNeverTranslateSite) {
 }
 
 TEST_F(TranslateBubbleViewTest, AdvancedLink) {
+  CreateAndShowBubble();
   EXPECT_EQ(TranslateBubbleModel::VIEW_STATE_BEFORE_TRANSLATE,
             bubble_->GetViewState());
 
@@ -219,7 +321,20 @@ TEST_F(TranslateBubbleViewTest, AdvancedLink) {
   EXPECT_EQ(TranslateBubbleModel::VIEW_STATE_ADVANCED, bubble_->GetViewState());
 }
 
+TEST_F(TranslateBubbleViewTest, AdvancedLinkIn2016Q2UI) {
+  TurnOnTranslate2016Q2UIFlag();
+  CreateAndShowBubble();
+  EXPECT_EQ(TranslateBubbleModel::VIEW_STATE_BEFORE_TRANSLATE,
+            bubble_->GetViewState());
+
+  // call the StyledLabelLinkClicked();

[msw, 2016/04/29 20:00:02]

nit: "Click the styled label link."

[ftang, 2016/04/29 22:37:02]

Done.

+  views::StyledLabel styled_label(base::ASCIIToUTF16("test"), nullptr);

[msw, 2016/04/29 20:00:02]

You don't need to supply a valid styled label here; pass nullptr instead.

[ftang, 2016/04/29 22:37:02]

no, I tried it. It won't work and will give me error.

+  bubble_->StyledLabelLinkClicked(&styled_label, gfx::Range(), ui::EF_NONE);
+  EXPECT_EQ(TranslateBubbleModel::VIEW_STATE_ADVANCED, bubble_->GetViewState());
+}
+
 TEST_F(TranslateBubbleViewTest, ShowOriginalButton) {
+  CreateAndShowBubble();
   bubble_->SwitchView(TranslateBubbleModel::VIEW_STATE_AFTER_TRANSLATE);
 
   // Click the "Show original" button to revert translation.
@@ -229,6 +344,7 @@ TEST_F(TranslateBubbleViewTest, ShowOriginalButton) {
 }
 
 TEST_F(TranslateBubbleViewTest, TryAgainButton) {
+  CreateAndShowBubble();
   bubble_->SwitchToErrorView(translate::TranslateErrors::NETWORK);
 
   EXPECT_EQ(translate::TranslateErrors::NETWORK, mock_model_->error_type_);
@@ -240,6 +356,7 @@ TEST_F(TranslateBubbleViewTest, TryAgainButton) {
 }
 
 TEST_F(TranslateBubbleViewTest, AlwaysTranslateCheckboxAndCancelButton) {
+  CreateAndShowBubble();
   bubble_->SwitchView(TranslateBubbleModel::VIEW_STATE_ADVANCED);
 
   // Click the "Always Translate" checkbox. Changing the state of this checkbox
@@ -263,6 +380,7 @@ TEST_F(TranslateBubbleViewTest, AlwaysTranslateCheckboxAndCancelButton) {
 }
 
 TEST_F(TranslateBubbleViewTest, AlwaysTranslateCheckboxAndDoneButton) {
+  CreateAndShowBubble();
   bubble_->SwitchView(TranslateBubbleModel::VIEW_STATE_ADVANCED);
 
   // Click the "Always Translate" checkbox. Changing the state of this checkbox
@@ -286,6 +404,7 @@ TEST_F(TranslateBubbleViewTest, AlwaysTranslateCheckboxAndDoneButton) {
 }
 
 TEST_F(TranslateBubbleViewTest, DoneButton) {
+  CreateAndShowBubble();
   bubble_->SwitchView(TranslateBubbleModel::VIEW_STATE_ADVANCED);
 
   // Click the "Done" button to translate. The selected languages by the user
@@ -304,6 +423,7 @@ TEST_F(TranslateBubbleViewTest, DoneButton) {
 }
 
 TEST_F(TranslateBubbleViewTest, DoneButtonWithoutTranslating) {
+  CreateAndShowBubble();
   EXPECT_EQ(TranslateBubbleModel::VIEW_STATE_BEFORE_TRANSLATE,
             bubble_->GetViewState());
 
@@ -331,6 +451,7 @@ TEST_F(TranslateBubbleViewTest, DoneButtonWithoutTranslating) {
 }
 
 TEST_F(TranslateBubbleViewTest, CancelButtonReturningBeforeTranslate) {
+  CreateAndShowBubble();
   bubble_->SwitchView(TranslateBubbleModel::VIEW_STATE_BEFORE_TRANSLATE);
   bubble_->SwitchView(TranslateBubbleModel::VIEW_STATE_ADVANCED);
 
@@ -342,6 +463,7 @@ TEST_F(TranslateBubbleViewTest, CancelButtonReturningBeforeTranslate) {
 }
 
 TEST_F(TranslateBubbleViewTest, CancelButtonReturningAfterTranslate) {
+  CreateAndShowBubble();
   bubble_->SwitchView(TranslateBubbleModel::VIEW_STATE_AFTER_TRANSLATE);
   bubble_->SwitchView(TranslateBubbleModel::VIEW_STATE_ADVANCED);
 
@@ -353,6 +475,7 @@ TEST_F(TranslateBubbleViewTest, CancelButtonReturningAfterTranslate) {
 }
 
 TEST_F(TranslateBubbleViewTest, CancelButtonReturningError) {
+  CreateAndShowBubble();
   bubble_->SwitchView(TranslateBubbleModel::VIEW_STATE_ERROR);
   bubble_->SwitchView(TranslateBubbleModel::VIEW_STATE_ADVANCED);