Simplification of background-painting logic for bookmarks bar and

toolbar.

This eliminates a lot of complicated math in favor of just finding the
offset of the child (bookmarks bar, toolbar) into the background image
and painting the view background.

Bug: 993502
Change-Id: I0502a1468abfe50ea919de863244f0bd5558d4b6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1847775
Commit-Queue: Dana Fried <dfried@chromium.org>
Reviewed-by: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704048}

chrome/browser/ui/BUILD.gn

@@ -2794,6 +2794,8 @@ jumbo_split_static_library("ui") {
       "views/frame/terminal_system_app_menu_button_chromeos.cc",
       "views/frame/terminal_system_app_menu_button_chromeos.h",
       "views/frame/toolbar_button_provider.h",
+      "views/frame/top_container_background.cc",
+      "views/frame/top_container_background.h",
       "views/frame/top_container_view.cc",
       "views/frame/top_container_view.h",
       "views/frame/top_controls_slide_controller.h",

chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc

@@ -47,6 +47,7 @@
 #include "chrome/browser/ui/views/chrome_layout_provider.h"
 #include "chrome/browser/ui/views/event_utils.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
+#include "chrome/browser/ui/views/frame/top_container_background.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_ink_drop_util.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "chrome/common/chrome_switches.h"
@@ -533,6 +534,10 @@ BookmarkBarView::BookmarkBarView(Browser* browser, BrowserView* browser_view)
   size_animation_.Reset(1);
   if (!gfx::Animation::ShouldRenderRichAnimation())
     animations_enabled = false;
+
+  // May be null for tests.
+  if (browser_view)
+    SetBackground(std::make_unique<TopContainerBackground>(browser_view));
 }
 
 BookmarkBarView::~BookmarkBarView() {

chrome/browser/ui/views/frame/browser_view.cc

@@ -256,38 +256,6 @@ void InsertIntoFocusOrderAfter(views::View* insert_after,
     old_prev->SetNextFocusableView(old_next);
 }
 
-// Paints the background (including the theme image behind content area) for
-// the Bookmarks Bar.
-// |background_origin| is the origin to use for painting the theme image.
-void PaintBackground(gfx::Canvas* canvas,
-                     const ui::ThemeProvider* theme_provider,
-                     const gfx::Rect& bounds,
-                     const gfx::Point& background_origin) {
-  canvas->DrawColor(theme_provider->GetColor(ThemeProperties::COLOR_TOOLBAR));
-
-  // If there's a non-default background image, tile it.
-  if (theme_provider->HasCustomImage(IDR_THEME_TOOLBAR)) {
-    canvas->TileImageInt(*theme_provider->GetImageSkiaNamed(IDR_THEME_TOOLBAR),
-                         background_origin.x(),
-                         background_origin.y(),
-                         bounds.x(),
-                         bounds.y(),
-                         bounds.width(),
-                         bounds.height());
-  }
-}
-
-void PaintBookmarkBar(gfx::Canvas* canvas,
-                      BookmarkBarView* view,
-                      BrowserView* browser_view) {
-  // Paint background for attached state.
-  gfx::Point background_image_offset =
-      browser_view->OffsetPointForToolbarBackgroundImage(
-          gfx::Point(view->GetMirroredX(), view->y()));
-  PaintBackground(canvas, view->GetThemeProvider(), view->GetLocalBounds(),
-                  background_image_offset);
-}
-
 bool GetGestureCommand(ui::GestureEvent* event, int* command) {
   DCHECK(command);
   *command = 0;
@@ -417,34 +385,6 @@ class BrowserViewLayoutDelegateImpl : public BrowserViewLayoutDelegate {
   DISALLOW_COPY_AND_ASSIGN(BrowserViewLayoutDelegateImpl);
 };
 
-// This class is used to paint the background for Bookmarks Bar.
-class BookmarkBarViewBackground : public views::Background {
- public:
-  BookmarkBarViewBackground(BrowserView* browser_view,
-                            BookmarkBarView* bookmark_bar_view);
-
-  // views:Background:
-  void Paint(gfx::Canvas* canvas, views::View* view) const override;
-
- private:
-  BrowserView* browser_view_;
-
-  // The view hosting this background.
-  BookmarkBarView* bookmark_bar_view_;
-
-  DISALLOW_COPY_AND_ASSIGN(BookmarkBarViewBackground);
-};
-
-BookmarkBarViewBackground::BookmarkBarViewBackground(
-    BrowserView* browser_view,
-    BookmarkBarView* bookmark_bar_view)
-    : browser_view_(browser_view), bookmark_bar_view_(bookmark_bar_view) {}
-
-void BookmarkBarViewBackground::Paint(gfx::Canvas* canvas,
-                                      views::View* view) const {
-  PaintBookmarkBar(canvas, bookmark_bar_view_, browser_view_);
-}
-
 ///////////////////////////////////////////////////////////////////////////////
 // BrowserView, public:
 
@@ -548,17 +488,6 @@ int BrowserView::GetTabStripHeight() const {
   return IsTabStripVisible() ? tabstrip_->GetPreferredSize().height() : 0;
 }
 
-gfx::Point BrowserView::OffsetPointForToolbarBackgroundImage(
-    const gfx::Point& point) const {
-  // The background image starts tiling horizontally at the window left edge and
-  // vertically at the top edge of the horizontal tab strip (or where it would
-  // be).  We expect our parent's origin to be the window origin.
-  gfx::Point window_point(point + GetMirroredPosition().OffsetFromOrigin());
-  window_point.Offset(frame_->GetThemeBackgroundXInset(),
-                      -frame_->GetTopInset());
-  return window_point;
-}
-
 bool BrowserView::IsTabStripVisible() const {
   // Return false if this window does not normally display a tabstrip.
   if (!browser_->SupportsWindowFeature(Browser::FEATURE_TABSTRIP))
@@ -2621,8 +2550,8 @@ void BrowserView::InitViews() {
   }
 #endif  // BUILDFLAG(ENABLE_WEBUI_TAB_STRIP)
 
-  toolbar_ = new ToolbarView(browser_.get(), this);
-  top_container_->AddChildView(toolbar_);
+  toolbar_ = top_container_->AddChildView(
+      std::make_unique<ToolbarView>(browser_.get(), this));
   toolbar_->Init();
 
   contents_separator_ =
@@ -2743,9 +2672,6 @@ bool BrowserView::MaybeShowBookmarkBar(WebContents* contents) {
     bookmark_bar_view_ =
         std::make_unique<BookmarkBarView>(browser_.get(), this);
     bookmark_bar_view_->set_owned_by_client();
-    bookmark_bar_view_->SetBackground(
-        std::make_unique<BookmarkBarViewBackground>(this,
-                                                    bookmark_bar_view_.get()));
     bookmark_bar_view_->SetBookmarkBarState(
         browser_->bookmark_bar_state(),
         BookmarkBar::DONT_ANIMATE_STATE_CHANGE);

chrome/browser/ui/views/frame/browser_view.h

@@ -156,12 +156,6 @@ class BrowserView : public BrowserWindow,
   // incognito avatar icon.
   int GetTabStripHeight() const;
 
-  // Takes some view's origin (relative to this BrowserView) and offsets it such
-  // that it can be used as the source origin for seamlessly tiling the toolbar
-  // background image over that view.
-  gfx::Point OffsetPointForToolbarBackgroundImage(
-      const gfx::Point& point) const;
-
   // Container for the tabstrip, toolbar, etc.
   TopContainerView* top_container() { return top_container_; }
 

chrome/browser/ui/views/frame/top_container_background.cc

+// Copyright 2019 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/frame/top_container_background.h"
+
+#include "chrome/browser/themes/theme_properties.h"
+#include "chrome/browser/ui/views/frame/browser_view.h"
+#include "chrome/grit/theme_resources.h"
+#include "ui/base/theme_provider.h"
+
+TopContainerBackground::TopContainerBackground(BrowserView* browser_view)
+    : browser_view_(browser_view) {}
+
+void TopContainerBackground::Paint(gfx::Canvas* canvas,
+                                   views::View* view) const {
+  const ui::ThemeProvider* const theme_provider = view->GetThemeProvider();
+  if (theme_provider->HasCustomImage(IDR_THEME_TOOLBAR)) {
+    // Calculate the offset of the upper-left corner of the owner view in the
+    // browser view. Not all views are directly parented to the browser, so we
+    // have to walk up the view hierarchy. This will tell us the offset into the
+    // tiled image that will correspond to the upper-left corner of the view.
+    int x = 0;
+    int y = 0;
+    views::View* current = view;
+    for (; current != browser_view_; current = current->parent()) {
+      x += current->x();
+      y += current->y();
+    }
+    x += browser_view_->frame()->GetThemeBackgroundXInset();
+
+    // Start tiling from coordinates (x, y) in the image into local space,
+    // filling the entire local bounds of the owner view.
+    canvas->TileImageInt(*theme_provider->GetImageSkiaNamed(IDR_THEME_TOOLBAR),
+                         x, y, 0, 0, view->width(), view->height());
+  } else {
+    canvas->DrawColor(theme_provider->GetColor(ThemeProperties::COLOR_TOOLBAR));
+  }
+}

chrome/browser/ui/views/frame/top_container_background.h

+// Copyright 2019 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.
+
+#ifndef CHROME_BROWSER_UI_VIEWS_FRAME_TOP_CONTAINER_BACKGROUND_H_
+#define CHROME_BROWSER_UI_VIEWS_FRAME_TOP_CONTAINER_BACKGROUND_H_
+
+#include "ui/views/background.h"
+
+class BrowserView;
+
+// Background which renders the appropriate toolbar/bookmarks/etc. background
+// on a view which must be a descendant of the browser view in the hierarchy. If
+// there is a background image, it will be painted or tiled appropriately.
+class TopContainerBackground : public views::Background {
+ public:
+  // Construct a themed background for the specified browser.
+  explicit TopContainerBackground(BrowserView* browser_view);
+
+ private:
+  // views::Background:
+  void Paint(gfx::Canvas* canvas, views::View* view) const override;
+
+  BrowserView* const browser_view_;
+
+  // Disallow copy and assign.
+  TopContainerBackground(const TopContainerBackground& other) = delete;
+  TopContainerBackground& operator=(const TopContainerBackground& other) =
+      delete;
+};
+
+#endif  // CHROME_BROWSER_UI_VIEWS_FRAME_TOP_CONTAINER_BACKGROUND_H_

chrome/browser/ui/views/toolbar/toolbar_view.cc

@@ -40,6 +40,7 @@
 #include "chrome/browser/ui/views/extensions/extensions_toolbar_container.h"
 #include "chrome/browser/ui/views/frame/browser_non_client_frame_view.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
+#include "chrome/browser/ui/views/frame/top_container_background.h"
 #include "chrome/browser/ui/views/global_media_controls/media_toolbar_button_view.h"
 #include "chrome/browser/ui/views/location_bar/star_view.h"
 #include "chrome/browser/ui/views/media_router/cast_toolbar_button.h"
@@ -149,6 +150,9 @@ ToolbarView::ToolbarView(Browser* browser, BrowserView* browser_view)
 
   UpgradeDetector::GetInstance()->AddObserver(this);
   md_observer_.Add(ui::MaterialDesignController::GetInstance());
+
+  if (display_mode_ == DisplayMode::NORMAL)
+    SetBackground(std::make_unique<TopContainerBackground>(browser_view));
 }
 
 ToolbarView::~ToolbarView() {
@@ -632,30 +636,6 @@ void ToolbarView::Layout() {
   AccessiblePaneView::Layout();
 }
 
-void ToolbarView::OnPaintBackground(gfx::Canvas* canvas) {
-  if (display_mode_ != DisplayMode::NORMAL)
-    return;
-
-  const ui::ThemeProvider* tp = GetThemeProvider();
-
-  // If the toolbar has a theme image, it gets composited against the toolbar
-  // background color when it's imported, so we only need to specificallh draw
-  // the background color if there is no custom image.
-  if (tp->HasCustomImage(IDR_THEME_TOOLBAR)) {
-    const int x_offset =
-        GetMirroredX() + browser_view_->GetMirroredX() +
-        browser_view_->frame()->GetFrameView()->GetThemeBackgroundXInset();
-    const int y_offset = GetLayoutConstant(TAB_HEIGHT) -
-                         browser_view_->tabstrip()->GetStrokeThickness() -
-                         GetLayoutConstant(TABSTRIP_TOOLBAR_OVERLAP);
-    canvas->TileImageInt(*tp->GetImageSkiaNamed(IDR_THEME_TOOLBAR), x_offset,
-                         y_offset, 0, 0, width(), height());
-  } else {
-    canvas->FillRect(GetLocalBounds(),
-                     tp->GetColor(ThemeProperties::COLOR_TOOLBAR));
-  }
-}
-
 void ToolbarView::OnThemeChanged() {
   if (!initialized_)
     return;

chrome/browser/ui/views/toolbar/toolbar_view.h

@@ -192,7 +192,6 @@ class ToolbarView : public views::AccessiblePaneView,
   gfx::Size CalculatePreferredSize() const override;
   gfx::Size GetMinimumSize() const override;
   void Layout() override;
-  void OnPaintBackground(gfx::Canvas* canvas) override;
   void OnThemeChanged() override;
   const char* GetClassName() const override;
   bool AcceleratorPressed(const ui::Accelerator& acc) override;