Infinite Suggest for mac

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 29 matching lines @@
 // How far to offset the text column from the left.
 const CGFloat kTextXOffset = 28.0;
 
 // Animation duration when animating the popup window smaller.
 const NSTimeInterval kShrinkAnimationDuration = 0.1;
 
 // Maximum fraction of the popup width that can be used to display match
 // contents.
 const CGFloat kMaxContentsFraction = 0.7;
 
+const char16 kEllipsis[] = { 0x2026, 0x20, 0x0 };

[Scott Hess - ex-Googler, 2013/12/12 19:11:15]

Don't define your own constant.  ElideString() uses gfx::ElideText(), which uses
gfx::kEllipsisUTF16.  Doesn't have the space, but that's easy enough to
construct.

[Anuj, 2013/12/12 23:56:22]

Done.

+
 // Background colors for different states of the popup elements.
 NSColor* BackgroundColor() {
   return [NSColor controlBackgroundColor];
 }
 
 NSColor* ContentTextColor() {
   return [NSColor blackColor];
 }
 NSColor* DimContentTextColor() {
   return [NSColor darkGrayColor];
@@ skipping 56 matching lines @@
   gfx::Font result_font(OmniboxViewMac::GetFieldFont());
 
   // 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_width = 0;
+  CGFloat max_required_width = 0;
+  CGFloat ignored_prefix_width = 0;
+  CGFloat ellipsis_prefix_width = 0;
   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 setAttributedTitle:MatchText(match, result_font, matrix_width)];
+    NSAttributedString* match_text =
+        MatchText(match, result_font, matrix_width);

[Scott Hess - ex-Googler, 2013/12/12 19:11:15]

This is going to construct a result which you are then going to throw away in
the SEARCH_SUGGEST_INFINITE case and construct all over again using very similar
code.  So it seems like it might make sense to abstract it out into a similar
helper, rather than having it inline, obscuring the overall matrix setup.

I'm still trying to detangle how one might handle ignored_prefix_width and
ellipsis_prefix_width, which seem constant for all SEARCH_SUGGEST_INFINITE,
versus the max_ values which aren't.

Your screen shots show the offsets as being the same for all values, so the
per-iteration calculations in the second loop seem suspect, to me.  I'm
wondering if it would be more appropriate to tell the cell itself whether it's
one of these cells, and then calculate a constant offset and set it for _all_ of
the cells, with only the cells set to that type actually adjusting by it.

[Anuj, 2013/12/12 19:40:55]

1. I can change the code as follows
if (ignored_prefix_width == 0)
  ignored_prefix_width = [ignored_prefix_as size].width;

Same for ellipsis_prefix_width.

2. max_match_width and max_required_width still need to be computed.

3. You are right - The additional_offset calculation can be moved out of the
second loop.

4. I should probably use scoped_ptr<NSMutableAttributedString> inside the loop -
please advice.

+
+    if (match.type == AutocompleteMatchType::SEARCH_SUGGEST_INFINITE) {
+

[Scott Hess - ex-Googler, 2013/12/12 19:11:15]

No empty line.

[Anuj, 2013/12/12 23:56:22]

Done.

+      string16 ignored_prefix = match.fill_into_edit.substr(
+          0, match.fill_into_edit.rfind(match.contents));
+      NSMutableAttributedString* ignored_prefix_as = DecorateMatchedString(
+        ignored_prefix, ACMatchClassifications(), ContentTextColor(),
+        DimContentTextColor(), result_font);
+      ignored_prefix_width = [ignored_prefix_as size].width;
+      [ignored_prefix_as appendAttributedString:match_text];
+      CGFloat required_width = [ignored_prefix_as size].width;
+      if (required_width > max_required_width)
+        max_required_width = required_width;
+
+      NSMutableAttributedString* ellipsis_prefix = DecorateMatchedString(
+          kEllipsis, ACMatchClassifications(), ContentTextColor(),
+          DimContentTextColor(), result_font);
+      ellipsis_prefix_width = [ellipsis_prefix size].width;
+      [ellipsis_prefix appendAttributedString:match_text];
+      CGFloat match_width = [ellipsis_prefix size].width;
+      if (match_width > max_match_width)
+        max_match_width = match_width;
+      match_text = ellipsis_prefix;
+    }
+    [cell setAttributedTitle:match_text];
+  }
+
+  for (size_t ii = 0; ii < rows; ++ii) {
+    const AutocompleteMatch& match = GetResult().match_at(ii + start_match);
+    OmniboxPopupCell* cell = [matrix_ cellAtRow:ii column:0];
+    [cell setAdditionalOffset:0];
+    if (match.type != AutocompleteMatchType::SEARCH_SUGGEST_INFINITE) {
+      continue;
+    }
+
+    const float text_width = matrix_width - kTextXOffset;
+    CGFloat additional_offset = (max_required_width < text_width) ?
+        (ignored_prefix_width - ellipsis_prefix_width) :
+        (text_width - max_match_width);
+    if (additional_offset > 0)
+      [cell setAdditionalOffset:additional_offset];
   }
 
   // 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)];
 
@@ skipping 351 matching lines @@
   return OmniboxViewMac::ImageForResource(resource_id);
 }
 
 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(), row);
 }