[AiS] changing mac omnibox suggestions form NSMatrix to NSTableView

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

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/cocoa/omnibox/omnibox_popup_view_mac.h"
 
 #include <cmath>
 
 #include "base/stl_util.h"
 #include "base/strings/sys_string_conversions.h"
@@ skipping 47 matching lines @@
   DCHECK(edit_model);
 }
 
 OmniboxPopupViewMac::~OmniboxPopupViewMac() {
   // Destroy the popup model before this object is destroyed, because
   // it can call back to us in the destructor.
   model_.reset();
 
   // Break references to |this| because the popup may not be
   // deallocated immediately.
-  [matrix_ setDelegate:NULL];
+  [matrix_ setTheDelegate:NULL];
 }
 
 bool OmniboxPopupViewMac::IsOpen() const {
   return popup_ != nil;
 }
 
 void OmniboxPopupViewMac::UpdatePopupAppearance() {
   DCHECK([NSThread isMainThread]);
   const AutocompleteResult& result = GetResult();
   const size_t start_match = result.ShouldHideTopMatch() ? 1 : 0;
   const size_t rows = result.size() - start_match;
   if (rows == 0) {
     [[popup_ parentWindow] removeChildWindow:popup_];
     [popup_ orderOut:nil];
 
     // Break references to |this| because the popup may not be
     // deallocated immediately.
-    [matrix_ setDelegate:nil];
+    [matrix_ setTheDelegate:NULL];
     matrix_.reset();
 
     popup_.reset(nil);
 
     target_popup_frame_ = NSZeroRect;
 
     return;
   }
 
   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];
-  CGFloat max_match_contents_width = 0.0f;
-  CGFloat contents_offset = -1.0f;
+  NSMutableArray* array = [NSMutableArray array];
+  NSInteger popupHeight = 0;
+  NSRect cellRect = NSZeroRect;
+  cellRect.size.width = NSWidth([field_ bounds]);
   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];
-    if (match.type == AutocompleteMatchType::SEARCH_SUGGEST_TAIL) {
-      max_match_contents_width = std::max(max_match_contents_width,
-                                           [cell getMatchContentsWidth]);
-      if (contents_offset < 0.0f) {
-        contents_offset = [OmniboxPopupCell computeContentsOffset:match];
-      }
-      [cell setContentsOffset:contents_offset];
-    }
+    NSMutableDictionary* dictionary = [NSMutableDictionary dictionary];
+    AutocompleteMatchWrapper* acm =
+        [[[AutocompleteMatchWrapper alloc] init] autorelease];
+    acm->match_ = match;

[groby-ooo-7-16, 2015/05/07 02:39:59]

Why not keep match and image in the wrapped match? (BTW: wrappedMatch is a more
readable name than acm)

[dschuyler, 2015/05/07 20:53:10]

It feels like there are two opposing options to choose from.  One is to go with
a struct/class style to hold the data and another is to go with a dictionary
style.  I'd like to go one direction or the other, by either removing the
dictionary local or breaking pieces out as separate key/value pairs in the
dictionary.

There's a higher level choice that can be made here.  That is whether we should
keep an array of NSCell objects or whether to keep an array of either structs or
dictionaries.

I'd like to chat about the options if we get a chance :)

Done.

[Scott Hess - ex-Googler, 2015/05/07 22:35:42]

Keep in mind that we're an Objective-C++ project, and C++ is really common. 
Boxing C structures in Objective-C objects is a very FoundationKit thing to do,
but when you can just as easily use an STL container without all the boxing, I'd
say go with that.  The results sometimes look funky, but IMHO things like
-valueWithRect: are already pretty funky.

+    [dictionary setObject:acm forKey:@"match"];
+    NSImage* image = ImageForMatch(match);
+    [dictionary setObject:image forKey:@"image"];
+    cellRect.origin.y = popupHeight;
+    CGFloat height = [image size].height + kCellHeightAdjust;
+    cellRect.size.height = height;
+    [dictionary setObject:[NSValue valueWithRect:cellRect] forKey:@"rect"];
+    [array addObject:dictionary];
+    popupHeight += height;
   }
-
-  for (size_t ii = 0; ii < rows; ++ii) {
-    OmniboxPopupCell* cell = [matrix_ cellAtRow:ii column:0];
-    [cell 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];
+  if (rows)
+    popupHeight += [matrix_ intercellSpacing].height * (rows - 1);
 
   // 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
   // animation.
   DCHECK_EQ([matrix_ intercellSpacing].height, 0.0);
-  PositionPopup(rows * cell_height);
+  PositionPopup(popupHeight);
 }
 
 gfx::Rect OmniboxPopupViewMac::GetTargetBounds() {
   // Flip the coordinate system before returning.
   NSScreen* screen = [[NSScreen screens] objectAtIndex:0];
   NSRect monitor_frame = [screen frame];
   gfx::Rect bounds(NSRectToCGRect(target_popup_frame_));
   bounds.set_y(monitor_frame.size.height - bounds.y() - bounds.height());
   return bounds;
 }
 
 // This is only called by model in SetSelectedLine() after updating
 // 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];
   }
-
+  [matrix_ selectRowIndexes:indexSet byExtendingSelection:NO];
 }
 
 void OmniboxPopupViewMac::OnMatrixRowSelected(OmniboxPopupMatrix* matrix,
                                               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) {
@@ skipping 60 matching lines @@
   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;
   popup_frame.size.height += [OmniboxPopupTopSeparatorView preferredHeight];
   popup_frame.size.height += [OmniboxPopupBottomSeparatorView preferredHeight];
   popup_frame.origin.y -= NSHeight(popup_frame);
   // Shift to screen coordinates.
   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);
   top_separator_frame.size.height =
       [OmniboxPopupTopSeparatorView preferredHeight];
   [top_separator_view_ setFrame:top_separator_frame];
 
   // Bottom separator.
   NSRect bottom_separator_frame = NSZeroRect;
   bottom_separator_frame.size.width = NSWidth(popup_frame);
   bottom_separator_frame.size.height =
       [OmniboxPopupBottomSeparatorView preferredHeight];
   bottom_separator_frame.origin.y =
       NSHeight(popup_frame) - NSHeight(bottom_separator_frame);
   [bottom_separator_view_ setFrame:bottom_separator_frame];
 
   // Background view.
   NSRect background_rect = NSZeroRect;
   background_rect.size.width = NSWidth(popup_frame);
   background_rect.size.height = NSHeight(popup_frame) -
       NSHeight(top_separator_frame) - NSHeight(bottom_separator_frame);
   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];
 
   NSRect current_poup_frame = [popup_ frame];
   target_popup_frame_ = popup_frame;
 
   // Animate the frame change if the only change is that the height got smaller.
   // Otherwise, resize immediately.
   bool animate = (NSHeight(popup_frame) < NSHeight(current_poup_frame) &&
                   NSWidth(popup_frame) == NSWidth(current_poup_frame));
@@ skipping 39 matching lines @@
 }
 
 void OmniboxPopupViewMac::OpenURLForRow(size_t row,
                                         WindowOpenDisposition disposition) {
   size_t start_match = model_->result().ShouldHideTopMatch() ? 1 : 0;
   row += start_match;
   DCHECK_LT(row, GetResult().size());
   omnibox_view_->OpenMatch(GetResult().match_at(row), disposition, GURL(),
                            base::string16(), row);
 }