[Mac] Fix some omnibox breakage for small windows.

Change the code to decorate the contents string normally, then elide
the marked-up string if it doesn't fit.

http://crbug.com/23279
TEST=See bug.

Review URL: http://codereview.chromium.org/264012

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@28603 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 149 insertions, 10 deletions

diff --git a/chrome/browser/autocomplete/autocomplete_popup_view_mac.h b/chrome/browser/autocomplete/autocomplete_popup_view_mac.h
index 2dddd70..3782ef8 100644
--- a/chrome/browser/autocomplete/autocomplete_popup_view_mac.h
+++ b/chrome/browser/autocomplete/autocomplete_popup_view_mac.h
@@ -93,6 +93,18 @@ class AutocompletePopupViewMac : public AutocompletePopupView {
       const AutocompleteMatch::ACMatchClassifications &classifications,
       NSColor* textColor, gfx::Font& font);
 
+  // Helper for MatchText() to elide a marked-up string using
+  // gfx::ElideText() as a model.  Modifies |aString| in place.
+  // TODO(shess): Consider breaking AutocompleteButtonCell out of this
+  // code, and modifying it to have something like -setMatch:, so that
+  // these convolutions to expose internals for testing can be
+  // cleaner.
+  static NSMutableAttributedString* ElideString(
+      NSMutableAttributedString* aString,
+      const std::wstring originalString,
+      const gfx::Font& font,
+      const float cellWidth);
+
  private:
   // Create the popup_ instance if needed.
   void CreatePopupIfNeeded();
diff --git a/chrome/browser/autocomplete/autocomplete_popup_view_mac.mm b/chrome/browser/autocomplete/autocomplete_popup_view_mac.mm
index d476e830..31ecaa1 100644
--- a/chrome/browser/autocomplete/autocomplete_popup_view_mac.mm
+++ b/chrome/browser/autocomplete/autocomplete_popup_view_mac.mm
@@ -48,7 +48,7 @@ const float kShrinkAnimationDuration = 0.1;
 
 // Maximum fraction of the popup width that can be used to display match
 // contents.
-const float kMaxMatchContentsWidth = 0.7;
+const float kMaxContentsFraction = 0.7;
 
 // NSEvent -buttonNumber for middle mouse button.
 const static NSInteger kMiddleButtonNumber(2);
@@ -176,27 +176,63 @@ NSMutableAttributedString* AutocompletePopupViewMac::DecorateMatchedString(
   return as;
 }
 
+NSMutableAttributedString* AutocompletePopupViewMac::ElideString(
+    NSMutableAttributedString* aString,
+    const std::wstring originalString,
+    const gfx::Font& font,
+    const float width) {
+  // If it already fits, nothing to be done.
+  if ([aString size].width <= width) {
+    return aString;
+  }
+
+  // If ElideText() decides to do nothing, nothing to be done.
+  const std::wstring elided(ElideText(originalString, font, width));
+  if (0 == elided.compare(originalString)) {
+    return aString;
+  }
+
+  // If everything was elided away, clear the string.
+  if (elided.size() == 0) {
+    [aString deleteCharactersInRange:NSMakeRange(0, [aString length])];
+    return aString;
+  }
+
+  // The ellipses should be the last character, and everything before
+  // that should match the original string.
+  const size_t i(elided.size() - 1);
+  DCHECK(0 != elided.compare(0, i, originalString));
+
+  // Replace the end of |aString| with the ellipses from |elided|.
+  NSString* s = base::SysWideToNSString(elided.substr(i));
+  [aString replaceCharactersInRange:NSMakeRange(i, [aString length] - i)
+                         withString:s];
+
+  return aString;
+}
+
 // Return the text to show for the match, based on the match's
 // contents and description.  Result will be in |font|, with the
 // boldfaced version used for matches.
 NSAttributedString* AutocompletePopupViewMac::MatchText(
     const AutocompleteMatch& match, gfx::Font& font, float cellWidth) {
-  // If there is a description, then the URL can take at most 70% of the
-  // available width, with 30% being reserved for the description.  If there is
-  // no description, then the URL can take the full 100%.
-  float availableWidth = cellWidth - kTextXOffset - kLeftRightMargin;
-  BOOL hasDescription = match.description.empty() ? NO : YES;
-  float urlWidth = hasDescription ? availableWidth * kMaxMatchContentsWidth
-                                  : availableWidth;
-
   NSMutableAttributedString *as =
-      DecorateMatchedString(gfx::ElideText(match.contents, font, urlWidth),
+      DecorateMatchedString(match.contents,
                             match.contents_class,
                             ContentTextColor(), font);
 
   // If there is a description, append it, separated from the contents
   // with an en dash, and decorated with a distinct color.
   if (!match.description.empty()) {
+    // Make sure the current string fits w/in kMaxContentsFraction of
+    // the cell to make sure the description will be at least
+    // partially visible.
+    // TODO(shess): Consider revising our NSCell subclass to have two
+    // bits and just draw them right, rather than truncating here.
+    const float textWidth = cellWidth - kTextXOffset - kLeftRightMargin;
+    as = ElideString(as, match.contents, font,
+                     textWidth * kMaxContentsFraction);
+
     NSDictionary* attributes =
         [NSDictionary dictionaryWithObjectsAndKeys:
              font.nativeFont(), NSFontAttributeName,
diff --git a/chrome/browser/autocomplete/autocomplete_popup_view_mac_unittest.mm b/chrome/browser/autocomplete/autocomplete_popup_view_mac_unittest.mm
index d845ee6..03ea922 100644
--- a/chrome/browser/autocomplete/autocomplete_popup_view_mac_unittest.mm
+++ b/chrome/browser/autocomplete/autocomplete_popup_view_mac_unittest.mm
@@ -4,6 +4,7 @@
 
 #import "chrome/browser/autocomplete/autocomplete_popup_view_mac.h"
 
+#include "app/gfx/text_elider.h"
 #include "base/scoped_ptr.h"
 #include "base/sys_string_conversions.h"
 #include "chrome/browser/autocomplete/autocomplete.h"
@@ -418,6 +419,96 @@ TEST_F(AutocompletePopupViewMacTest, MatchTextDescriptionMatch) {
                                NSBoldFontMask));
 }
 
+TEST_F(AutocompletePopupViewMacTest, ElideString) {
+  const NSString* contents = @"This is a test with long contents";
+  const std::wstring wideContents(base::SysNSStringToWide(contents));
+
+  const float kWide = 1000.0;
+  const float kNarrow = 20.0;
+
+  NSDictionary* attributes =
+      [NSDictionary dictionaryWithObject:font_.nativeFont()
+                                  forKey:NSFontAttributeName];
+  scoped_nsobject<NSMutableAttributedString> as(
+      [[NSMutableAttributedString alloc] initWithString:contents
+                                             attributes:attributes]);
+
+  // Nothing happens if the space is really wide.
+  NSMutableAttributedString* ret =
+      AutocompletePopupViewMac::ElideString(as, wideContents, font_, kWide);
+  EXPECT_TRUE(ret == as);
+  EXPECT_TRUE([[as string] isEqualToString:contents]);
+
+  // When elided, result is the same as ElideText().
+  ret = AutocompletePopupViewMac::ElideString(as, wideContents, font_, kNarrow);
+  std::wstring elided(ElideText(wideContents, font_, kNarrow));
+  EXPECT_TRUE(ret == as);
+  EXPECT_FALSE([[as string] isEqualToString:contents]);
+  EXPECT_TRUE([[as string] isEqualToString:base::SysWideToNSString(elided)]);
+
+  // When elided, result is the same as ElideText().
+  ret = AutocompletePopupViewMac::ElideString(as, wideContents, font_, 0.0);
+  elided = ElideText(wideContents, font_, 0.0);
+  EXPECT_TRUE(ret == as);
+  EXPECT_FALSE([[as string] isEqualToString:contents]);
+  EXPECT_TRUE([[as string] isEqualToString:base::SysWideToNSString(elided)]);
+}
+
+TEST_F(AutocompletePopupViewMacTest, MatchTextElide) {
+  const NSString* contents = @"This is a test with long contents";
+  const NSString* description = @"That was a test";
+  // Match "long".
+  const NSUInteger runLength1 = 20, runLength2 = 4, runLength3 = 9;
+  // Make sure nobody messed up the inputs.
+  EXPECT_EQ(runLength1 + runLength2 + runLength3, [contents length]);
+
+  AutocompleteMatch m = MakeMatch(base::SysNSStringToWide(contents),
+                                  base::SysNSStringToWide(description));
+
+  // Push each run onto contents classifications.
+  m.contents_class.push_back(
+      ACMatchClassification(0, ACMatchClassification::NONE));
+  m.contents_class.push_back(
+      ACMatchClassification(runLength1, ACMatchClassification::MATCH));
+  m.contents_class.push_back(
+      ACMatchClassification(runLength1 + runLength2,
+                            ACMatchClassification::URL));
+
+  // Figure out the width of the contents.
+  NSDictionary* attributes =
+      [NSDictionary dictionaryWithObject:font_.nativeFont()
+                                  forKey:NSFontAttributeName];
+  const float contentsWidth = [contents sizeWithAttributes:attributes].width;
+
+  // After accounting for the width of the image, this will force us
+  // to elide the contents.
+  float cellWidth = ceil(contentsWidth / 0.7);
+
+  NSAttributedString* decorated =
+      AutocompletePopupViewMac::MatchText(m, font_, cellWidth);
+
+  // Results contain a prefix of the contents and all of description.
+  NSString* commonPrefix =
+      [[decorated string] commonPrefixWithString:contents options:0];
+  EXPECT_GT([commonPrefix length], 0U);
+  EXPECT_LT([commonPrefix length], [contents length]);
+  EXPECT_TRUE([[decorated string] hasSuffix:description]);
+
+  // At one point the code had a bug where elided text was being
+  // marked up using pre-elided offsets, resulting in out-of-range
+  // values being passed to NSAttributedString.  Push the ellipsis
+  // through part of each run to verify that we don't continue to see
+  // such things.
+  while([commonPrefix length] > runLength1 - 3) {
+    EXPECT_GT(cellWidth, 0.0);
+    cellWidth -= 1.0;
+    decorated = AutocompletePopupViewMac::MatchText(m, font_, cellWidth);
+    commonPrefix =
+        [[decorated string] commonPrefixWithString:contents options:0];
+    ASSERT_GT([commonPrefix length], 0U);
+  }
+}
+
 // TODO(shess): Test that
 // AutocompletePopupViewMac::UpdatePopupAppearance() creates/destroys
 // the popup according to result contents.  Test that the matrix gets