[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 8e403f0622416c3759f9ca9b209afd89a158211c..724f6dce5fbd647b5c509d6d8d6281bb1550d074 100644
--- a/chrome/browser/ui/cocoa/omnibox/omnibox_popup_view_mac.mm
+++ b/chrome/browser/ui/cocoa/omnibox/omnibox_popup_view_mac.mm
@@ -29,10 +29,6 @@
 
 namespace {
 
-// How much to adjust the cell sizing up from the default determined
-// by the font.
-const CGFloat kCellHeightAdjust = 6.0;
-
 // Padding between matrix and the top and bottom of the popup window.
 const CGFloat kPopupPaddingVertical = 5.0;
 
@@ -83,7 +79,7 @@ void OmniboxPopupViewMac::UpdatePopupAppearance() {
 
     // Break references to |this| because the popup may not be
     // deallocated immediately.
-    [matrix_ setDelegate:nil];
+    [matrix_ setObserver:NULL];
     matrix_.reset();
 
     popup_.reset(nil);
@@ -95,43 +91,39 @@ 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);
-
   // Load the results into the popup's matrix.
   DCHECK_GT(rows, 0U);
-  [matrix_ renewRows:rows columns:1];
+  base::scoped_nsobject<NSMutableArray> array([[NSMutableArray alloc] init]);

[groby-ooo-7-16, 2015/05/20 01:02:31]

Theoretically...

What we're building here _is_ essentially the NSTableViewDataSource.

Do you think it makes sense to have an initWithMatches: on the data source, and
then just set the data source on the NSTableView?

[dschuyler, 2015/05/21 00:38:39]

I've moved the data init into the Controller (which is both the table delegate
and the table data source).  Does this look better?

Done.

+  CGFloat popupHeight = 0;
+  NSRect cellRect = NSZeroRect;
+  cellRect.size.width = NSWidth([field_ bounds]);
   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];
     const AutocompleteMatch& match = GetResult().match_at(ii + start_match);
-    [cell setImage:ImageForMatch(match)];
-    [cell setMatch:match];
+    NSImage* image = ImageForMatch(match);
+    base::scoped_nsobject<OmniboxPopupCellData> cellData(
+        [[OmniboxPopupCellData alloc] initWithMatch:match image:image]);
+    [array addObject:cellData];
+    popupHeight += [cellData rowHeight];
     if (match.type == AutocompleteMatchType::SEARCH_SUGGEST_TAIL) {
-      max_match_contents_width = std::max(max_match_contents_width,
-                                           [cell getMatchContentsWidth]);
+      max_match_contents_width =
+          std::max(max_match_contents_width, [cellData getMatchContentsWidth]);
       if (contents_offset < 0.0f) {
         contents_offset = [OmniboxPopupCell computeContentsOffset:match];
       }
-      [cell setContentsOffset:contents_offset];
+      [cellData setContentsOffset:contents_offset];
     }
   }
 
   for (size_t ii = 0; ii < rows; ++ii) {
-    OmniboxPopupCell* cell = [matrix_ cellAtRow:ii column:0];
-    [cell setMaxMatchContentsWidth:max_match_contents_width];
+    OmniboxPopupCellData* cellData = [array objectAtIndex:ii];
+    [cellData setMaxMatchContentsWidth:max_match_contents_width];
   }
 
-  // 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)];
+  [matrix_ setDataArray:array];
+  [matrix_ reloadData];
+  popupHeight += [matrix_ intercellSpacing].height * (rows - 1);
 
   // Update the selection before placing (and displaying) the window.
   PaintUpdatesNow();
@@ -140,7 +132,7 @@ void OmniboxPopupViewMac::UpdatePopupAppearance() {
   // because actually resizing the matrix messed up the popup size
   // animation.
   DCHECK_EQ([matrix_ intercellSpacing].height, 0.0);
-  PositionPopup(rows * cell_height);
+  PositionPopup(popupHeight);
 }
 
 gfx::Rect OmniboxPopupViewMac::GetTargetBounds() {
@@ -156,12 +148,14 @@ 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];
+  NSIndexSet* indexSet = nil;

[groby-ooo-7-16, 2015/05/20 01:02:31]

Since we're only allow single selection, why not move this onto the matrix as
-selectRow:

[dschuyler, 2015/05/21 00:38:39]

How about selectRowIndex.

Done.

+  if (start_match <= model_->selected_line()) {
+    indexSet =
+        [NSIndexSet indexSetWithIndex:model_->selected_line() - start_match];
   } else {
-    [matrix_ selectCellAtRow:model_->selected_line() - start_match column:0];
+    indexSet = [NSIndexSet indexSet];
   }
-
+  [matrix_ selectRowIndexes:indexSet byExtendingSelection:NO];
 }
 
 void OmniboxPopupViewMac::OnMatrixRowSelected(OmniboxPopupMatrix* matrix,
@@ -242,10 +236,6 @@ void OmniboxPopupViewMac::PositionPopup(const CGFloat matrixHeight) {
   popup_frame.origin =
       [[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;
-
   // Top separator.
   NSRect top_separator_frame = NSZeroRect;
   top_separator_frame.size.width = NSWidth(popup_frame);
@@ -270,13 +260,18 @@ void OmniboxPopupViewMac::PositionPopup(const CGFloat matrixHeight) {
   background_rect.origin.y = NSMaxY(top_separator_frame);
   [background_view_ setFrame:background_rect];
 
+  // 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);
+
   // Matrix.
   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.width = tableWidth;
   matrix_frame.size.height = matrixHeight;
   [matrix_ setFrame:matrix_frame];