[AiS] for desktop, two lines and font sytles

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

 // 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/views/omnibox/omnibox_popup_contents_view.h"
 
 #include <algorithm>
 
 #include "chrome/browser/search/search.h"
 #include "chrome/browser/themes/theme_properties.h"
 #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"
 #include "ui/gfx/path.h"
 #include "ui/resources/grit/ui_resources.h"
 #include "ui/views/controls/image_view.h"
 #include "ui/views/view_targeter.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/window/non_client_view.h"
 
@@ skipping 357 matching lines @@
   DCHECK_GE(static_cast<size_t>(child_count()), model_->result().size());
   int popup_height = 0;
   for (size_t i = model_->result().ShouldHideTopMatch() ? 1 : 0;
        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);
+  // The * 2 accounts for vertical padding used at the top and bottom of each
+  // row.
+
   outside_vertical_padding_ =
-      (result_view->GetPreferredSize().height() -
-       result_view->GetTextHeight());
-
+      OmniboxResultView::kMinimumTextVerticalPadding * 2;

[Peter Kasting, 2015/03/17 22:01:44]

Man, the more times I look at this code the more I find was wrong :P.

It turns out the * 2 is a mistake.  If we want the space between e.g. the bottom
row and the bottom border to match, we should just be using
kMinimumVerticalTextPadding directly.  This is because we'll already have one
copy of that padding present in the bottom row, so we just need one more.  I
verified on a Chrome screenshot that this would look more correct.

So what I would do is eliminate this member variable entirely, and use
OmniboxResultView::kMinimumTextVerticalPadding directly everywhere we currently
use |outside_vertical_padding_|.

[dschuyler, 2015/03/17 22:57:55]

It's mostly an aesthetic choice, so all the changes have been variations of
right (not wrong)  :)
Done.

   return popup_height +
          views::NonClientFrameView::kClientEdgeThickness +  // Top border.
          outside_vertical_padding_ * 2 +                    // Padding.
          bottom_shadow_->height() - kBorderInterior;        // Bottom border.
 }
 
 OmniboxResultView* OmniboxPopupContentsView::CreateResultView(
     int model_index,
     const gfx::FontList& font_list) {
   return new OmniboxResultView(this, model_index, location_bar_view_,
@@ skipping 98 matching lines @@
   size_t index = GetIndexForPoint(event.location());
   if (!HasMatchAt(index))
     return;
   omnibox_view_->OpenMatch(model_->result().match_at(index), disposition,
                            GURL(), base::string16(), index);
 }
 
 OmniboxResultView* OmniboxPopupContentsView::result_view_at(size_t i) {
   return static_cast<OmniboxResultView*>(child_at(static_cast<int>(i)));
 }