[AiS] for desktop, two lines and font sytles

chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc

Index: chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc
diff --git a/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc b/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc
index fe061f65a9c0175152fd9e6d013d990b650f4663..a2112c0dfe707af48e11c687295ff60c217f862b 100644
--- a/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc
+++ b/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc
@@ -11,6 +11,7 @@
 #include "chrome/browser/ui/omnibox/omnibox_view.h"
 #include "chrome/browser/ui/views/location_bar/location_bar_view.h"
 #include "chrome/browser/ui/views/omnibox/omnibox_result_view.h"
+#include "components/omnibox/omnibox_field_trial.h"
 #include "ui/base/theme_provider.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/image/image.h"
@@ -384,18 +385,26 @@ int OmniboxPopupContentsView::CalculatePopupHeight() {
        i < model_->result().size(); ++i)
     popup_height += child_at(i)->GetPreferredSize().height();
 
-  // Add enough space on the top and bottom so it looks like there is the same
-  // amount of space between the text and the popup border as there is in the
-  // interior between each row of text.
-  //
-  // Discovering the exact amount of leading and padding around the font is
-  // a bit tricky and platform-specific, but this computation seems to work in
-  // practice.
-  OmniboxResultView* result_view = result_view_at(0);
-  outside_vertical_padding_ =
-      (result_view->GetPreferredSize().height() -
-       result_view->GetTextHeight());
-
+  if (OmniboxFieldTrial::EnableAnswersInSuggest()) {
+    // The cell heights for the result views can
+    // vary cell to cell.  They may vary by the number of lines used for the
+    // result or by the fonts used.  Because of this, we're not able to use the
+    // <preferred size height minus text height> method used below.
+    // The * 2 makes this approximation match the prior method (empirically).
+    outside_vertical_padding_ =
+        result_view_at(0)->get_minimum_text_vertical_padding() * 2;

[Peter Kasting, 2015/03/14 01:54:14]

Good digging.

After reading the code in both files more, I think we should just switch to
doing this unconditionally.  We can change the comment to something like what's
below, but with the second paragraph replaced by a comment about how normally
each row pads the top and bottom with the minimum text padding, so between two
rows would be 2 * that padding.

I also think we can nuke the accessor for the result view's minimum text
padding, because the setter (and, indeed, the other padding setters) are never
called (says code search).  So let's just kill all the setters, kill the member
variables, use the constants directly in that file, and make the relevant
constant here public.

[dschuyler, 2015/03/16 21:49:09]

Done.

+  } else {
+    // Add enough space on the top and bottom so it looks like there is the same
+    // amount of space between the text and the popup border as there is in the
+    // interior between each row of text.
+    //
+    // Discovering the exact amount of leading and padding around the font is
+    // a bit tricky and platform-specific, but this computation seems to work in
+    // practice.
+    OmniboxResultView* result_view = result_view_at(0);
+    outside_vertical_padding_ = (result_view->GetPreferredSize().height() -
+                                 result_view->GetTextHeight());
+  }
   return popup_height +
          views::NonClientFrameView::kClientEdgeThickness +  // Top border.
          outside_vertical_padding_ * 2 +                    // Padding.