Add test for WebBluetooth chooser spinner, status text and rescan button

chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc

Index: chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc
diff --git a/chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc b/chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc
index 0fb6111e3110c876b297f6ec8337975c0dd91eb9..8d4e26d55d12bcdc6cea31805a35dc4f616720b1 100644
--- a/chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc
+++ b/chrome/browser/ui/views/extensions/chooser_dialog_view_browsertest.cc
@@ -5,6 +5,7 @@
 #include "chrome/browser/ui/views/extensions/chooser_dialog_view.h"
 
 #include "base/macros.h"
+#include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/chooser_controller/mock_chooser_controller.h"
 #include "chrome/browser/extensions/extension_browsertest.h"
 #include "chrome/browser/platform_util.h"
@@ -14,8 +15,11 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/views/controls/button/label_button.h"
+#include "ui/views/controls/link.h"
 #include "ui/views/controls/styled_label.h"
 #include "ui/views/controls/table/table_view.h"
+#include "ui/views/controls/throbber.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/window/dialog_client_view.h"
 
@@ -41,6 +45,17 @@ class ChooserDialogViewTest : public ExtensionBrowserTest {
         platform_util::GetViewForWindow(
             browser()->window()->GetNativeWindow()));
     modal_dialog->Show();
+    throbber_ = chooser_dialog_view_->chooser_content_view_for_test()
+                    ->throbber_for_test();
+    ASSERT_TRUE(throbber_);
+    discovery_state_ = chooser_dialog_view_->chooser_content_view_for_test()
+                           ->discovery_state_for_test();
+    ASSERT_TRUE(discovery_state_);
+    ok_button_ = chooser_dialog_view_->GetDialogClientView()->ok_button();
+    ASSERT_TRUE(ok_button_);
+    cancel_button_ =
+        chooser_dialog_view_->GetDialogClientView()->cancel_button();
+    ASSERT_TRUE(cancel_button_);
     styled_label_ = chooser_dialog_view_->chooser_content_view_for_test()
                         ->styled_label_for_test();
     ASSERT_TRUE(styled_label_);
@@ -51,6 +66,10 @@ class ChooserDialogViewTest : public ExtensionBrowserTest {
   ChooserDialogView* chooser_dialog_view_;
   views::TableView* table_view_;
   ui::TableModel* table_model_;
+  views::Throbber* throbber_;
+  views::Link* discovery_state_;
+  views::LabelButton* ok_button_;
+  views::LabelButton* cancel_button_;
   views::StyledLabel* styled_label_;
 
  private:
@@ -58,6 +77,8 @@ class ChooserDialogViewTest : public ExtensionBrowserTest {
 };
 
 IN_PROC_BROWSER_TEST_F(ChooserDialogViewTest, InitialState) {

[msw, 2016/07/22 23:37:05]

It seems like some of these could be unit tests; they don't need a browser.

[juncai, 2016/07/25 20:14:10]

Removed some redundant tests that are already in unit tests.

Done.

+  // |table_view_| is visible.

[msw, 2016/07/22 23:37:05]

ditto nit here and elsewhere

[juncai, 2016/07/25 20:14:10]

Removed this line.

+  EXPECT_TRUE(table_view_->visible());
   // Since "No devices found." needs to be displayed on the |table_view_|,
   // the number of rows is 1.
   EXPECT_EQ(1, table_view_->RowCount());
@@ -69,6 +90,143 @@ IN_PROC_BROWSER_TEST_F(ChooserDialogViewTest, InitialState) {
   // No option selected.
   EXPECT_EQ(0, table_view_->SelectedRowCount());
   EXPECT_EQ(-1, table_view_->FirstSelectedRow());
+  // |throbber_| is not visible.
+  EXPECT_FALSE(throbber_->visible());
+  // OK button is disabled since there is no option.
+  EXPECT_FALSE(ok_button_->enabled());
+  // Cancel button is always enabled.
+  EXPECT_TRUE(cancel_button_->enabled());
+  // Check the text on buttons.
+  EXPECT_EQ(
+      l10n_util::GetStringUTF16(IDS_USB_DEVICE_CHOOSER_CONNECT_BUTTON_TEXT),
+      ok_button_->GetText());
+  EXPECT_EQ(l10n_util::GetStringUTF16(IDS_DEVICE_CHOOSER_CANCEL_BUTTON_TEXT),
+            cancel_button_->GetText());
+  // |discovery_state_|'s text is an empty string.
+  EXPECT_EQ(base::string16(), discovery_state_->text());

[msw, 2016/07/22 23:37:04]

nit: EXPECT_TRUE(discovery_state_->text().empty());

[juncai, 2016/07/25 20:14:10]

Removed this line.

+}
+
+IN_PROC_BROWSER_TEST_F(ChooserDialogViewTest, SelectAndDeselectAnOption) {
+  mock_chooser_controller_->OptionAdded(base::ASCIIToUTF16("a"));
+  mock_chooser_controller_->OptionAdded(base::ASCIIToUTF16("b"));
+  mock_chooser_controller_->OptionAdded(base::ASCIIToUTF16("c"));
+  // OK button is disabled since no option is selected.
+  EXPECT_FALSE(ok_button_->enabled());
+  EXPECT_TRUE(cancel_button_->enabled());
+
+  // Select option 0.
+  table_view_->Select(0);
+  // OK button is enabled since an option is selected.
+  EXPECT_TRUE(ok_button_->enabled());
+  EXPECT_TRUE(cancel_button_->enabled());
+
+  // Unselect option 0.
+  table_view_->Select(-1);
+  // OK button is disabled since no option is selected.
+  EXPECT_FALSE(ok_button_->enabled());
+  EXPECT_TRUE(cancel_button_->enabled());
+
+  // Select option 1.
+  table_view_->Select(1);
+  // OK button is enabled since an option is selected.
+  EXPECT_TRUE(ok_button_->enabled());
+  EXPECT_TRUE(cancel_button_->enabled());
+
+  // Unselect option 1.
+  table_view_->Select(-1);
+  // OK button is disabled since no option is selected.
+  EXPECT_FALSE(ok_button_->enabled());
+  EXPECT_TRUE(cancel_button_->enabled());
+}
+
+IN_PROC_BROWSER_TEST_F(ChooserDialogViewTest,
+                       SelectAnOptionAndThenSelectAnotherOption) {
+  mock_chooser_controller_->OptionAdded(base::ASCIIToUTF16("a"));
+  mock_chooser_controller_->OptionAdded(base::ASCIIToUTF16("b"));
+  mock_chooser_controller_->OptionAdded(base::ASCIIToUTF16("c"));
+  EXPECT_FALSE(ok_button_->enabled());
+  EXPECT_TRUE(cancel_button_->enabled());
+
+  // Select option 0.
+  table_view_->Select(0);
+  EXPECT_TRUE(ok_button_->enabled());
+  EXPECT_TRUE(cancel_button_->enabled());
+
+  // Select option 1.
+  table_view_->Select(1);
+  // Both OK and Cancel buttons are still enabled.
+  EXPECT_TRUE(ok_button_->enabled());
+  EXPECT_TRUE(cancel_button_->enabled());
+
+  // Select option 2.
+  table_view_->Select(2);
+  // Both OK and Cancel buttons are still enabled.
+  EXPECT_TRUE(ok_button_->enabled());
+  EXPECT_TRUE(cancel_button_->enabled());
+}
+
+IN_PROC_BROWSER_TEST_F(ChooserDialogViewTest,
+                       SelectAnOptionAndRemoveAnotherOption) {
+  mock_chooser_controller_->OptionAdded(base::ASCIIToUTF16("a"));
+  mock_chooser_controller_->OptionAdded(base::ASCIIToUTF16("b"));
+  mock_chooser_controller_->OptionAdded(base::ASCIIToUTF16("c"));
+  EXPECT_FALSE(ok_button_->enabled());
+  EXPECT_TRUE(cancel_button_->enabled());
+
+  // Select option 1.
+  table_view_->Select(1);
+  EXPECT_TRUE(ok_button_->enabled());
+  EXPECT_TRUE(cancel_button_->enabled());
+
+  // Remove option 0, the list becomes: b c.
+  mock_chooser_controller_->OptionRemoved(base::ASCIIToUTF16("a"));
+  // Both OK and Cancel buttons are still enabled.
+  EXPECT_TRUE(ok_button_->enabled());
+  EXPECT_TRUE(cancel_button_->enabled());
+
+  // Remove option 1.
+  mock_chooser_controller_->OptionRemoved(base::ASCIIToUTF16("c"));
+  // Both OK and Cancel buttons are still enabled.
+  EXPECT_TRUE(ok_button_->enabled());
+  EXPECT_TRUE(cancel_button_->enabled());
+}
+
+IN_PROC_BROWSER_TEST_F(ChooserDialogViewTest,
+                       SelectAnOptionAndRemoveTheSelectedOption) {
+  mock_chooser_controller_->OptionAdded(base::ASCIIToUTF16("a"));
+  mock_chooser_controller_->OptionAdded(base::ASCIIToUTF16("b"));
+  mock_chooser_controller_->OptionAdded(base::ASCIIToUTF16("c"));
+  EXPECT_FALSE(ok_button_->enabled());
+  EXPECT_TRUE(cancel_button_->enabled());
+
+  // Select option 1.
+  table_view_->Select(1);
+  EXPECT_TRUE(ok_button_->enabled());
+  EXPECT_TRUE(cancel_button_->enabled());
+
+  // Remove option 1.
+  mock_chooser_controller_->OptionRemoved(base::ASCIIToUTF16("b"));
+  // OK button is disabled since the selected option is removed.
+  EXPECT_FALSE(ok_button_->enabled());
+  EXPECT_TRUE(cancel_button_->enabled());
+}
+
+IN_PROC_BROWSER_TEST_F(ChooserDialogViewTest,
+                       AddAnOptionAndSelectItAndRemoveTheSelectedOption) {

[msw, 2016/07/22 23:37:05]

How is this usefully different from SelectAnOptionAndRemoveTheSelectedOption? It
just adds one option instead of three.

[juncai, 2016/07/25 20:14:10]

In this case, since there is only one option added, and after removing it, there
is no remaining option, so I think it is worth testing this case?

[msw, 2016/07/26 00:18:56]

I suppose it's okay.

+  mock_chooser_controller_->OptionAdded(base::ASCIIToUTF16("a"));
+  EXPECT_FALSE(ok_button_->enabled());
+  EXPECT_TRUE(cancel_button_->enabled());
+
+  // Select option 0.
+  table_view_->Select(0);
+  EXPECT_TRUE(ok_button_->enabled());
+  EXPECT_TRUE(cancel_button_->enabled());
+
+  // Remove option 0.
+  mock_chooser_controller_->OptionRemoved(base::ASCIIToUTF16("a"));
+  // There is no option shown now and the OK button is disabled.
+  EXPECT_FALSE(ok_button_->enabled());
+  EXPECT_TRUE(cancel_button_->enabled());
 }
 
 IN_PROC_BROWSER_TEST_F(ChooserDialogViewTest, Accept) {
@@ -90,3 +248,101 @@ IN_PROC_BROWSER_TEST_F(ChooserDialogViewTest, ClickStyledLabelLink) {
   EXPECT_CALL(*mock_chooser_controller_, OpenHelpCenterUrl()).Times(1);
   styled_label_->LinkClicked(nullptr, 0);
 }
+
+IN_PROC_BROWSER_TEST_F(ChooserDialogViewTest, AdapterOnAndOffAndOn) {
+  mock_chooser_controller_->OnAdapterPresenceChanged(
+      MockChooserController::AdapterPresence::ON);
+  EXPECT_FALSE(ok_button_->enabled());
+  EXPECT_TRUE(cancel_button_->enabled());
+
+  // Add options.
+  mock_chooser_controller_->OptionAdded(base::ASCIIToUTF16("a"));
+  mock_chooser_controller_->OptionAdded(base::ASCIIToUTF16("b"));
+  mock_chooser_controller_->OptionAdded(base::ASCIIToUTF16("c"));
+  EXPECT_FALSE(ok_button_->enabled());
+  EXPECT_TRUE(cancel_button_->enabled());
+
+  // Select option 1.
+  table_view_->Select(1);
+  EXPECT_TRUE(ok_button_->enabled());
+  EXPECT_TRUE(cancel_button_->enabled());
+
+  mock_chooser_controller_->OnAdapterPresenceChanged(
+      MockChooserController::AdapterPresence::OFF);
+  // Since the adapter is turned off, the previously selected option

[msw, 2016/07/22 23:37:04]

nit: check the option count here and after the adapter is re-enabled?

[juncai, 2016/07/25 20:14:10]

Done.

+  // becomes invalid, the OK button is disabled.
+  EXPECT_FALSE(ok_button_->enabled());
+  EXPECT_TRUE(cancel_button_->enabled());
+
+  mock_chooser_controller_->OnAdapterPresenceChanged(
+      MockChooserController::AdapterPresence::ON);
+  EXPECT_FALSE(ok_button_->enabled());
+  EXPECT_TRUE(cancel_button_->enabled());
+}
+
+IN_PROC_BROWSER_TEST_F(ChooserDialogViewTest, DiscoveringAndIdle) {
+  // Add options.
+  mock_chooser_controller_->OptionAdded(base::ASCIIToUTF16("a"));
+  mock_chooser_controller_->OptionAdded(base::ASCIIToUTF16("b"));
+  mock_chooser_controller_->OptionAdded(base::ASCIIToUTF16("c"));
+  // |table_view_| is visible.
+  EXPECT_TRUE(table_view_->visible());
+  EXPECT_EQ(3, table_view_->RowCount());
+  EXPECT_EQ(base::ASCIIToUTF16("a"), table_model_->GetText(0, 0));

[msw, 2016/07/22 23:37:04]

nit: Try to avoid adding tangential checks here and elsewhere in all these new
tests; keep each fixture minimal and focused on testing one thing. (This fixture
isn't directed toward checking that the options added are really added with the
correct strings, but maybe there should be such a fixture if one doesn't already
exist?) It's helpful to be thorough by adding many fixtures with unique checks,
but it's not helpful to add repeated tangential checks everywhere.

[juncai, 2016/07/25 20:14:10]

Done.

+  EXPECT_EQ(base::ASCIIToUTF16("b"), table_model_->GetText(1, 0));
+  EXPECT_EQ(base::ASCIIToUTF16("c"), table_model_->GetText(2, 0));
+  EXPECT_TRUE(table_view_->enabled());
+  // No option selected.
+  EXPECT_EQ(0, table_view_->SelectedRowCount());
+  EXPECT_EQ(-1, table_view_->FirstSelectedRow());
+  // Select option 1.
+  table_view_->Select(1);
+  EXPECT_EQ(1, table_view_->SelectedRowCount());
+  EXPECT_EQ(1, table_view_->FirstSelectedRow());
+  // |throbber_| is not visible.
+  EXPECT_FALSE(throbber_->visible());
+  // Both OK and Cancel buttons are enabled.
+  EXPECT_TRUE(ok_button_->enabled());
+  EXPECT_TRUE(cancel_button_->enabled());
+  // |discovery_state_|'s text is an empty string.
+  EXPECT_EQ(base::string16(), discovery_state_->text());
+
+  mock_chooser_controller_->OnDiscoveryStateChanged(
+      MockChooserController::DiscoveryState::DISCOVERING);
+  // |table_view_| is not visible.
+  EXPECT_FALSE(table_view_->visible());
+  // |throbber_| is visible.
+  EXPECT_TRUE(throbber_->visible());
+  // |discovery_state_| is disabled and show a label instead of a link.

[msw, 2016/07/22 23:37:04]

nit: shows

[juncai, 2016/07/25 20:14:10]

Done.

+  EXPECT_FALSE(discovery_state_->enabled());
+  EXPECT_EQ(l10n_util::GetStringUTF16(IDS_BLUETOOTH_DEVICE_CHOOSER_SCANNING),
+            discovery_state_->text());
+  // OK button is disabled since the chooser is refreshing options.
+  EXPECT_FALSE(ok_button_->enabled());
+  EXPECT_TRUE(cancel_button_->enabled());
+
+  mock_chooser_controller_->OnDiscoveryStateChanged(
+      MockChooserController::DiscoveryState::IDLE);
+  // |table_view_| is visible.
+  EXPECT_TRUE(table_view_->visible());
+  // Since "No devices found." needs to be displayed on the |table_view_|,
+  // the number of rows is 1.
+  EXPECT_EQ(1, table_view_->RowCount());
+  EXPECT_EQ(
+      l10n_util::GetStringUTF16(IDS_DEVICE_CHOOSER_NO_DEVICES_FOUND_PROMPT),
+      table_model_->GetText(0, 0));
+  // |table_view_| should be disabled since there is no option shown.
+  EXPECT_FALSE(table_view_->enabled());
+  // No option selected.
+  EXPECT_EQ(0, table_view_->SelectedRowCount());
+  EXPECT_EQ(-1, table_view_->FirstSelectedRow());
+  // |throbber_| is not visible.
+  EXPECT_FALSE(throbber_->visible());
+  // |discovery_state_| is enabled and show a link.

[msw, 2016/07/22 23:37:05]

nit: shows

[juncai, 2016/07/25 20:14:10]

Done.

+  EXPECT_TRUE(discovery_state_->enabled());
+  EXPECT_EQ(l10n_util::GetStringUTF16(IDS_BLUETOOTH_DEVICE_CHOOSER_RE_SCAN),
+            discovery_state_->text());
+  // OK button is disabled since the chooser refreshed options.
+  EXPECT_FALSE(ok_button_->enabled());
+  EXPECT_TRUE(cancel_button_->enabled());
+}