omnibox: Don't paint children from inside OnPaint().

OnPaint() is not part of the recursion, it won't have access to the
neccessary pieces to do recursion.

If you need to do painting while inside the recursion, this should
happen in PaintChildren(). So move the contents of OnPaint() to
PaintChildren().

R=pkasting
BUG=466426

Review URL: https://codereview.chromium.org/1016033011

Cr-Commit-Position: refs/heads/master@{#322520}

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

@@ -370,12 +370,6 @@ void OmniboxPopupContentsView::OnGestureEvent(ui::GestureEvent* event) {
 ////////////////////////////////////////////////////////////////////////////////
 // OmniboxPopupContentsView, protected:
 
-void OmniboxPopupContentsView::PaintResultViews(gfx::Canvas* canvas) {
-  canvas->DrawColor(result_view_at(0)->GetColor(
-      OmniboxResultView::NORMAL, OmniboxResultView::BACKGROUND));
-  View::PaintChildren(canvas, views::CullSet());
-}
-
 int OmniboxPopupContentsView::CalculatePopupHeight() {
   DCHECK_GE(static_cast<size_t>(child_count()), model_->result().size());
   int popup_height = 0;
@@ -409,19 +403,6 @@ const char* OmniboxPopupContentsView::GetClassName() const {
 }
 
 void OmniboxPopupContentsView::OnPaint(gfx::Canvas* canvas) {
-  gfx::Rect contents_bounds = GetContentsBounds();
-  contents_bounds.set_height(
-      contents_bounds.height() - bottom_shadow_->height() + kBorderInterior);
-
-  gfx::Path path;
-  MakeContentsPath(&path, contents_bounds);
-  canvas->Save();
-  canvas->sk_canvas()->clipPath(path,
-                                SkRegion::kIntersect_Op,
-                                true /* doAntialias */);
-  PaintResultViews(canvas);
-  canvas->Restore();
-
   // Top border.
   canvas->FillRect(
       gfx::Rect(0, 0, width(), views::NonClientFrameView::kClientEdgeThickness),
@@ -435,7 +416,18 @@ void OmniboxPopupContentsView::OnPaint(gfx::Canvas* canvas) {
 
 void OmniboxPopupContentsView::PaintChildren(gfx::Canvas* canvas,
                                              const views::CullSet& cull_set) {
-  // We paint our children inside OnPaint().
+  gfx::Rect contents_bounds = GetContentsBounds();
+  contents_bounds.Inset(0, views::NonClientFrameView::kClientEdgeThickness, 0,
+                        bottom_shadow_->height() - kBorderInterior);
+
+  canvas->Save();
+  canvas->sk_canvas()->clipRect(gfx::RectToSkRect(contents_bounds),
+                                SkRegion::kIntersect_Op,
+                                true /* doAntialias */);
+  canvas->DrawColor(result_view_at(0)->GetColor(OmniboxResultView::NORMAL,
+                                                OmniboxResultView::BACKGROUND));
+  View::PaintChildren(canvas, cull_set);
+  canvas->Restore();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -456,17 +448,6 @@ const AutocompleteMatch& OmniboxPopupContentsView::GetMatchAtIndex(
   return model_->result().match_at(index);
 }
 
-void OmniboxPopupContentsView::MakeContentsPath(
-    gfx::Path* path,
-    const gfx::Rect& bounding_rect) {
-  SkRect rect;
-  rect.set(SkIntToScalar(bounding_rect.x()),
-           SkIntToScalar(bounding_rect.y()),
-           SkIntToScalar(bounding_rect.right()),
-           SkIntToScalar(bounding_rect.bottom()));
-  path->addRect(rect);
-}
-
 size_t OmniboxPopupContentsView::GetIndexForPoint(
     const gfx::Point& point) {
   if (!HitTestPoint(point))

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

@@ -81,8 +81,6 @@ class OmniboxPopupContentsView : public views::View,
 
   LocationBarView* location_bar_view() { return location_bar_view_; }
 
-  virtual void PaintResultViews(gfx::Canvas* canvas);
-
   // Calculates the height needed to show all the results in the model.
   virtual int CalculatePopupHeight();
   virtual OmniboxResultView* CreateResultView(int model_index,
@@ -94,10 +92,6 @@ class OmniboxPopupContentsView : public views::View,
   // views::View:
   const char* GetClassName() const override;
   void OnPaint(gfx::Canvas* canvas) override;
-  // This method should not be triggered directly as we paint our children
-  // in an un-conventional way inside OnPaint. We use a separate canvas to
-  // paint the children. Hence we override this method to a no-op so that
-  // the view hierarchy does not "accidentally" trigger this.
   void PaintChildren(gfx::Canvas* canvas,
                      const views::CullSet& cull_set) override;
 
@@ -113,10 +107,6 @@ class OmniboxPopupContentsView : public views::View,
   // Returns the match at the specified index within the popup model.
   const AutocompleteMatch& GetMatchAtIndex(size_t index) const;
 
-  // Fill a path for the contents' roundrect. |bounding_rect| is the rect that
-  // bounds the path.
-  void MakeContentsPath(gfx::Path* path, const gfx::Rect& bounding_rect);
-
   // Find the index of the match under the given |point|, specified in window
   // coordinates. Returns OmniboxPopupModel::kNoMatch if there isn't a match at
   // the specified point.