Omnibox UI Experiments: Port Vertical Layout experiment to Mac Cocoa.

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

Index: chrome/browser/ui/cocoa/omnibox/omnibox_popup_cell.mm
diff --git a/chrome/browser/ui/cocoa/omnibox/omnibox_popup_cell.mm b/chrome/browser/ui/cocoa/omnibox/omnibox_popup_cell.mm
index a41858a5db1156096b72a0cae272bd888bf6b149..68d6a337c52eb177dcbf069605343954756c82bd 100644
--- a/chrome/browser/ui/cocoa/omnibox/omnibox_popup_cell.mm
+++ b/chrome/browser/ui/cocoa/omnibox/omnibox_popup_cell.mm
@@ -9,6 +9,7 @@
 #include <algorithm>
 #include <cmath>
 
+#include "base/feature_list.h"
 #include "base/i18n/rtl.h"
 #include "base/mac/foundation_util.h"
 #include "base/mac/objc_property_releaser.h"
@@ -40,10 +41,12 @@ constexpr CGFloat kMaterialTextStartOffset = 27.0;
 
 constexpr CGFloat kMaterialImageXOffset = 6.0;
 
+constexpr CGFloat kDefaultVerticalMargin = 3.0;
+
+constexpr CGFloat kDefaultTextHeight = 19;
+
 // Returns the margin that should appear at the top and bottom of the result.
 CGFloat GetVerticalMargin() {
-  constexpr CGFloat kDefaultVerticalMargin = 3.0;
-
   return base::GetFieldTrialParamByFeatureAsInt(
       omnibox::kUIExperimentVerticalMargin,
       OmniboxFieldTrial::kUIVerticalMarginParam, kDefaultVerticalMargin);
@@ -424,8 +427,18 @@ NSAttributedString* CreateClassifiedAttributedString(
         description_ = [CreateClassifiedAttributedString(
             match.description, DimTextColor(isDarkTheme),
             match.description_class, isDarkTheme) retain];
+
+        if (base::FeatureList::IsEnabled(
+                omnibox::kUIExperimentVerticalLayout) &&
+            !AutocompleteMatch::IsSearchType(match.type)) {
+          // Somtimes swap the contents and description in vertical layouts.

[groby-ooo-7-16, 2017/05/31 00:14:51]

When is "sometimes"?

[tommycli, 2017/05/31 18:20:40]

Done.

+          // TODO(tommycli): If vertical layouts become permanent, swap these
+          // in the underlying match instead of the UI code.

[groby-ooo-7-16, 2017/05/31 00:14:51]

Rendering order should not really affect anything *but* UI code. 

[tommycli, 2017/05/31 18:20:40]

Done.

+          NSAttributedString* temp = contents_;

[groby-ooo-7-16, 2017/05/31 00:14:51]

std::swap(contents_, description_)

[tommycli, 2017/05/31 18:20:40]

Done. Always forget that ObjC is also C++ ... or is it... i'm not completely
sure.

+          contents_ = description_;
+          description_ = temp;
+        }
       }
-      maxLines_ = 1;
     }
     propertyReleaser_OmniboxPopupCellData_.Init(self,
                                                 [OmniboxPopupCellData class]);
@@ -463,6 +476,9 @@ NSAttributedString* CreateClassifiedAttributedString(
 }
 
 - (void)drawMatchWithFrame:(NSRect)cellFrame inView:(NSView*)controlView {
+  bool isVerticalLayout =
+      base::FeatureList::IsEnabled(omnibox::kUIExperimentVerticalLayout);
+
   OmniboxPopupCellData* cellData =
       base::mac::ObjCCastStrict<OmniboxPopupCellData>([self objectValue]);
   OmniboxPopupMatrix* tableView =
@@ -476,12 +492,12 @@ NSAttributedString* CreateClassifiedAttributedString(
   int contentsMaxWidth, descriptionMaxWidth;
   OmniboxPopupModel::ComputeMatchMaxWidths(
       ceilf(contentsWidth), ceilf(separatorWidth), ceilf(descriptionWidth),
-      ceilf(remainingWidth),
-      [cellData isAnswer],
-      !AutocompleteMatch::IsSearchType([cellData matchType]),
-      &contentsMaxWidth,
+      ceilf(remainingWidth), [cellData isAnswer] || isVerticalLayout,
+      !AutocompleteMatch::IsSearchType([cellData matchType]), &contentsMaxWidth,
       &descriptionMaxWidth);
 
+  CGFloat halfLineHeight = (kDefaultTextHeight + kDefaultVerticalMargin) / 2;
+
   NSWindow* parentWindow = [[controlView window] parentWindow];
   BOOL isDarkTheme = [parentWindow hasDarkTheme];
   NSRect imageRect = cellFrame;
@@ -491,6 +507,8 @@ NSAttributedString* CreateClassifiedAttributedString(
   imageRect.origin.x += kMaterialImageXOffset + [tableView contentLeftPadding];
   imageRect.origin.y +=
       GetVerticalMargin() + kMaterialExtraVerticalImagePadding;
+  if (isVerticalLayout)
+    imageRect.origin.y += halfLineHeight;
   [theImage drawInRect:FlipIfRTL(imageRect, cellFrame)
               fromRect:NSZeroRect
              operation:NSCompositeSourceOver
@@ -498,9 +516,13 @@ NSAttributedString* CreateClassifiedAttributedString(
         respectFlipped:YES
                  hints:nil];
 
-  NSPoint origin =
-      NSMakePoint(kMaterialTextStartOffset + [tableView contentLeftPadding],
-                  GetVerticalMargin());
+  CGFloat left = kMaterialTextStartOffset + [tableView contentLeftPadding];
+  NSPoint origin = NSMakePoint(left, GetVerticalMargin());
+
+  // For matches lacking description in vertical layout, center vertically.
+  if (isVerticalLayout && descriptionMaxWidth == 0)
+    origin.y += halfLineHeight;
+
   if ([cellData matchType] == AutocompleteMatchType::SEARCH_SUGGEST_TAIL) {
     // Tail suggestions are rendered with a prefix (usually ellipsis), which
     // appear vertically stacked.
@@ -517,9 +539,8 @@ NSAttributedString* CreateClassifiedAttributedString(
 
   if (descriptionMaxWidth > 0) {
     if ([cellData isAnswer]) {
-      origin = NSMakePoint(
-          kMaterialTextStartOffset + [tableView contentLeftPadding],
-          [OmniboxPopupCell getContentTextHeight] - GetVerticalMargin());
+      origin = NSMakePoint(left, [OmniboxPopupCell getContentTextHeight:false] -
+                                     GetVerticalMargin());
       CGFloat imageSize = [tableView answerLineHeight];
       NSRect imageRect =
           NSMakeRect(NSMinX(cellFrame) + origin.x, NSMinY(cellFrame) + origin.y,
@@ -538,17 +559,22 @@ NSAttributedString* CreateClassifiedAttributedString(
         origin.y += 1;
       }
     } else {
-      origin.x += [self drawMatchPart:[tableView separator]
-                            withFrame:cellFrame
-                               origin:origin
-                         withMaxWidth:separatorWidth
-                         forDarkTheme:isDarkTheme];
+      if (isVerticalLayout) {
+        origin.x = left;

[groby-ooo-7-16, 2017/05/31 00:14:51]

If you assign the whole point:

origin = NSMakePoint(left, halfLineHeight * 2);

[tommycli, 2017/05/31 18:20:39]

Hey. I actually need to add to the y coordinate, rather than re-assign it.

[groby-ooo-7-16, 2017/05/31 19:24:16]

And so you do. TIL my brain skips characters at color boundaries, sorry :(

+        origin.y += halfLineHeight * 2;
+      } else {
+        origin.x += [self drawMatchPart:[tableView separator]
+                              withFrame:cellFrame
+                                 origin:origin
+                           withMaxWidth:separatorWidth
+                           forDarkTheme:isDarkTheme];
+      }
     }
-    origin.x += [self drawMatchPart:[cellData description]
-                          withFrame:cellFrame
-                             origin:origin
-                       withMaxWidth:descriptionMaxWidth
-                       forDarkTheme:isDarkTheme];
+    [self drawMatchPart:[cellData description]
+              withFrame:cellFrame
+                 origin:origin
+           withMaxWidth:descriptionMaxWidth
+           forDarkTheme:isDarkTheme];
   }
 }
 
@@ -695,9 +721,11 @@ NSAttributedString* CreateClassifiedAttributedString(
   return NSWidth(cellFrame) - kMaterialTextStartOffset;
 }
 
-+ (CGFloat)getContentTextHeight {
-  constexpr CGFloat kDefaultTextHeight = 19;
-  return kDefaultTextHeight + 2 * GetVerticalMargin();
++ (CGFloat)getContentTextHeight:(BOOL)isTwoLine {
+  CGFloat height = kDefaultTextHeight + 2 * GetVerticalMargin();
+  if (isTwoLine)
+    height += kDefaultTextHeight + kDefaultVerticalMargin;
+  return height;
 }
 
 @end