Add signal strength indicator icon to WebBluetooth 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 dd9fce5ee84234608c362d86616ec815db820393..f9da3aaf89a78d943ae2ff66587ccdd255d7c535 100644
--- a/chrome/browser/ui/cocoa/chooser_content_view_cocoa.mm
+++ b/chrome/browser/ui/cocoa/chooser_content_view_cocoa.mm
@@ -12,9 +12,11 @@
 #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 "grit/ui_resources.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"
 
 namespace {
 
@@ -24,6 +26,12 @@ const CGFloat kChooserWidth = 350.0f;
 // Chooser height.
 const CGFloat kChooserHeight = 300.0f;
 
+// Signal strength level image size.
+const CGFloat kSignalStrengthLevelImageSize = 20.0f;
+
+// Table row view height.
+const CGFloat kTableRowViewHeight = 23.0f;
+
 // Spinner size.
 const CGFloat kSpinnerSize = 24.0f;
 
@@ -42,8 +50,86 @@ const CGFloat kSeparatorAlphaValue = 0.6f;
 // Separator height.
 const CGFloat kSeparatorHeight = 1.0f;
 
+// The lookup table for signal strength level image.
+const int signalStrengthLevelImageIds[5] = {IDR_SIGNAL_0_BAR, IDR_SIGNAL_1_BAR,

[Jeffrey Yasskin, 2016/08/19 18:30:05]

Use kConstant naming:
https://google.github.io/styleguide/cppguide.html#Constant_Names

[juncai, 2016/08/22 19:18:20]

Done.

+                                            IDR_SIGNAL_2_BAR, IDR_SIGNAL_3_BAR,
+                                            IDR_SIGNAL_4_BAR};
+
 }  // namespace
 
+// A table row view that contains one line of text, and optionally contains an
+// image in front of the text.
+@interface TableRowView : NSView {
+ @private
+  base::scoped_nsobject<NSImageView> image_;
+  base::scoped_nsobject<NSTextField> text_;
+}
+
+// Designated initializer.
+- (instancetype)initWithText:(NSString*)text
+         signalStrengthLevel:(NSInteger)level;
+
+// Gets the image in front of the text.
+- (NSImageView*)image;
+
+// Gets the text.
+- (NSTextField*)text;
+
+@end
+
+@implementation TableRowView
+
+- (instancetype)initWithText:(NSString*)text
+         signalStrengthLevel:(NSInteger)level {
+  if ((self = [super initWithFrame:NSZeroRect])) {
+    if (level != -1) {
+      DCHECK_GE(level, 0);
+      DCHECK_LT(level, base::checked_cast<NSInteger>(
+                           arraysize(signalStrengthLevelImageIds)));
+      ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
+      NSImage* signalStrengthLevelImage =
+          rb.GetNativeImageNamed(signalStrengthLevelImageIds[level])
+              .ToNSImage();
+
+      image_.reset([[NSImageView alloc]
+          initWithFrame:NSMakeRect(0, (kTableRowViewHeight -
+                                       kSignalStrengthLevelImageSize) /
+                                          2,
+                                   kSignalStrengthLevelImageSize,
+                                   kSignalStrengthLevelImageSize)]);
+      [image_ setImage:signalStrengthLevelImage];
+      [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];
+    CGFloat textHeight = NSHeight([text_ frame]);
+    [text_ setFrameOrigin:NSMakePoint(
+                              level == -1 ? 0 : kSignalStrengthLevelImageSize +
+                                                    kVerticalPadding,

[Jeffrey Yasskin, 2016/08/19 18:30:05]

Shouldn't this be kHorizontalPadding?

[juncai, 2016/08/22 19:18:20]

Done.

+                              (kTableRowViewHeight - textHeight) / 2)];
+    [self addSubview:text_];
+  }
+
+  return self;
+}
+
+- (NSImageView*)image {
+  return image_.get();
+}
+
+- (NSTextField*)text {
+  return text_.get();
+}
+
+@end
+
 class ChooserContentViewController : public ChooserController::View {
  public:
   ChooserContentViewController(ChooserContentViewCocoa* chooser_content_view,
@@ -175,7 +261,15 @@ void ChooserContentViewController::OnRefreshStateChanged(bool refreshing) {
 
 void ChooserContentViewController::UpdateTableView() {
   [table_view_ setEnabled:chooser_controller_->NumOptions() > 0];
+  // For NSView-based table views, calling reloadData will deselect the
+  // currently selected row, so |selected_row| stores the currently selected
+  // row in order to select it again.
+  NSInteger selected_row = [table_view_ selectedRow];
   [table_view_ reloadData];
+  if (selected_row != -1) {
+    [table_view_ selectRowIndexes:[NSIndexSet indexSetWithIndex:selected_row]
+             byExtendingSelection:NO];
+  }
 }
 
 @implementation ChooserContentViewCocoa
@@ -395,6 +489,27 @@ void ChooserContentViewController::UpdateTableView() {
   return titleView;
 }
 
+- (NSView*)createTableRowView:(NSInteger)rowIndex {
+  NSInteger level = -1;
+  size_t numOptions = chooserController_->NumOptions();
+  if (chooserController_->ShouldShowIconBeforeText() && numOptions > 0) {

[Jeffrey Yasskin, 2016/08/19 18:30:05]

Dumb question: why is createTableRowView called with a particular rowIndex if
there are no options?

[juncai, 2016/08/22 19:18:20]

If numOptions == 0, the actual number rows is 1 since it needs to display
"Bluetooth devices not found" or "Devices not found" in the table view, so still
need to call createTableRowView to create a row, and a DCHECK checks the
rowIndex is always 0 in this case:
https://cs.chromium.org/chromium/src/chrome/browser/ui/cocoa/chooser_content_...

[Jeffrey Yasskin, 2016/08/22 21:59:24]

Ah, I see. Could you comment that here? Like "This function is called with
numOptions==0 to show the "not found" message."

[juncai, 2016/08/23 00:30:40]

Done.

+    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)));
+  }
+
+  base::scoped_nsobject<TableRowView> tableRowView([[TableRowView alloc]
+             initWithText:[self optionAtIndex:rowIndex]

[Jeffrey Yasskin, 2016/08/19 18:30:05]

This indentation looks kinda weird.

[juncai, 2016/08/22 19:18:20]

I used "git cl format" to format it. I guess here the two ":" need to be
aligned, and "signalStrengthLevel:level]);" has longer string length before the
":" than "initWithText:[self optionAtIndex:rowIndex]", so
signalStrengthLevel:level]); is indented 4 characters.

[Jeffrey Yasskin, 2016/08/22 21:59:24]

If `git cl format` did it, it's fine with me. :)

[juncai, 2016/08/23 00:30:40]

Acknowledged.

+      signalStrengthLevel:level]);
+  return tableRowView.autorelease();
+}
+
+- (CGFloat)tableRowViewHeight:(NSInteger)row {
+  return kTableRowViewHeight;
+}
+
 - (base::scoped_nsobject<NSButton>)createButtonWithTitle:(NSString*)title {
   base::scoped_nsobject<NSButton> button(
       [[ConstrainedWindowButton alloc] initWithFrame:NSZeroRect]);
@@ -575,4 +690,16 @@ void ChooserContentViewController::UpdateTableView() {
   chooserController_->OpenHelpCenterUrl();
 }
 
+- (NSImageView*)tableRowViewImage:(NSInteger)row {
+  TableRowView* tableRowView =
+      [tableView_ viewAtColumn:0 row:row makeIfNecessary:YES];
+  return [tableRowView image];
+}
+
+- (NSTextField*)tableRowViewText:(NSInteger)row {
+  TableRowView* tableRowView =
+      [tableView_ viewAtColumn:0 row:row makeIfNecessary:YES];
+  return [tableRowView text];
+}
+
 @end