Reland: Update image and text color when row is selected in the chooser on Mac

chrome/browser/ui/cocoa/extensions/chooser_dialog_cocoa_controller_unittest.mm

Index: chrome/browser/ui/cocoa/extensions/chooser_dialog_cocoa_controller_unittest.mm
diff --git a/chrome/browser/ui/cocoa/extensions/chooser_dialog_cocoa_controller_unittest.mm b/chrome/browser/ui/cocoa/extensions/chooser_dialog_cocoa_controller_unittest.mm
index 683a74413f45cc3848727bb718a28a1a18961db9..81fd522a0e425ab126acfc7dc837cbded8322834 100644
--- a/chrome/browser/ui/cocoa/extensions/chooser_dialog_cocoa_controller_unittest.mm
+++ b/chrome/browser/ui/cocoa/extensions/chooser_dialog_cocoa_controller_unittest.mm
@@ -17,6 +17,7 @@
 #import "chrome/browser/ui/cocoa/extensions/chooser_dialog_cocoa.h"
 #include "chrome/browser/ui/cocoa/spinner_view.h"
 #include "chrome/grit/generated_resources.h"
+#include "skia/ext/skia_utils_mac.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/gtest_mac.h"
@@ -35,6 +36,10 @@ namespace {
 const int kSignalStrengthLevelImageIds[5] = {IDR_SIGNAL_0_BAR, IDR_SIGNAL_1_BAR,
                                              IDR_SIGNAL_2_BAR, IDR_SIGNAL_3_BAR,
                                              IDR_SIGNAL_4_BAR};
+const int kSignalStrengthLevelImageSelectedIds[5] = {
+    IDR_SIGNAL_0_BAR_SELECTED, IDR_SIGNAL_1_BAR_SELECTED,
+    IDR_SIGNAL_2_BAR_SELECTED, IDR_SIGNAL_3_BAR_SELECTED,
+    IDR_SIGNAL_4_BAR_SELECTED};
 
 }  // namespace
 
@@ -89,24 +94,27 @@ class ChooserDialogCocoaControllerTest : public CocoaProfileTest {
   }
 
   void ExpectSignalStrengthLevelImageIs(int row,
-                                        int expected_signal_strength_level) {
+                                        int expected_signal_strength_level,
+                                        int expected_color) {
     NSImageView* image_view =
         [chooser_content_view_ tableRowViewImage:static_cast<NSInteger>(row)];
     ASSERT_TRUE(image_view);
-    EXPECT_NSEQ(
-        rb_.GetNativeImageNamed(
-               kSignalStrengthLevelImageIds[expected_signal_strength_level])
-            .ToNSImage(),
-        [image_view image]);
+    int image_id =
+        expected_color == MockChooserController::kImageColorBlack
+            ? kSignalStrengthLevelImageIds[expected_signal_strength_level]
+            : kSignalStrengthLevelImageSelectedIds
+                  [expected_signal_strength_level];
+    EXPECT_NSEQ(rb_.GetNativeImageNamed(image_id).ToNSImage(),
+                [image_view image]);
   }
 
-  void ExpectRowImageIsConnectedImage(int row) {
+  void ExpectRowImageIsConnectedImage(int row, SkColor expected_color) {
     NSImageView* image_view =
         [chooser_content_view_ tableRowViewImage:static_cast<NSInteger>(row)];
     ASSERT_TRUE(image_view);
     EXPECT_TRUE(gfx::test::AreImagesEqual(
         gfx::Image(gfx::CreateVectorIcon(gfx::VectorIconId::BLUETOOTH_CONNECTED,
-                                         gfx::kChromeIconGrey)),
+                                         expected_color)),
         gfx::Image([[image_view image] copy])));
   }
 
@@ -116,6 +124,12 @@ class ChooserDialogCocoaControllerTest : public CocoaProfileTest {
                     tableRowViewText:static_cast<NSInteger>(row)] stringValue]);
   }
 
+  void ExpectRowTextColorIs(int row, NSColor* expected_color) {
+    EXPECT_NSEQ(expected_color,
+                [[chooser_content_view_
+                    tableRowViewText:static_cast<NSInteger>(row)] textColor]);
+  }
+
   bool IsRowPaired(int row) {
     NSTextField* paired_status = [chooser_content_view_
         tableRowViewPairedStatus:static_cast<NSInteger>(row)];
@@ -128,6 +142,13 @@ class ChooserDialogCocoaControllerTest : public CocoaProfileTest {
     }
   }
 
+  void ExpectPairedTextColorIs(int row, NSColor* expected_color) {
+    EXPECT_NSEQ(
+        expected_color,
+        [[chooser_content_view_
+            tableRowViewPairedStatus:static_cast<NSInteger>(row)] textColor]);
+  }
+
   ui::ResourceBundle& rb_;
 
   std::unique_ptr<ChooserDialogCocoa> chooser_dialog_;
@@ -182,7 +203,7 @@ TEST_F(ChooserDialogCocoaControllerTest, AddOption) {
   // |table_view_| should be enabled since there is an option.
   EXPECT_TRUE(table_view_.enabled);
   EXPECT_EQ(-1, table_view_.selectedRow);
-  ExpectRowImageIsConnectedImage(0);
+  ExpectRowImageIsConnectedImage(0, gfx::kChromeIconGrey);
   ExpectRowTextIs(0, @"a");
   EXPECT_TRUE(IsRowPaired(0));
   EXPECT_FALSE(connect_button_.enabled);
@@ -197,7 +218,8 @@ TEST_F(ChooserDialogCocoaControllerTest, AddOption) {
   EXPECT_TRUE(table_view_.enabled);
   EXPECT_EQ(-1, table_view_.selectedRow);
   ExpectSignalStrengthLevelImageIs(
-      1, MockChooserController::kSignalStrengthLevel0Bar);
+      1, MockChooserController::kSignalStrengthLevel0Bar,
+      MockChooserController::kImageColorBlack);
   ExpectRowTextIs(1, @"b");
   EXPECT_FALSE(IsRowPaired(1));
 
@@ -209,7 +231,8 @@ TEST_F(ChooserDialogCocoaControllerTest, AddOption) {
   EXPECT_TRUE(table_view_.enabled);
   EXPECT_EQ(-1, table_view_.selectedRow);
   ExpectSignalStrengthLevelImageIs(
-      2, MockChooserController::kSignalStrengthLevel1Bar);
+      2, MockChooserController::kSignalStrengthLevel1Bar,
+      MockChooserController::kImageColorBlack);
   ExpectRowTextIs(2, @"c");
   EXPECT_FALSE(IsRowPaired(2));
 
@@ -217,7 +240,8 @@ TEST_F(ChooserDialogCocoaControllerTest, AddOption) {
       base::ASCIIToUTF16("d"), MockChooserController::kSignalStrengthLevel2Bar,
       MockChooserController::ConnectedPairedStatus::NONE);
   ExpectSignalStrengthLevelImageIs(
-      3, MockChooserController::kSignalStrengthLevel2Bar);
+      3, MockChooserController::kSignalStrengthLevel2Bar,
+      MockChooserController::kImageColorBlack);
   ExpectRowTextIs(3, @"d");
   EXPECT_FALSE(IsRowPaired(3));
 
@@ -225,7 +249,8 @@ TEST_F(ChooserDialogCocoaControllerTest, AddOption) {
       base::ASCIIToUTF16("e"), MockChooserController::kSignalStrengthLevel3Bar,
       MockChooserController::ConnectedPairedStatus::NONE);
   ExpectSignalStrengthLevelImageIs(
-      4, MockChooserController::kSignalStrengthLevel3Bar);
+      4, MockChooserController::kSignalStrengthLevel3Bar,
+      MockChooserController::kImageColorBlack);
   ExpectRowTextIs(4, @"e");
   EXPECT_FALSE(IsRowPaired(4));
 
@@ -233,7 +258,8 @@ TEST_F(ChooserDialogCocoaControllerTest, AddOption) {
       base::ASCIIToUTF16("f"), MockChooserController::kSignalStrengthLevel4Bar,
       MockChooserController::ConnectedPairedStatus::NONE);
   ExpectSignalStrengthLevelImageIs(
-      5, MockChooserController::kSignalStrengthLevel4Bar);
+      5, MockChooserController::kSignalStrengthLevel4Bar,
+      MockChooserController::kImageColorBlack);
   ExpectRowTextIs(5, @"f");
   EXPECT_FALSE(IsRowPaired(5));
 }
@@ -258,11 +284,12 @@ TEST_F(ChooserDialogCocoaControllerTest, RemoveOption) {
   EXPECT_EQ(1, table_view_.numberOfColumns);
   EXPECT_TRUE(table_view_.enabled);
   EXPECT_EQ(-1, table_view_.selectedRow);
-  ExpectRowImageIsConnectedImage(0);
+  ExpectRowImageIsConnectedImage(0, gfx::kChromeIconGrey);
   ExpectRowTextIs(0, @"a");
   EXPECT_TRUE(IsRowPaired(0));
   ExpectSignalStrengthLevelImageIs(
-      1, MockChooserController::kSignalStrengthLevel1Bar);
+      1, MockChooserController::kSignalStrengthLevel1Bar,
+      MockChooserController::kImageColorBlack);
   ExpectRowTextIs(1, @"c");
   EXPECT_FALSE(IsRowPaired(1));
 
@@ -321,14 +348,15 @@ TEST_F(ChooserDialogCocoaControllerTest, UpdateOption) {
   EXPECT_EQ(1, table_view_.numberOfColumns);
   EXPECT_TRUE(table_view_.enabled);
   EXPECT_EQ(-1, table_view_.selectedRow);
-  ExpectRowImageIsConnectedImage(0);
+  ExpectRowImageIsConnectedImage(0, gfx::kChromeIconGrey);
   ExpectRowTextIs(0, @"a");
   EXPECT_TRUE(IsRowPaired(0));
-  ExpectRowImageIsConnectedImage(1);
+  ExpectRowImageIsConnectedImage(1, gfx::kChromeIconGrey);
   ExpectRowTextIs(1, @"d");
   EXPECT_TRUE(IsRowPaired(1));
   ExpectSignalStrengthLevelImageIs(
-      2, MockChooserController::kSignalStrengthLevel1Bar);
+      2, MockChooserController::kSignalStrengthLevel1Bar,
+      MockChooserController::kImageColorBlack);
   ExpectRowTextIs(2, @"c");
   EXPECT_FALSE(IsRowPaired(2));
 }
@@ -389,15 +417,188 @@ TEST_F(ChooserDialogCocoaControllerTest, UpdateAndRemoveTheUpdatedOption) {
   EXPECT_EQ(1, table_view_.numberOfColumns);
   EXPECT_TRUE(table_view_.enabled);
   EXPECT_EQ(-1, table_view_.selectedRow);
-  ExpectRowImageIsConnectedImage(0);
+  ExpectRowImageIsConnectedImage(0, gfx::kChromeIconGrey);
   ExpectRowTextIs(0, @"a");
   EXPECT_TRUE(IsRowPaired(0));
   ExpectSignalStrengthLevelImageIs(
-      1, MockChooserController::kSignalStrengthLevel1Bar);
+      1, MockChooserController::kSignalStrengthLevel1Bar,
+      MockChooserController::kImageColorBlack);
   ExpectRowTextIs(1, @"c");
   EXPECT_FALSE(IsRowPaired(1));
 }
 
+TEST_F(ChooserDialogCocoaControllerTest,
+       RowImageAndTextChangeColorWhenSelectionChanges) {
+  CreateChooserDialog();
+
+  mock_chooser_controller_->OptionAdded(
+      base::ASCIIToUTF16("a"),
+      MockChooserController::kNoSignalStrengthLevelImage,
+      MockChooserController::ConnectedPairedStatus::CONNECTED |
+          MockChooserController::ConnectedPairedStatus::PAIRED);
+  mock_chooser_controller_->OptionAdded(
+      base::ASCIIToUTF16("b"), MockChooserController::kSignalStrengthLevel0Bar,
+      MockChooserController::ConnectedPairedStatus::NONE);
+  mock_chooser_controller_->OptionAdded(
+      base::ASCIIToUTF16("c"), MockChooserController::kSignalStrengthLevel1Bar,
+      MockChooserController::ConnectedPairedStatus::NONE);
+
+  ExpectRowImageIsConnectedImage(0, gfx::kChromeIconGrey);
+  ExpectSignalStrengthLevelImageIs(
+      1, MockChooserController::kSignalStrengthLevel0Bar,
+      MockChooserController::kImageColorBlack);
+  ExpectSignalStrengthLevelImageIs(
+      2, MockChooserController::kSignalStrengthLevel1Bar,
+      MockChooserController::kImageColorBlack);
+  ExpectRowTextColorIs(0, [NSColor blackColor]);
+  ExpectRowTextColorIs(1, [NSColor blackColor]);
+  ExpectRowTextColorIs(2, [NSColor blackColor]);
+  ExpectPairedTextColorIs(
+      0, skia::SkColorToCalibratedNSColor(gfx::kGoogleGreen700));
+
+  // Select option 0.
+  [table_view_ selectRowIndexes:[NSIndexSet indexSetWithIndex:0]
+           byExtendingSelection:NO];
+  EXPECT_EQ(0, table_view_.selectedRow);
+  ExpectRowImageIsConnectedImage(0, SK_ColorWHITE);
+  ExpectSignalStrengthLevelImageIs(
+      1, MockChooserController::kSignalStrengthLevel0Bar,
+      MockChooserController::kImageColorBlack);
+  ExpectSignalStrengthLevelImageIs(
+      2, MockChooserController::kSignalStrengthLevel1Bar,
+      MockChooserController::kImageColorBlack);
+  ExpectRowTextColorIs(0, [NSColor whiteColor]);
+  ExpectRowTextColorIs(1, [NSColor blackColor]);
+  ExpectRowTextColorIs(2, [NSColor blackColor]);
+  ExpectPairedTextColorIs(
+      0, skia::SkColorToCalibratedNSColor(gfx::kGoogleGreen300));
+
+  // Deselect option 0.
+  [table_view_ deselectRow:0];
+  EXPECT_EQ(-1, table_view_.selectedRow);
+  ExpectRowImageIsConnectedImage(0, gfx::kChromeIconGrey);
+  ExpectSignalStrengthLevelImageIs(
+      1, MockChooserController::kSignalStrengthLevel0Bar,
+      MockChooserController::kImageColorBlack);
+  ExpectSignalStrengthLevelImageIs(
+      2, MockChooserController::kSignalStrengthLevel1Bar,
+      MockChooserController::kImageColorBlack);
+  ExpectRowTextColorIs(0, [NSColor blackColor]);
+  ExpectRowTextColorIs(1, [NSColor blackColor]);
+  ExpectRowTextColorIs(2, [NSColor blackColor]);
+  ExpectPairedTextColorIs(
+      0, skia::SkColorToCalibratedNSColor(gfx::kGoogleGreen700));
+
+  // Select option 1.

[Jeffrey Yasskin, 2016/09/26 22:25:46]

In these comments, say why you're doing the operation, not just what operation
you're doing. Is this here to test that there's no special case for the 0th
option?

[juncai, 2016/09/27 22:10:48]

Since option 0 shows a Bluetooth connected image and option 1 and 2 show a
signal strength level image, need to test in both cases the image color changes
when selected/deselected.

I added comments to clarify it.

Done.

+  [table_view_ selectRowIndexes:[NSIndexSet indexSetWithIndex:1]
+           byExtendingSelection:NO];
+  EXPECT_EQ(1, table_view_.selectedRow);
+  ExpectRowImageIsConnectedImage(0, gfx::kChromeIconGrey);
+  ExpectSignalStrengthLevelImageIs(
+      1, MockChooserController::kSignalStrengthLevel0Bar,
+      MockChooserController::kImageColorWhite);
+  ExpectSignalStrengthLevelImageIs(
+      2, MockChooserController::kSignalStrengthLevel1Bar,
+      MockChooserController::kImageColorBlack);
+  ExpectRowTextColorIs(0, [NSColor blackColor]);
+  ExpectRowTextColorIs(1, [NSColor whiteColor]);
+  ExpectRowTextColorIs(2, [NSColor blackColor]);
+  ExpectPairedTextColorIs(
+      0, skia::SkColorToCalibratedNSColor(gfx::kGoogleGreen700));
+
+  // Deselect option 1.
+  [table_view_ deselectRow:1];
+  EXPECT_EQ(-1, table_view_.selectedRow);
+  ExpectRowImageIsConnectedImage(0, gfx::kChromeIconGrey);
+  ExpectSignalStrengthLevelImageIs(
+      1, MockChooserController::kSignalStrengthLevel0Bar,
+      MockChooserController::kImageColorBlack);
+  ExpectSignalStrengthLevelImageIs(
+      2, MockChooserController::kSignalStrengthLevel1Bar,
+      MockChooserController::kImageColorBlack);
+  ExpectRowTextColorIs(0, [NSColor blackColor]);
+  ExpectRowTextColorIs(1, [NSColor blackColor]);
+  ExpectRowTextColorIs(2, [NSColor blackColor]);
+  ExpectPairedTextColorIs(
+      0, skia::SkColorToCalibratedNSColor(gfx::kGoogleGreen700));
+
+  // Select option 0.

[Jeffrey Yasskin, 2016/09/26 22:25:46]

You already tested that selecting option 0 has the expected effect, so it's not
clear what this repetition adds. Say what it's for in the comment, or if there's
no particular code or potential behavior that it's checking for, you should omit
it.

[juncai, 2016/09/27 22:10:48]

Done.

+  [table_view_ selectRowIndexes:[NSIndexSet indexSetWithIndex:0]
+           byExtendingSelection:NO];
+  EXPECT_EQ(0, table_view_.selectedRow);
+  ExpectRowImageIsConnectedImage(0, SK_ColorWHITE);
+  ExpectSignalStrengthLevelImageIs(
+      1, MockChooserController::kSignalStrengthLevel0Bar,
+      MockChooserController::kImageColorBlack);
+  ExpectSignalStrengthLevelImageIs(
+      2, MockChooserController::kSignalStrengthLevel1Bar,
+      MockChooserController::kImageColorBlack);
+  ExpectRowTextColorIs(0, [NSColor whiteColor]);
+  ExpectRowTextColorIs(1, [NSColor blackColor]);
+  ExpectRowTextColorIs(2, [NSColor blackColor]);
+  ExpectPairedTextColorIs(
+      0, skia::SkColorToCalibratedNSColor(gfx::kGoogleGreen300));
+
+  // Select option 1.
+  [table_view_ selectRowIndexes:[NSIndexSet indexSetWithIndex:1]
+           byExtendingSelection:NO];
+  EXPECT_EQ(1, table_view_.selectedRow);
+  ExpectRowImageIsConnectedImage(0, gfx::kChromeIconGrey);
+  ExpectSignalStrengthLevelImageIs(
+      1, MockChooserController::kSignalStrengthLevel0Bar,
+      MockChooserController::kImageColorWhite);
+  ExpectSignalStrengthLevelImageIs(
+      2, MockChooserController::kSignalStrengthLevel1Bar,
+      MockChooserController::kImageColorBlack);
+  ExpectRowTextColorIs(0, [NSColor blackColor]);
+  ExpectRowTextColorIs(1, [NSColor whiteColor]);
+  ExpectRowTextColorIs(2, [NSColor blackColor]);
+  ExpectPairedTextColorIs(
+      0, skia::SkColorToCalibratedNSColor(gfx::kGoogleGreen700));
+
+  // Select option 2.
+  [table_view_ selectRowIndexes:[NSIndexSet indexSetWithIndex:2]
+           byExtendingSelection:NO];
+  EXPECT_EQ(2, table_view_.selectedRow);
+  ExpectRowImageIsConnectedImage(0, gfx::kChromeIconGrey);
+  ExpectSignalStrengthLevelImageIs(
+      1, MockChooserController::kSignalStrengthLevel0Bar,
+      MockChooserController::kImageColorBlack);
+  ExpectSignalStrengthLevelImageIs(
+      2, MockChooserController::kSignalStrengthLevel1Bar,
+      MockChooserController::kImageColorWhite);
+  ExpectRowTextColorIs(0, [NSColor blackColor]);
+  ExpectRowTextColorIs(1, [NSColor blackColor]);
+  ExpectRowTextColorIs(2, [NSColor whiteColor]);
+  ExpectPairedTextColorIs(
+      0, skia::SkColorToCalibratedNSColor(gfx::kGoogleGreen700));
+
+  // Update option 2.
+  mock_chooser_controller_->OptionUpdated(
+      base::ASCIIToUTF16("c"), base::ASCIIToUTF16("e"),
+      MockChooserController::kSignalStrengthLevel2Bar,
+      MockChooserController::ConnectedPairedStatus::NONE);
+  ExpectSignalStrengthLevelImageIs(
+      2, MockChooserController::kSignalStrengthLevel2Bar,
+      MockChooserController::kImageColorWhite);
+  ExpectRowTextColorIs(0, [NSColor blackColor]);
+  ExpectRowTextColorIs(1, [NSColor blackColor]);
+  ExpectRowTextColorIs(2, [NSColor whiteColor]);
+
+  // Update option 2 again.
+  mock_chooser_controller_->OptionUpdated(
+      base::ASCIIToUTF16("e"), base::ASCIIToUTF16("f"),
+      MockChooserController::kNoSignalStrengthLevelImage,
+      MockChooserController::ConnectedPairedStatus::CONNECTED |
+          MockChooserController::ConnectedPairedStatus::PAIRED);
+  ExpectRowImageIsConnectedImage(2, SK_ColorWHITE);
+  ExpectRowTextColorIs(0, [NSColor blackColor]);
+  ExpectRowTextColorIs(1, [NSColor blackColor]);
+  ExpectRowTextColorIs(2, [NSColor whiteColor]);
+  ExpectPairedTextColorIs(
+      2, skia::SkColorToCalibratedNSColor(gfx::kGoogleGreen300));
+}
+
 TEST_F(ChooserDialogCocoaControllerTest, SelectAndDeselectAnOption) {
   CreateChooserDialog();
 
@@ -567,14 +768,15 @@ TEST_F(ChooserDialogCocoaControllerTest,
           MockChooserController::ConnectedPairedStatus::PAIRED);
 
   EXPECT_EQ(1, table_view_.selectedRow);
-  ExpectRowImageIsConnectedImage(0);
+  ExpectRowImageIsConnectedImage(0, gfx::kChromeIconGrey);
   ExpectRowTextIs(0, @"a");
   EXPECT_TRUE(IsRowPaired(0));
-  ExpectRowImageIsConnectedImage(1);
+  ExpectRowImageIsConnectedImage(1, SK_ColorWHITE);
   ExpectRowTextIs(1, @"d");
   EXPECT_TRUE(IsRowPaired(1));
   ExpectSignalStrengthLevelImageIs(
-      2, MockChooserController::kSignalStrengthLevel1Bar);
+      2, MockChooserController::kSignalStrengthLevel1Bar,
+      MockChooserController::kImageColorBlack);
   ExpectRowTextIs(2, @"c");
   EXPECT_FALSE(IsRowPaired(2));
   EXPECT_TRUE(connect_button_.enabled);
@@ -855,7 +1057,8 @@ TEST_F(ChooserDialogCocoaControllerTest,
   // No option selected.
   EXPECT_EQ(-1, table_view_.selectedRow);
   ExpectSignalStrengthLevelImageIs(
-      0, MockChooserController::kSignalStrengthLevel2Bar);
+      0, MockChooserController::kSignalStrengthLevel2Bar,
+      MockChooserController::kImageColorBlack);
   ExpectRowTextIs(0, @"d");
   EXPECT_FALSE(IsRowPaired(0));
   EXPECT_TRUE(spinner_.hidden);