Consolidate calls to kUIExperimentNarrowDropdown into omnibox_popup_contents_view.cc

features::kTouchableChrome gets more creative, but the narrow dropdown
is a good starting point. That is, features::kTouchableChrome may imply
a narrow dropdown regardless of kUIExperimentNarrowDropdown.

Consolidate the logic under c/b/ui/views before it gets more complex.
See linked bugs for context and links the the big picture CL.

Bug: 801583, 728844
Change-Id: I0b6ca6f8f9c6cded9ee2b1ccf0e2b8b34e737cd9
Reviewed-on: https://chromium-review.googlesource.com/892603Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Reviewed-by: Tommy Li <tommycli@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533084}

chrome/browser/ui/views/location_bar/location_bar_view.cc

@@ -8,7 +8,6 @@
 #include <map>
 #include <memory>
 
-#include "base/feature_list.h"
 #include "base/i18n/rtl.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
@@ -57,7 +56,6 @@
 #include "chrome/grit/generated_resources.h"
 #include "components/bookmarks/common/bookmark_pref_names.h"
 #include "components/favicon/content/content_favicon_driver.h"
-#include "components/omnibox/browser/omnibox_field_trial.h"
 #include "components/omnibox/browser/omnibox_popup_model.h"
 #include "components/omnibox/browser/omnibox_popup_view.h"
 #include "components/prefs/pref_service.h"
@@ -375,33 +373,6 @@ views::View* LocationBarView::GetSecurityBubbleAnchorView() {
   return location_icon_view()->GetImageView();
 }
 
-void LocationBarView::GetOmniboxPopupPositioningInfo(
-    gfx::Point* top_left_screen_coord,
-    int* popup_width,
-    int* left_margin,
-    int* right_margin,
-    int top_edge_overlap) {
-  // The popup contents are always sized matching the location bar size.
-  const int popup_contents_left = x();
-  const int popup_contents_right = bounds().right();
-
-  // The popup itself may either be the same width as the contents, or as wide
-  // as the toolbar.
-  bool narrow_popup =
-      base::FeatureList::IsEnabled(omnibox::kUIExperimentNarrowDropdown);
-  const int popup_left = narrow_popup ? popup_contents_left : 0;
-  const int popup_right =
-      narrow_popup ? popup_contents_right : parent()->width();
-
-  *top_left_screen_coord =
-      gfx::Point(popup_left, parent()->height() - top_edge_overlap);
-  views::View::ConvertPointToScreen(parent(), top_left_screen_coord);
-
-  *popup_width = popup_right - popup_left;
-  *left_margin = popup_contents_left - popup_left;
-  *right_margin = popup_right - popup_contents_right;
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 // LocationBarView, public LocationBar implementation:
 

chrome/browser/ui/views/location_bar/location_bar_view.h

@@ -206,17 +206,6 @@ class LocationBarView : public LocationBar,
   OmniboxViewViews* omnibox_view() { return omnibox_view_; }
   const OmniboxViewViews* omnibox_view() const { return omnibox_view_; }
 
-  // Returns the position and width that the popup should be, and also the left
-  // edge that the results should align themselves to (which will leave some
-  // border on the left of the popup). |top_edge_overlap| specifies the number
-  // of pixels the top edge of the popup should overlap the bottom edge of
-  // the toolbar.
-  void GetOmniboxPopupPositioningInfo(gfx::Point* top_left_screen_coord,
-                                      int* popup_width,
-                                      int* left_margin,
-                                      int* right_margin,
-                                      int top_edge_overlap);
-
   // Updates the controller, and, if |contents| is non-null, restores saved
   // state that the tab holds.
   void Update(const content::WebContents* contents);

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

@@ -43,7 +43,7 @@ base::LazyInstance<gfx::ImageSkia>::DestructorAtExit g_top_shadow =
 base::LazyInstance<gfx::ImageSkia>::DestructorAtExit g_bottom_shadow =
     LAZY_INSTANCE_INITIALIZER;
 
-const int kPopupVerticalPadding = 4;
+constexpr int kPopupVerticalPadding = 4;
 
 }  // namespace
 
@@ -77,7 +77,7 @@ OmniboxPopupContentsView::OmniboxPopupContentsView(
   // The contents is owned by the LocationBarView.
   set_owned_by_client();
 
-  bool narrow_popup =
+  const bool narrow_popup =
       base::FeatureList::IsEnabled(omnibox::kUIExperimentNarrowDropdown);
 
   if (g_top_shadow.Get().isNull() && !narrow_popup) {
@@ -218,26 +218,8 @@ void OmniboxPopupContentsView::UpdatePopupAppearance() {
   for (size_t i = result_size; i < AutocompleteResult::GetMaxMatches(); ++i)
     child_at(i)->SetVisible(false);
 
-  int top_edge_overlap = 0;
-  bool narrow_popup =
-      base::FeatureList::IsEnabled(omnibox::kUIExperimentNarrowDropdown);
-  if (!narrow_popup) {
-    // We want the popup to appear to overlay the bottom of the toolbar. So we
-    // shift the popup to completely cover the client edge, and then draw an
-    // additional semitransparent shadow above that.
-    top_edge_overlap = g_top_shadow.Get().height() +
-                       views::NonClientFrameView::kClientEdgeThickness;
-  }
-
-  gfx::Point top_left_screen_coord;
-  int width;
-  location_bar_view_->GetOmniboxPopupPositioningInfo(
-      &top_left_screen_coord, &width, &start_margin_,
-      &end_margin_, top_edge_overlap);
-  gfx::Rect new_target_bounds(top_left_screen_coord,
-                              gfx::Size(width, CalculatePopupHeight()));
-
-  if (narrow_popup) {
+  gfx::Rect new_target_bounds = UpdateMarginsAndGetTargetBounds();
+  if (base::FeatureList::IsEnabled(omnibox::kUIExperimentNarrowDropdown)) {
     SkColor background_color = GetNativeTheme()->GetSystemColor(
         ui::NativeTheme::kColorId_ResultsTableNormalBackground);
     auto border = std::make_unique<views::BubbleBorder>(
@@ -399,6 +381,40 @@ void OmniboxPopupContentsView::OnGestureEvent(ui::GestureEvent* event) {
 ////////////////////////////////////////////////////////////////////////////////
 // OmniboxPopupContentsView, private:
 
+gfx::Rect OmniboxPopupContentsView::UpdateMarginsAndGetTargetBounds() {
+  int top_edge_overlap = 0;
+  // The popup itself may either be the same width as the contents, or as wide
+  // as the toolbar.
+  const bool narrow_popup =
+      base::FeatureList::IsEnabled(omnibox::kUIExperimentNarrowDropdown);
+  if (!narrow_popup) {
+    // We want the popup to appear to overlay the bottom of the toolbar. So we
+    // shift the popup to completely cover the client edge, and then draw an
+    // additional semitransparent shadow above that.
+    top_edge_overlap = g_top_shadow.Get().height() +
+                       views::NonClientFrameView::kClientEdgeThickness;
+  }
+
+  views::View* toolbar = location_bar_view_->parent();
+
+  // The popup contents are always sized matching the location bar size.
+  const int popup_contents_left = location_bar_view_->x();
+  const int popup_contents_right = location_bar_view_->bounds().right();
+  const int popup_left = narrow_popup ? popup_contents_left : 0;
+  const int popup_right =
+      narrow_popup ? popup_contents_right : toolbar->width();
+  const int width = popup_right - popup_left;
+
+  start_margin_ = popup_contents_left - popup_left;
+  end_margin_ = popup_right - popup_contents_right;
+
+  gfx::Point top_left_screen_coord =
+      gfx::Point(popup_left, toolbar->height() - top_edge_overlap);
+  views::View::ConvertPointToScreen(toolbar, &top_left_screen_coord);
+  return gfx::Rect(top_left_screen_coord,
+                   gfx::Size(width, CalculatePopupHeight()));
+}
+
 int OmniboxPopupContentsView::CalculatePopupHeight() {
   DCHECK_GE(static_cast<size_t>(child_count()), model_->result().size());
   int popup_height = 0;

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

@@ -77,6 +77,11 @@ class OmniboxPopupContentsView : public views::View,
  private:
   class AutocompletePopupWidget;
 
+  // Updates |start_margin_| and |end_margin_| and returns the target popup
+  // bounds by querying the bounds of |location_bar_view_| and its parent view
+  // on screen.
+  gfx::Rect UpdateMarginsAndGetTargetBounds();
+
   // Calculates the height needed to show all the results in the model.
   int CalculatePopupHeight();