Omnibox UI Experiments: Make suggestions dropdown match omnibox width.

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 "base/feature_list.h"
 #include "base/lazy_instance.h"
 #include "base/macros.h"
 #include "build/build_config.h"
 #include "chrome/browser/search/search.h"
 #include "chrome/browser/themes/theme_properties.h"
 #include "chrome/browser/ui/views/location_bar/location_bar_view.h"
 #include "chrome/browser/ui/views/omnibox/omnibox_result_view.h"
 #include "chrome/browser/ui/views/theme_copying_widget.h"
+#include "components/omnibox/browser/omnibox_field_trial.h"
 #include "components/omnibox/browser/omnibox_view.h"
 #include "third_party/skia/include/core/SkDrawLooper.h"
 #include "ui/base/theme_provider.h"
 #include "ui/compositor/clip_recorder.h"
 #include "ui/compositor/paint_recorder.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/geometry/safe_integer_conversions.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/image/image_skia_operations.h"
 #include "ui/gfx/path.h"
@@ skipping 421 matching lines @@
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // OmniboxPopupContentsView, views::View overrides, private:
 
 const char* OmniboxPopupContentsView::GetClassName() const {
   return "OmniboxPopupContentsView";
 }
 
 void OmniboxPopupContentsView::OnPaint(gfx::Canvas* canvas) {
-  canvas->TileImageInt(g_top_shadow.Get(), 0, 0, width(),
+  bool match_omnibox_width = base::FeatureList::IsEnabled(

[Peter Kasting, 2017/06/05 18:25:45]

It seems like the implementation in this file leaves the actual dropdown width
wide, but then draws the shadow and children more narrowly.

I would expect a simpler and clearer change, in the end, if instead the dropdown
itself was sized and positioned matching the omnibox, rather than just trying to
adjust where we paint.  Otherwise you're going to have other potential issues
with damage rects, event handling, etc. (and even if not, the intent just isn't
as clear).

[tommycli, 2017/06/05 22:39:21]

Done. Thank you for this feedback! This made things a lot simpler.

+      omnibox::kUIExperimentSuggestionsDropdownWidth);
+  int shadow_x = match_omnibox_width ? start_margin_ : 0;
+  int shadow_width =
+      match_omnibox_width ? width() - start_margin_ - end_margin_ : 0;
+
+  canvas->TileImageInt(g_top_shadow.Get(), shadow_x, 0, shadow_width,
                        g_top_shadow.Get().height());
-  canvas->TileImageInt(g_bottom_shadow.Get(), 0,
-                       height() - g_bottom_shadow.Get().height(), width(),
+  canvas->TileImageInt(g_bottom_shadow.Get(), shadow_x,
+                       height() - g_bottom_shadow.Get().height(), shadow_width,
                        g_bottom_shadow.Get().height());
 }
 
 void OmniboxPopupContentsView::PaintChildren(const ui::PaintContext& context) {
   gfx::Rect contents_bounds = GetContentsBounds();
-  contents_bounds.Inset(0, g_top_shadow.Get().height(), 0,
-                        g_bottom_shadow.Get().height());
+  bool match_omnibox_width = base::FeatureList::IsEnabled(
+      omnibox::kUIExperimentSuggestionsDropdownWidth);
+  int contents_start = match_omnibox_width ? start_margin_ : 0;
+  int contents_end = match_omnibox_width ? end_margin_ : 0;
+
+  contents_bounds.Inset(contents_start, g_top_shadow.Get().height(),
+                        contents_end, g_bottom_shadow.Get().height());
 
   ui::ClipRecorder clip_recorder(context);
   clip_recorder.ClipRect(contents_bounds);
   {
     ui::PaintRecorder recorder(context, size());
     SkColor background_color = result_view_at(0)->GetColor(
         OmniboxResultView::NORMAL, OmniboxResultView::BACKGROUND);
     recorder.canvas()->DrawColor(background_color);
   }
   View::PaintChildren(context);
@@ skipping 49 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)));
 }