[AiS] changing mac omnibox suggestions form NSMatrix to NSTableView

chrome/browser/ui/cocoa/omnibox/omnibox_popup_view_mac.mm

Index: chrome/browser/ui/cocoa/omnibox/omnibox_popup_view_mac.mm
diff --git a/chrome/browser/ui/cocoa/omnibox/omnibox_popup_view_mac.mm b/chrome/browser/ui/cocoa/omnibox/omnibox_popup_view_mac.mm
index 577a9392f2401cb7de20a158dffe14aa1ebf001f..30135da7c7aa02a8d108b3b62377bb49f6eab399 100644
--- a/chrome/browser/ui/cocoa/omnibox/omnibox_popup_view_mac.mm
+++ b/chrome/browser/ui/cocoa/omnibox/omnibox_popup_view_mac.mm
@@ -29,11 +29,7 @@
 
 namespace {
 
-// How much to adjust the cell sizing up from the default determined
-// by the font.
-const CGFloat kCellHeightAdjust = 6.0;

[dschuyler, 2015/04/24 18:43:09]

This moved to another file in this CL.

-
-// Padding between matrix and the top and bottom of the popup window.
+// Padding between table and the top and bottom of the popup window.
 const CGFloat kPopupPaddingVertical = 5.0;
 
 // Animation duration when animating the popup window smaller.
@@ -65,13 +61,19 @@ OmniboxPopupViewMac::~OmniboxPopupViewMac() {
 
   // Break references to |this| because the popup may not be
   // deallocated immediately.
-  [matrix_ setDelegate:NULL];
+  [table_view_ setCppDelegate:NULL];
 }
 
 bool OmniboxPopupViewMac::IsOpen() const {
   return popup_ != nil;
 }
 
+void OmniboxPopupViewMac::InvalidateLine(size_t line) {
+  // Update the selection state so that it's drawn properly later.
+  [[table_view_ cellAtRow:line]
+      setState:(line == model_->selected_line() ? NSOnState : NSOffState)];
+}
+
 void OmniboxPopupViewMac::UpdatePopupAppearance() {
   DCHECK([NSThread isMainThread]);
   const AutocompleteResult& result = GetResult();
@@ -83,8 +85,8 @@ void OmniboxPopupViewMac::UpdatePopupAppearance() {
 
     // Break references to |this| because the popup may not be
     // deallocated immediately.
-    [matrix_ setDelegate:nil];
-    matrix_.reset();
+    [table_view_ setCppDelegate:NULL];
+    table_view_.reset();
 
     popup_.reset(nil);
 
@@ -95,18 +97,14 @@ void OmniboxPopupViewMac::UpdatePopupAppearance() {
 
   CreatePopupIfNeeded();
 
-  // Calculate the width of the matrix based on backing out the popup's border
-  // from the width of the field.
-  const CGFloat matrix_width = NSWidth([field_ bounds]);
-  DCHECK_GT(matrix_width, 0.0);

[dschuyler, 2015/04/24 18:43:09]

This is moved elsewhere in this file in this CL.

+  [table_view_ setCellCount:rows];
 
-  // Load the results into the popup's matrix.
+  // Load the results into the popup's table.
   DCHECK_GT(rows, 0U);
-  [matrix_ renewRows:rows columns:1];
   CGFloat max_match_contents_width = 0.0f;
   CGFloat contents_offset = -1.0f;
   for (size_t ii = 0; ii < rows; ++ii) {
-    OmniboxPopupCell* cell = [matrix_ cellAtRow:ii column:0];
+    OmniboxPopupCell* cell = [table_view_ cellAtRow:ii];
     const AutocompleteMatch& match = GetResult().match_at(ii + start_match);
     [cell setImage:ImageForMatch(match)];
     [cell setMatch:match];
@@ -119,28 +117,26 @@ void OmniboxPopupViewMac::UpdatePopupAppearance() {
       [cell setContentsOffset:contents_offset];
     }
   }
+  [table_view_ noteNumberOfRowsChanged];
 
+  NSInteger popupHeight = 0;
   for (size_t ii = 0; ii < rows; ++ii) {
-    OmniboxPopupCell* cell = [matrix_ cellAtRow:ii column:0];
+    OmniboxPopupCell* cell = [table_view_ cellAtRow:ii];
     [cell setMaxMatchContentsWidth:max_match_contents_width];
+    popupHeight += [cell cellSize].height;
+  }
+  if (rows) {
+    popupHeight += [table_view_ intercellSpacing].height * (rows - 1);

[dschuyler, 2015/04/24 18:43:09]

This isn't strictly necessary.  Right now the spacing is always zero.  Though
this will be needed in the spacing is changed (and maybe it should be instead of
using kCellHeightAdjust to alter the frame by hand).

   }
-
-  // Set the cell size to fit a line of text in the cell's font.  All
-  // cells should use the same font and each should layout in one
-  // line, so they should all be about the same height.
-  const NSSize cell_size = [[matrix_ cellAtRow:0 column:0] cellSize];
-  DCHECK_GT(cell_size.height, 0.0);
-  const CGFloat cell_height = cell_size.height + kCellHeightAdjust;
-  [matrix_ setCellSize:NSMakeSize(matrix_width, cell_height)];
 
   // Update the selection before placing (and displaying) the window.
   PaintUpdatesNow();
 
-  // Calculate the matrix size manually rather than using -sizeToCells
-  // because actually resizing the matrix messed up the popup size
+  // Calculate the table size manually rather than using -sizeToCells
+  // because actually resizing the table messed up the popup size
   // animation.
-  DCHECK_EQ([matrix_ intercellSpacing].height, 0.0);
-  PositionPopup(rows * cell_height);
+  DCHECK_EQ([table_view_ intercellSpacing].height, 0.0);
+  PositionPopup(popupHeight);
 }
 
 gfx::Rect OmniboxPopupViewMac::GetTargetBounds() {
@@ -156,28 +152,29 @@ gfx::Rect OmniboxPopupViewMac::GetTargetBounds() {
 // everything.  Popup should already be visible.
 void OmniboxPopupViewMac::PaintUpdatesNow() {
   size_t start_match = model_->result().ShouldHideTopMatch() ? 1 : 0;
-  if (start_match > model_->selected_line()) {
-    [matrix_ deselectAllCells];
-  } else {
-    [matrix_ selectCellAtRow:model_->selected_line() - start_match column:0];
+  NSIndexSet* indexSet = [NSIndexSet indexSet];
+  if (start_match <= model_->selected_line()) {
+    indexSet =
+        [NSIndexSet indexSetWithIndex:model_->selected_line() - start_match];
   }
-
+  [table_view_ selectRowIndexes:indexSet byExtendingSelection:NO];
 }
 
-void OmniboxPopupViewMac::OnMatrixRowSelected(OmniboxPopupMatrix* matrix,
-                                              size_t row) {
+void OmniboxPopupViewMac::OnTableRowSelected(OmniboxPopupTableView* table_view,
+                                             size_t row) {
   size_t start_match = model_->result().ShouldHideTopMatch() ? 1 : 0;
   model_->SetSelectedLine(row + start_match, false, false);
 }
 
-void OmniboxPopupViewMac::OnMatrixRowClicked(OmniboxPopupMatrix* matrix,
-                                             size_t row) {
+void OmniboxPopupViewMac::OnTableRowClicked(OmniboxPopupTableView* table_view,
+                                            size_t row) {
   OpenURLForRow(row,
                 ui::WindowOpenDispositionFromNSEvent([NSApp currentEvent]));
 }
 
-void OmniboxPopupViewMac::OnMatrixRowMiddleClicked(OmniboxPopupMatrix* matrix,
-                                                   size_t row) {
+void OmniboxPopupViewMac::OnTableRowMiddleClicked(
+    OmniboxPopupTableView* table_view,
+    size_t row) {
   OpenURLForRow(row, NEW_BACKGROUND_TAB);
 }
 
@@ -195,13 +192,13 @@ void OmniboxPopupViewMac::CreatePopupIfNeeded() {
     [popup_ setBackgroundColor:[NSColor clearColor]];
     [popup_ setOpaque:NO];
 
-    // Use a flipped view to pin the matrix top the top left. This is needed
+    // Use a flipped view to pin the table view top the top left. This is needed
     // for animated resize.
     base::scoped_nsobject<FlippedView> contentView(
         [[FlippedView alloc] initWithFrame:NSZeroRect]);
     [popup_ setContentView:contentView];
 
-    // View to draw a background beneath the matrix.
+    // View to draw a background beneath the table.
     background_view_.reset([[NSBox alloc] initWithFrame:NSZeroRect]);
     [background_view_ setBoxType:NSBoxCustom];
     [background_view_ setBorderType:NSNoBorder];
@@ -209,8 +206,8 @@ void OmniboxPopupViewMac::CreatePopupIfNeeded() {
     [background_view_ setContentViewMargins:NSZeroSize];
     [contentView addSubview:background_view_];
 
-    matrix_.reset([[OmniboxPopupMatrix alloc] initWithDelegate:this]);
-    [background_view_ addSubview:matrix_];
+    table_view_.reset([[OmniboxPopupTableView alloc] initWithDelegate:this]);
+    [background_view_ addSubview:table_view_];
 
     top_separator_view_.reset(
         [[OmniboxPopupTopSeparatorView alloc] initWithFrame:NSZeroRect]);
@@ -226,15 +223,15 @@ void OmniboxPopupViewMac::CreatePopupIfNeeded() {
   }
 }
 
-void OmniboxPopupViewMac::PositionPopup(const CGFloat matrixHeight) {
+void OmniboxPopupViewMac::PositionPopup(const CGFloat tableHeight) {
   BrowserWindowController* controller =
       [BrowserWindowController browserWindowControllerForView:field_];
   NSRect anchor_rect_base = [controller omniboxPopupAnchorRect];
 
   // Calculate the popup's position on the screen.
   NSRect popup_frame = anchor_rect_base;
-  // Size to fit the matrix and shift down by the size.
-  popup_frame.size.height = matrixHeight + kPopupPaddingVertical * 2.0;
+  // Size to fit the table view and shift down by the size.
+  popup_frame.size.height = tableHeight + kPopupPaddingVertical * 2.0;
   popup_frame.size.height += [OmniboxPopupTopSeparatorView preferredHeight];
   popup_frame.size.height += [OmniboxPopupBottomSeparatorView preferredHeight];
   popup_frame.origin.y -= NSHeight(popup_frame);
@@ -243,8 +240,8 @@ void OmniboxPopupViewMac::PositionPopup(const CGFloat matrixHeight) {
       [[controller window] convertBaseToScreen:popup_frame.origin];
 
   // Do nothing if the popup is already animating to the given |frame|.
-  if (NSEqualRects(popup_frame, target_popup_frame_))
-    return;
+  // if (NSEqualRects(popup_frame, target_popup_frame_))
+  //   return;

[dschuyler, 2015/04/24 18:43:09]

This short circuit causes a problem with the highlight background widths.  I
thought about just removing it, but it's still here while I re-think whether
there is merit in converting this to work with the TableView.  What are your
thoughts on it? 

[groby-ooo-7-16, 2015/04/24 20:11:17]

I'm not sure what exactly this does, or why removing it is helpful, so it's hard
to make that call :)

 
   // Top separator.
   NSRect top_separator_frame = NSZeroRect;
@@ -270,15 +267,19 @@ void OmniboxPopupViewMac::PositionPopup(const CGFloat matrixHeight) {
   background_rect.origin.y = NSMaxY(top_separator_frame);
   [background_view_ setFrame:background_rect];
 
-  // Matrix.
+  // Calculate the width of the table based on backing out the popup's border
+  // from the width of the field.
+  const CGFloat tableWidth = NSWidth([field_ bounds]);
+  DCHECK_GT(tableWidth, 0.0);

[dschuyler, 2015/04/24 18:43:08]

This is moved here from elsewhere in the code, in this CL.

+
   NSPoint field_origin_base =
       [field_ convertPoint:[field_ bounds].origin toView:nil];
-  NSRect matrix_frame = NSZeroRect;
-  matrix_frame.origin.x = field_origin_base.x - NSMinX(anchor_rect_base);
-  matrix_frame.origin.y = kPopupPaddingVertical;
-  matrix_frame.size.width = [matrix_ cellSize].width;
-  matrix_frame.size.height = matrixHeight;
-  [matrix_ setFrame:matrix_frame];
+  NSRect table_frame = NSZeroRect;
+  table_frame.origin.x = field_origin_base.x - NSMinX(anchor_rect_base);
+  table_frame.origin.y = kPopupPaddingVertical;
+  table_frame.size.width = tableWidth;
+  table_frame.size.height = tableHeight;
+  [table_view_ setFrame:table_frame];
 
   NSRect current_poup_frame = [popup_ frame];
   target_popup_frame_ = popup_frame;