Factor out the layout code from OpaqueBrowserFrameView for testing.

As is, OpaqueBrowserFrameView is untestable due to its
dependencies. This patch separates out all the layout and sizing logic
into a LayoutManager subclass, makes OpaqueBrowserFrameView use that,
and then adds tests for the new LayoutManager.

Note that I've deliberately made as few cleanups to the layout logic as
possible. The point is to get this code under test before I start more
radical changes.

BUG=281788

Review URL: https://chromiumcodereview.appspot.com/23531006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@220683 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/ui/view_ids.h

@@ -14,6 +14,17 @@ enum ViewID {
   // BROWSER WINDOW VIEWS
   // ------------------------------------------------------
 
+  // Views which make up the skyline. These are used only
+  // on views.
+  VIEW_ID_MINIMIZE_BUTTON,
+  VIEW_ID_MAXIMIZE_BUTTON,
+  VIEW_ID_RESTORE_BUTTON,
+  VIEW_ID_CLOSE_BUTTON,
+  VIEW_ID_WINDOW_ICON,
+  VIEW_ID_WINDOW_TITLE,
+  VIEW_ID_AVATAR_LABEL,
+  VIEW_ID_AVATAR_BUTTON,
+
   // Tabs within a window/tab strip, counting from the left.
   VIEW_ID_TAB_0,
   VIEW_ID_TAB_1,

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

@@ -9,6 +9,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_info_cache.h"
 #include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/ui/view_ids.h"
 #include "chrome/browser/ui/views/avatar_label.h"
 #include "chrome/browser/ui/views/avatar_menu_button.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
@@ -54,11 +55,13 @@ void BrowserNonClientFrameView::UpdateAvatarInfo() {
       Profile* profile = browser_view_->browser()->profile();
       if (profile->IsManaged() && !avatar_label_) {
         avatar_label_ = new AvatarLabel(browser_view_);
+        avatar_label_->set_id(VIEW_ID_AVATAR_LABEL);
         AddChildView(avatar_label_);
       }
       avatar_button_ = new AvatarMenuButton(
           browser_view_->browser(),
           browser_view_->IsOffTheRecord() && !browser_view_->IsGuestSession());
+      avatar_button_->set_id(VIEW_ID_AVATAR_BUTTON);
       AddChildView(avatar_button_);
       frame_->GetRootView()->Layout();
     }

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

@@ -6,8 +6,10 @@
 #define CHROME_BROWSER_UI_VIEWS_FRAME_OPAQUE_BROWSER_FRAME_VIEW_H_
 
 #include "base/memory/scoped_ptr.h"
+#include "chrome/browser/ui/view_ids.h"
 #include "chrome/browser/ui/views/frame/browser_frame.h"
 #include "chrome/browser/ui/views/frame/browser_non_client_frame_view.h"
+#include "chrome/browser/ui/views/frame/opaque_browser_frame_view_layout_delegate.h"
 #include "chrome/browser/ui/views/tab_icon_view_model.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
@@ -15,6 +17,7 @@
 #include "ui/views/window/non_client_view.h"
 
 class BrowserView;
+class OpaqueBrowserFrameViewLayout;
 class TabIconView;
 
 namespace views {
@@ -26,7 +29,8 @@ class Label;
 class OpaqueBrowserFrameView : public BrowserNonClientFrameView,
                                public content::NotificationObserver,
                                public views::ButtonListener,
-                               public chrome::TabIconViewModel {
+                               public chrome::TabIconViewModel,
+                               public OpaqueBrowserFrameViewLayoutDelegate {
  public:
   // Constructs a non-client view for an BrowserFrame.
   OpaqueBrowserFrameView(BrowserFrame* frame, BrowserView* browser_view);
@@ -45,16 +49,6 @@ class OpaqueBrowserFrameView : public BrowserNonClientFrameView,
   views::ImageButton* restore_button() const { return restore_button_; }
   views::ImageButton* close_button() const { return close_button_; }
 
-  // Used to allow subclasses to reserve height for other components they
-  // will add.  The space is reserved below the ClientView.
-  virtual int GetReservedHeight() const;
-  virtual gfx::Rect GetBoundsForReservedArea() const;
-
-  // Returns the height of the entire nonclient top border, including the window
-  // frame, any title area, and any connected client edge.  If |restored| is
-  // true, acts as if the window is restored regardless of the real mode.
-  int NonClientTopBorderHeight(bool restored) const;
-
   // Overridden from views::NonClientFrameView:
   virtual gfx::Rect GetBoundsForClientView() const OVERRIDE;
   virtual gfx::Rect GetWindowBoundsForClientBounds(
@@ -68,7 +62,6 @@ class OpaqueBrowserFrameView : public BrowserNonClientFrameView,
 
   // Overridden from views::View:
   virtual void OnPaint(gfx::Canvas* canvas) OVERRIDE;
-  virtual void Layout() OVERRIDE;
   virtual bool HitTestRect(const gfx::Rect& rect) const OVERRIDE;
   virtual void GetAccessibleState(ui::AccessibleViewState* state) OVERRIDE;
 
@@ -85,6 +78,23 @@ class OpaqueBrowserFrameView : public BrowserNonClientFrameView,
                        const content::NotificationSource& source,
                        const content::NotificationDetails& details) OVERRIDE;
 
+  // OpaqueBrowserFrameViewLayoutDelegate implementation:
+  virtual bool ShouldShowWindowIcon() const OVERRIDE;
+  virtual bool ShouldShowWindowTitle() const OVERRIDE;
+  virtual string16 GetWindowTitle() const OVERRIDE;
+  virtual int GetIconSize() const OVERRIDE;
+  virtual bool ShouldLeaveOffsetNearTopBorder() const OVERRIDE;
+  virtual gfx::Size GetBrowserViewMinimumSize() const OVERRIDE;
+  virtual bool ShouldShowAvatar() const OVERRIDE;
+  virtual gfx::ImageSkia GetOTRAvatarIcon() const OVERRIDE;
+  virtual bool IsMaximized() const OVERRIDE;
+  virtual bool IsMinimized() const OVERRIDE;
+  virtual bool IsFullscreen() const OVERRIDE;
+  virtual bool IsTabStripVisible() const OVERRIDE;
+  virtual int GetTabStripHeight() const OVERRIDE;
+  virtual int GetAdditionalReservedSpaceInTabStrip() const OVERRIDE;
+  virtual gfx::Size GetTabstripPreferredSize() const OVERRIDE;
+
  private:
   // Creates, adds and returns a new image button with |this| as its listener.
   // Memory is owned by the caller.
@@ -92,7 +102,8 @@ class OpaqueBrowserFrameView : public BrowserNonClientFrameView,
                                               int hot_image_id,
                                               int pushed_image_id,
                                               int mask_image_id,
-                                              int accessibility_string_id);
+                                              int accessibility_string_id,
+                                              ViewID view_id);
 
   // Returns the thickness of the border that makes up the window frame edges.
   // This does not include any client edge.  If |restored| is true, acts as if
@@ -107,26 +118,10 @@ class OpaqueBrowserFrameView : public BrowserNonClientFrameView,
   // borders, including both the window frame and any client edge.
   int NonClientBorderThickness() const;
 
-  // Returns the y-coordinate of the caption buttons.  If |restored| is true,
-  // acts as if the window is restored regardless of the real mode.
-  int CaptionButtonY(bool restored) const;
-
-  // Returns the thickness of the 3D edge along the bottom of the titlebar.  If
-  // |restored| is true, acts as if the window is restored regardless of the
-  // real mode.
-  int TitlebarBottomThickness(bool restored) const;
-
-  // Returns the size of the titlebar icon.  This is used even when the icon is
-  // not shown, e.g. to set the titlebar height.
-  int IconSize() const;
-
   // Returns the bounds of the titlebar icon (or where the icon would be if
   // there was one).
   gfx::Rect IconBounds() const;
 
-  // Returns the combined bounds for the tab strip and avatar area.
-  gfx::Rect GetBoundsForTabStripAndAvatarArea(views::View* tabstrip) const;
-
   // Paint various sub-components of this view.  The *FrameBorder() functions
   // also paint the background of the titlebar area, since the top frame border
   // and titlebar background are a contiguous component.
@@ -141,16 +136,11 @@ class OpaqueBrowserFrameView : public BrowserNonClientFrameView,
   gfx::ImageSkia* GetFrameOverlayImage() const;
   int GetTopAreaHeight() const;
 
-  // Layout various sub-components of this view.
-  void LayoutWindowControls();
-  void LayoutTitleBar();
-  void LayoutAvatar();
-
   // Returns the bounds of the client area for the specified view size.
   gfx::Rect CalculateClientAreaBounds(int width, int height) const;
 
-  // The layout rect of the avatar icon, if visible.
-  gfx::Rect avatar_bounds_;
+  // Our layout manager also calculates various bounds.
+  OpaqueBrowserFrameViewLayout* layout_;
 
   // Window controls.
   views::ImageButton* minimize_button_;
@@ -162,9 +152,6 @@ class OpaqueBrowserFrameView : public BrowserNonClientFrameView,
   TabIconView* window_icon_;
   views::Label* window_title_;
 
-  // The bounds of the ClientView.
-  gfx::Rect client_view_bounds_;
-
   content::NotificationRegistrar registrar_;
 
   // Background painter for the window frame.

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

+// Copyright 2013 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_OPAQUE_BROWSER_FRAME_VIEW_LAYOUT_H_
+#define CHROME_BROWSER_UI_VIEWS_FRAME_OPAQUE_BROWSER_FRAME_VIEW_LAYOUT_H_
+
+#include "chrome/browser/ui/views/frame/opaque_browser_frame_view.h"
+#include "ui/views/layout/layout_manager.h"
+
+class OpaqueBrowserFrameViewLayoutDelegate;
+
+namespace views {
+class ImageButton;
+class Label;
+}
+
+// Calculates the position of the widgets in the opaque browser frame view.
+//
+// This is separated out for testing reasons. OpaqueBrowserFrameView has tight
+// dependencies with Browser and classes that depend on Browser.
+class OpaqueBrowserFrameViewLayout : public views::LayoutManager {
+ public:
+  explicit OpaqueBrowserFrameViewLayout(
+      OpaqueBrowserFrameViewLayoutDelegate* delegate);
+  virtual ~OpaqueBrowserFrameViewLayout();
+
+  // Whether we should add the (minimize,maximize,close) buttons. Can be false
+  // on Windows 8 in metro mode.
+  static bool ShouldAddDefaultCaptionButtons();
+
+  gfx::Rect GetBoundsForTabStrip(
+      const gfx::Size& tabstrip_preferred_size,
+      int available_width) const;
+  gfx::Rect GetBoundsForTabStripAndAvatarArea(
+      const gfx::Size& tabstrip_preferred_size,
+      int available_width) const;
+
+  gfx::Size GetMinimumSize(int available_width) const;
+
+  // Returns the bounds of the window required to display the content area at
+  // the specified bounds.
+  gfx::Rect GetWindowBoundsForClientBounds(
+      const gfx::Rect& client_bounds) const;
+
+  // Returns the thickness of the border that makes up the window frame edges.
+  // This does not include any client edge.  If |restored| is true, acts as if
+  // the window is restored regardless of the real mode.
+  int FrameBorderThickness(bool restored) const;
+
+  // Returns the thickness of the entire nonclient left, right, and bottom
+  // borders, including both the window frame and any client edge.
+  int NonClientBorderThickness() const;
+
+  // Returns the height of the entire nonclient top border, including the window
+  // frame, any title area, and any connected client edge.  If |restored| is
+  // true, acts as if the window is restored regardless of the real mode.
+  int NonClientTopBorderHeight(bool restored) const;
+
+  int GetTabStripInsetsTop(bool restored) const;
+
+  // Returns the y-coordinate of the caption buttons.  If |restored| is true,
+  // acts as if the window is restored regardless of the real mode.
+  int CaptionButtonY(bool restored) const;
+
+  // Returns the thickness of the 3D edge along the bottom of the titlebar.  If
+  // |restored| is true, acts as if the window is restored regardless of the
+  // real mode.
+  int TitlebarBottomThickness(bool restored) const;
+
+  // Returns the bounds of the titlebar icon (or where the icon would be if
+  // there was one).
+  gfx::Rect IconBounds() const;
+
+  // Returns the bounds of the client area for the specified view size.
+  gfx::Rect CalculateClientAreaBounds(int width, int height) const;
+
+  const gfx::Rect& client_view_bounds() const { return client_view_bounds_; }
+
+ private:
+  // Layout various sub-components of this view.
+  void LayoutWindowControls(views::View* host);
+  void LayoutTitleBar();
+  void LayoutAvatar();
+
+  // Internal implementation of ViewAdded() and ViewRemoved().
+  void SetView(int id, views::View* view);
+
+  // Overriden from views::LayoutManager:
+  virtual void Layout(views::View* host) OVERRIDE;
+  virtual gfx::Size GetPreferredSize(views::View* host) OVERRIDE;
+  virtual void ViewAdded(views::View* host, views::View* view) OVERRIDE;
+  virtual void ViewRemoved(views::View* host, views::View* view) OVERRIDE;
+
+  OpaqueBrowserFrameViewLayoutDelegate* delegate_;
+
+  // The layout rect of the avatar icon, if visible.
+  gfx::Rect avatar_bounds_;
+
+  // The bounds of the ClientView.
+  gfx::Rect client_view_bounds_;
+
+  // Window controls.
+  views::ImageButton* minimize_button_;
+  views::ImageButton* maximize_button_;
+  views::ImageButton* restore_button_;
+  views::ImageButton* close_button_;
+
+  views::View* window_icon_;
+  views::Label* window_title_;
+
+  views::View* avatar_label_;
+  views::View* avatar_button_;
+
+  DISALLOW_COPY_AND_ASSIGN(OpaqueBrowserFrameViewLayout);
+};
+
+#endif  // CHROME_BROWSER_UI_VIEWS_FRAME_OPAQUE_BROWSER_FRAME_VIEW_LAYOUT_H_

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

+// Copyright 2013 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_OPAQUE_BROWSER_FRAME_VIEW_LAYOUT_DELEGATE_H_
+#define CHROME_BROWSER_UI_VIEWS_FRAME_OPAQUE_BROWSER_FRAME_VIEW_LAYOUT_DELEGATE_H_
+
+namespace gfx {
+class Size;
+}
+
+// Delegate interface to control layout decisions without having to depend on
+// Browser{,Frame,View}.
+class OpaqueBrowserFrameViewLayoutDelegate {
+ public:
+  // Controls the visual placement of the window icon/title in non-tabstrip
+  // mode.
+  virtual bool ShouldShowWindowIcon() const = 0;
+  virtual bool ShouldShowWindowTitle() const = 0;
+  virtual string16 GetWindowTitle() const = 0;
+
+  // Returns the size of the window icon. This can be platform dependent
+  // because of differences in fonts, so its part of the interface.
+  virtual int GetIconSize() const = 0;
+
+  // Returns true if we should leave any offset at the frame caption. Typically
+  // when the frame is maximized/full screen we want to leave no offset at the
+  // top.
+  virtual bool ShouldLeaveOffsetNearTopBorder() const = 0;
+
+  // Returns the browser's minimum view size. Used because we need to calculate
+  // the minimum size for the entire non-client area.
+  virtual gfx::Size GetBrowserViewMinimumSize() const = 0;
+
+  // Controls the visualization of the avatar
+  virtual bool ShouldShowAvatar() const = 0;
+
+  // We don't have a ThemeProvider in the layout manager, so plumb in the icon
+  // source here.
+  virtual gfx::ImageSkia GetOTRAvatarIcon() const = 0;
+
+  // Controls window state.
+  virtual bool IsMaximized() const = 0;
+  virtual bool IsMinimized() const = 0;
+  virtual bool IsFullscreen() const = 0;
+
+  virtual bool IsTabStripVisible() const = 0;
+  virtual int GetTabStripHeight() const = 0;
+
+  // Various platforms need to be able to add more space to the
+  // tabstrip. Windows 8 metro mode uses this to account for the window
+  // switcher button.
+  virtual int GetAdditionalReservedSpaceInTabStrip() const = 0;
+
+  // Returns the tabstrips preferred size so the frame layout can work around
+  // it.
+  virtual gfx::Size GetTabstripPreferredSize() const = 0;
+
+ protected:
+  virtual ~OpaqueBrowserFrameViewLayoutDelegate() {}
+};
+
+#endif  // CHROME_BROWSER_UI_VIEWS_FRAME_OPAQUE_BROWSER_FRAME_VIEW_LAYOUT_DELEGATE_H_

chrome/chrome_browser_ui.gypi

@@ -1735,6 +1735,9 @@
         'browser/ui/views/frame/native_browser_frame.h',
         'browser/ui/views/frame/opaque_browser_frame_view.cc',
         'browser/ui/views/frame/opaque_browser_frame_view.h',
+        'browser/ui/views/frame/opaque_browser_frame_view_layout.cc',
+        'browser/ui/views/frame/opaque_browser_frame_view_layout.h',
+        'browser/ui/views/frame/opaque_browser_frame_view_layout_delegate.h',
         'browser/ui/views/frame/popup_non_client_frame_view.cc',
         'browser/ui/views/frame/popup_non_client_frame_view.h',
         'browser/ui/views/frame/scroll_end_effect_controller.h',

chrome/chrome_tests_unit.gypi

@@ -1638,6 +1638,7 @@
         'browser/ui/views/frame/browser_view_layout_unittest.cc',
         'browser/ui/views/frame/browser_view_unittest.cc',
         'browser/ui/views/frame/immersive_mode_controller_ash_unittest.cc',
+        'browser/ui/views/frame/opaque_browser_frame_view_layout_unittest.cc',
         'browser/ui/views/reload_button_unittest.cc',
         'browser/ui/views/select_file_dialog_extension_unittest.cc',
         'browser/ui/views/status_icons/status_tray_win_unittest.cc',

ui/views/controls/button/image_button.cc

@@ -16,6 +16,8 @@ namespace views {
 static const int kDefaultWidth = 16;   // Default button width if no theme.
 static const int kDefaultHeight = 14;  // Default button height if no theme.
 
+const char ImageButton::kViewClassName[] = "ImageButton";
+
 ////////////////////////////////////////////////////////////////////////////////
 // ImageButton, public:
 
@@ -84,6 +86,10 @@ gfx::Size ImageButton::GetPreferredSize() {
   return size;
 }
 
+const char* ImageButton::GetClassName() const {
+  return kViewClassName;
+}
+
 void ImageButton::OnPaint(gfx::Canvas* canvas) {
   // Call the base class first to paint any background/borders.
   View::OnPaint(canvas);

ui/views/controls/button/image_button.h

@@ -20,6 +20,8 @@ namespace views {
 
 class VIEWS_EXPORT ImageButton : public CustomButton {
  public:
+  static const char kViewClassName[];
+
   enum HorizontalAlignment {
     ALIGN_LEFT = 0,
     ALIGN_CENTER,
@@ -56,6 +58,7 @@ class VIEWS_EXPORT ImageButton : public CustomButton {
 
   // Overridden from View:
   virtual gfx::Size GetPreferredSize() OVERRIDE;
+  virtual const char* GetClassName() const OVERRIDE;
   virtual void OnPaint(gfx::Canvas* canvas) OVERRIDE;
 
   // Sets preferred size, so it could be correctly positioned in layout even if