Show device connection and paired status in chooser on Mac

chrome/browser/ui/cocoa/chooser_content_view_cocoa.mm

Index: chrome/browser/ui/cocoa/chooser_content_view_cocoa.mm
diff --git a/chrome/browser/ui/cocoa/chooser_content_view_cocoa.mm b/chrome/browser/ui/cocoa/chooser_content_view_cocoa.mm
index ba6990f50c09c09857a3e7404e918c992c21d06d..3217403d8e9d9b19197824b7aad47f0cb31260f6 100644
--- a/chrome/browser/ui/cocoa/chooser_content_view_cocoa.mm
+++ b/chrome/browser/ui/cocoa/chooser_content_view_cocoa.mm
@@ -12,10 +12,15 @@
 #import "chrome/browser/ui/cocoa/constrained_window/constrained_window_button.h"
 #include "chrome/browser/ui/cocoa/spinner_view.h"
 #include "chrome/grit/generated_resources.h"
+#include "skia/ext/skia_utils_mac.h"
 #import "third_party/google_toolbox_for_mac/src/AppKit/GTMUILocalizerAndLayoutTweaker.h"
 #import "ui/base/cocoa/controls/hyperlink_button_cell.h"
 #include "ui/base/l10n/l10n_util_mac.h"
 #include "ui/base/resource/resource_bundle.h"
+#include "ui/gfx/color_palette.h"
+#include "ui/gfx/image/image_skia_util_mac.h"
+#include "ui/gfx/paint_vector_icon.h"
+#include "ui/gfx/vector_icons_public.h"
 #include "ui/resources/grit/ui_resources.h"
 
 namespace {
@@ -26,11 +31,12 @@ const CGFloat kChooserWidth = 350.0f;
 // Chooser height.
 const CGFloat kChooserHeight = 300.0f;
 
-// Signal strength level image size.
-const CGFloat kSignalStrengthLevelImageSize = 20.0f;
+// Row view image size.
+const CGFloat kRowViewImageSize = 20.0f;
 
 // Table row view height.
-const CGFloat kTableRowViewHeight = 23.0f;
+const CGFloat kTableRowViewOneLineHeight = 23.0f;
+const CGFloat kTableRowViewTwoLinesHeight = 39.0f;
 
 // Spinner size.
 const CGFloat kSpinnerSize = 24.0f;
@@ -50,11 +56,29 @@ const CGFloat kSeparatorAlphaValue = 0.6f;
 // Separator height.
 const CGFloat kSeparatorHeight = 1.0f;
 
+// Distance between two views inside the table row view.
+const CGFloat kTableRowViewHorizontalPadding = 5.0f;
+const CGFloat kTableRowViewVerticalPadding = 1.0f;
+
 // The lookup table for signal strength level image.
 const int kSignalStrengthLevelImageIds[5] = {IDR_SIGNAL_0_BAR, IDR_SIGNAL_1_BAR,
                                              IDR_SIGNAL_2_BAR, IDR_SIGNAL_3_BAR,
                                              IDR_SIGNAL_4_BAR};
 
+// Creates a text field with |text|.

[Jeffrey Yasskin, 2016/09/07 02:01:47]

I think this is a label, rather than a general text field, since it's not
editable or selectable.

[juncai, 2016/09/09 20:06:15]

Done.

+base::scoped_nsobject<NSTextField> CreateTextField(NSString* text) {
+  base::scoped_nsobject<NSTextField> text_field(
+      [[NSTextField alloc] initWithFrame:NSZeroRect]);
+  [text_field setDrawsBackground:NO];

[Jeffrey Yasskin, 2016/09/07 02:01:47]

I hate that we need this long list of setters in order to create a label, but I
don't see any better way to do it in Chrome.

[juncai, 2016/09/09 20:06:15]

Acknowledged.

+  [text_field setBezeled:NO];
+  [text_field setEditable:NO];
+  [text_field setSelectable:NO];
+  [text_field setStringValue:text];
+  [text_field setFont:[NSFont systemFontOfSize:[NSFont systemFontSize]]];
+  [text_field sizeToFit];
+  return text_field;
+}
+
 }  // namespace
 
 // A table row view that contains one line of text, and optionally contains an
@@ -63,11 +87,18 @@ const int kSignalStrengthLevelImageIds[5] = {IDR_SIGNAL_0_BAR, IDR_SIGNAL_1_BAR,
  @private
   base::scoped_nsobject<NSImageView> image_;
   base::scoped_nsobject<NSTextField> text_;
+  base::scoped_nsobject<NSTextField> pairedStatus_;
 }
 
 // Designated initializer.
+// |horizontalPaddingOffset| is used to adjust the origin x of |text_|
+// when the chooser needs to show no devices found message.
 - (instancetype)initWithText:(NSString*)text
-         signalStrengthLevel:(NSInteger)level;
+         signalStrengthLevel:(NSInteger)level
+                 isConnected:(bool)isConnected
+                    isPaired:(bool)isPaired
+                   rowHeight:(CGFloat)rowHeight
+     horizontalPaddingOffset:(CGFloat)horizontalPaddingOffset;
 
 // Gets the image in front of the text.
 - (NSImageView*)image;
@@ -75,46 +106,89 @@ const int kSignalStrengthLevelImageIds[5] = {IDR_SIGNAL_0_BAR, IDR_SIGNAL_1_BAR,
 // Gets the text.
 - (NSTextField*)text;
 
+// Gets the paired status.
+- (NSTextField*)pairedStatus;
+
 @end
 
 @implementation ChooserContentTableRowView
 
 - (instancetype)initWithText:(NSString*)text
-         signalStrengthLevel:(NSInteger)level {
+         signalStrengthLevel:(NSInteger)level
+                 isConnected:(bool)isConnected
+                    isPaired:(bool)isPaired
+                   rowHeight:(CGFloat)rowHeight
+     horizontalPaddingOffset:(CGFloat)horizontalPaddingOffset {
   if ((self = [super initWithFrame:NSZeroRect])) {
-    if (level != -1) {
+    // Create the views.
+    // Image.
+    ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
+    NSImage* image = nullptr;
+    if (isConnected) {
+      image = gfx::NSImageFromImageSkia(gfx::CreateVectorIcon(
+          gfx::VectorIconId::BLUETOOTH_CONNECTED, SK_ColorBLACK));

[Evan Stade, 2016/09/07 16:41:42]

are you sure you want black and not kChromeIconGrey? As the name suggests, it's
the color used for more or less all icons in Chrome.

[juncai, 2016/09/09 20:06:15]

Done.

+    } else if (level != -1) {
       DCHECK_GE(level, 0);
       DCHECK_LT(level, base::checked_cast<NSInteger>(
                            arraysize(kSignalStrengthLevelImageIds)));
-      ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
-      NSImage* signalStrengthLevelImage =
-          rb.GetNativeImageNamed(kSignalStrengthLevelImageIds[level])
-              .ToNSImage();
+      image = rb.GetNativeImageNamed(kSignalStrengthLevelImageIds[level])
+                  .ToNSImage();
+    }
 
+    CGFloat imageOriginX = kTableRowViewHorizontalPadding;
+    CGFloat imageOriginY = (rowHeight - kRowViewImageSize) / 2;
+    if (image) {
       image_.reset([[NSImageView alloc]
-          initWithFrame:NSMakeRect(0, (kTableRowViewHeight -
-                                       kSignalStrengthLevelImageSize) /
-                                          2,
-                                   kSignalStrengthLevelImageSize,
-                                   kSignalStrengthLevelImageSize)]);
-      [image_ setImage:signalStrengthLevelImage];
+          initWithFrame:NSMakeRect(imageOriginX, imageOriginY,
+                                   kRowViewImageSize, kRowViewImageSize)]);
+      [image_ setImage:image];
       [self addSubview:image_];
     }
 
-    text_.reset([[NSTextField alloc] initWithFrame:NSZeroRect]);
-    [text_ setDrawsBackground:NO];
-    [text_ setBezeled:NO];
-    [text_ setEditable:NO];
-    [text_ setSelectable:NO];
-    [text_ setStringValue:text];
-    [text_ setFont:[NSFont systemFontOfSize:[NSFont systemFontSize]]];
-    [text_ sizeToFit];
+    // Text.
+    text_ = CreateTextField(text);
     CGFloat textHeight = NSHeight([text_ frame]);
-    [text_ setFrameOrigin:NSMakePoint(
-                              level == -1 ? 0 : kSignalStrengthLevelImageSize +
-                                                    kHorizontalPadding,
-                              (kTableRowViewHeight - textHeight) / 2)];
+
+    // Paired status.
+    CGFloat pairedStatusHeight = 0.0f;
+    if (isPaired) {
+      pairedStatus_ = CreateTextField(
+          l10n_util::GetNSString(IDS_DEVICE_CHOOSER_PAIRED_STATUS_TEXT));
+      [pairedStatus_
+          setTextColor:skia::SkColorToCalibratedNSColor(gfx::kGoogleGreen700)];
+      pairedStatusHeight = NSHeight([pairedStatus_ frame]);
+    }
+
+    // Lay out the views.
+    // Text.
+    CGFloat textOriginX = imageOriginX + kRowViewImageSize +
+                          kTableRowViewHorizontalPadding +
+                          horizontalPaddingOffset;
+    CGFloat textOriginY;
+    if (isPaired) {
+      textOriginY = pairedStatusHeight +
+                    (rowHeight - textHeight - pairedStatusHeight -
+                     kTableRowViewVerticalPadding) /
+                        2 +
+                    kTableRowViewVerticalPadding;
+    } else {
+      textOriginY = (rowHeight - textHeight) / 2;
+    }
+
+    [text_ setFrameOrigin:NSMakePoint(textOriginX, textOriginY)];
     [self addSubview:text_];
+
+    // Paired status.
+    if (isPaired) {
+      CGFloat pairedStatusOriginX = textOriginX;
+      CGFloat pairedStatusOriginY =
+          (rowHeight - textHeight - pairedStatusHeight -
+           kTableRowViewVerticalPadding) /
+          2;
+      [pairedStatus_
+          setFrameOrigin:NSMakePoint(pairedStatusOriginX, pairedStatusOriginY)];
+      [self addSubview:pairedStatus_];
+    }
   }
 
   return self;
@@ -128,6 +202,10 @@ const int kSignalStrengthLevelImageIds[5] = {IDR_SIGNAL_0_BAR, IDR_SIGNAL_1_BAR,
   return text_.get();
 }
 
+- (NSTextField*)pairedStatus {
+  return pairedStatus_.get();
+}
+
 @end
 
 class ChooserContentViewController : public ChooserController::View {
@@ -310,8 +388,8 @@ void ChooserContentViewController::UpdateTableView() {
     titleHeight_ = NSHeight([titleView_ frame]);
 
     // Status.
-    status_ = [self createTextField:l10n_util::GetNSString(
-                                        IDS_BLUETOOTH_DEVICE_CHOOSER_SCANNING)];
+    status_ = CreateTextField(
+        l10n_util::GetNSString(IDS_BLUETOOTH_DEVICE_CHOOSER_SCANNING));
     CGFloat statusWidth = kChooserWidth / 2 - kMarginX;
     // The height is arbitrary as it will be adjusted later.
     [status_ setFrameSize:NSMakeSize(statusWidth, 0.0f)];
@@ -341,9 +419,8 @@ void ChooserContentViewController::UpdateTableView() {
     separator_ = [self createSeparator];
 
     // Message.
-    message_ = [self createTextField:l10n_util::GetNSStringF(
-                                         IDS_DEVICE_CHOOSER_FOOTNOTE_TEXT,
-                                         base::string16())];
+    message_ = CreateTextField(l10n_util::GetNSStringF(
+        IDS_DEVICE_CHOOSER_FOOTNOTE_TEXT, base::string16()));
     CGFloat messageWidth = NSWidth([message_ frame]);
     messageHeight_ = NSHeight([message_ frame]);
 
@@ -489,25 +566,54 @@ void ChooserContentViewController::UpdateTableView() {
   return titleView;
 }
 
-- (base::scoped_nsobject<NSView>)createTableRowView:(NSInteger)rowIndex {
+- (base::scoped_nsobject<NSView>)createTableRowView:(NSInteger)row {
   NSInteger level = -1;
+  bool isConnected = false;
+  bool isPaired = false;
+  CGFloat horizontalPaddingOffset = 0.0f;
   size_t numOptions = chooserController_->NumOptions();
-  if (chooserController_->ShouldShowIconBeforeText() && numOptions > 0) {
-    DCHECK_GE(rowIndex, 0);
-    DCHECK_LT(rowIndex, base::checked_cast<NSInteger>(numOptions));
-    level = base::checked_cast<NSInteger>(
-        chooserController_->GetSignalStrengthLevel(
-            base::checked_cast<size_t>(rowIndex)));
+  if (numOptions == 0) {

[Jeffrey Yasskin, 2016/09/07 02:01:47]

There are enough special cases for numOptions==0 here that I suspect it'd be
simpler to have a different kind of view for the no-options text, instead of
multiplexing both cases into a single kind of view.

[juncai, 2016/09/09 20:06:15]

If using a different view, it will make the test code more complicated. So here
I added a constructor for class ChooserContentTableRowView that just initialize
its |text_|.

+    DCHECK_EQ(0, row);
+    // Here since the chooser needs to show no devices found message,
+    // the text's origin x needs to be adjusted.
+    horizontalPaddingOffset =
+        -(kRowViewImageSize + kTableRowViewHorizontalPadding);
+  } else {
+    DCHECK_GE(row, 0);
+    DCHECK_LT(row, base::checked_cast<NSInteger>(numOptions));
+    size_t rowIndex = base::checked_cast<size_t>(row);
+    if (chooserController_->ShouldShowIconBeforeText()) {
+      level = base::checked_cast<NSInteger>(
+          chooserController_->GetSignalStrengthLevel(rowIndex));
+    }
+    isConnected = chooserController_->IsConnected(rowIndex);
+    isPaired = chooserController_->IsPaired(rowIndex);
   }
 
   base::scoped_nsobject<NSView> tableRowView([[ChooserContentTableRowView alloc]
-             initWithText:[self optionAtIndex:rowIndex]
-      signalStrengthLevel:level]);
+                 initWithText:[self optionAtIndex:row]
+          signalStrengthLevel:level
+                  isConnected:isConnected
+                     isPaired:isPaired
+                    rowHeight:[self tableRowViewHeight:row]
+      horizontalPaddingOffset:horizontalPaddingOffset]);
   return tableRowView;
 }
 
 - (CGFloat)tableRowViewHeight:(NSInteger)row {
-  return kTableRowViewHeight;
+  size_t numOptions = chooserController_->NumOptions();
+  if (numOptions == 0) {
+    DCHECK_EQ(0, row);
+    return kTableRowViewOneLineHeight;
+  }
+
+  DCHECK_GE(row, 0);
+  DCHECK_LT(row, base::checked_cast<NSInteger>(numOptions));
+  size_t rowIndex = base::checked_cast<size_t>(row);
+  if (chooserController_->IsPaired(rowIndex))

[Jeffrey Yasskin, 2016/09/07 02:01:47]

This part of the calculation should be done inside the
ChooserContentTableRowView:initWithText. 

[juncai, 2016/09/09 20:06:15]

Since
https://cs.chromium.org/chromium/src/chrome/browser/ui/cocoa/website_settings...
needs to implement the heightOfRow function:
https://developer.apple.com/library/mac/documentation/Cocoa/Reference/NSTable...:
to support varying row heights, this function needs to return the appropriate
row height in this function.

[Jeffrey Yasskin, 2016/09/14 20:08:36]

Yep, oh well.

+    return kTableRowViewTwoLinesHeight;
+  else
+    return kTableRowViewOneLineHeight;
 }
 
 - (base::scoped_nsobject<NSButton>)createButtonWithTitle:(NSString*)title {
@@ -540,19 +646,6 @@ void ChooserContentViewController::UpdateTableView() {
   return spacer;
 }
 
-- (base::scoped_nsobject<NSTextField>)createTextField:(NSString*)text {
-  base::scoped_nsobject<NSTextField> textField(
-      [[NSTextField alloc] initWithFrame:NSZeroRect]);
-  [textField setDrawsBackground:NO];
-  [textField setBezeled:NO];
-  [textField setEditable:NO];
-  [textField setSelectable:NO];
-  [textField setStringValue:text];
-  [textField setFont:[NSFont systemFontOfSize:[NSFont systemFontSize]]];
-  [textField sizeToFit];
-  return textField;
-}
-
 - (base::scoped_nsobject<NSButton>)createHyperlinkButtonWithText:
     (NSString*)text {
   base::scoped_nsobject<NSButton> button(
@@ -704,4 +797,10 @@ void ChooserContentViewController::UpdateTableView() {
   return [tableRowView text];
 }
 
+- (NSTextField*)tableRowViewPairedStatus:(NSInteger)row {
+  ChooserContentTableRowView* tableRowView =
+      [tableView_ viewAtColumn:0 row:row makeIfNecessary:YES];
+  return [tableRowView pairedStatus];
+}
+
 @end