Pixel perfection on tabstrip for non-Aero frame. This gets both ends of the tabstrip positioned correctly for normal and incognito windows, in both restored and maximized modes.The chief change was to correctly transform the bounds returned by the non-client view into the BrowserView's coordinate system, as the Views system expected. To make this not break the world I then had to actually return the right coordinates in the non-client view :). Updated comments on function declarations to hopefully match reality.For convenience this also introduces a function to get the horizontal border size and cleans up existing code to use it.BUG=5054

Review URL: http://codereview.chromium.org/18019

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

5 files changed, 42 insertions, 36 deletions

diff --git a/chrome/browser/views/frame/aero_glass_non_client_view.h b/chrome/browser/views/frame/aero_glass_non_client_view.h
index e1a982e..0e4ff9b 100644
--- a/chrome/browser/views/frame/aero_glass_non_client_view.h
+++ b/chrome/browser/views/frame/aero_glass_non_client_view.h
@@ -18,6 +18,8 @@ class AeroGlassNonClientView : public views::NonClientView {
   AeroGlassNonClientView(AeroGlassFrame* frame, BrowserView* browser_view);
   virtual ~AeroGlassNonClientView();
 
+  // Retrieve the bounds for the specified |tabstrip|, in the coordinate system
+  // of the non-client view (which whould be window coordinates).
   gfx::Rect GetBoundsForTabStrip(TabStrip* tabstrip);
 
  protected:
diff --git a/chrome/browser/views/frame/browser_frame.h b/chrome/browser/views/frame/browser_frame.h
index c0b18af..28dd275 100644
--- a/chrome/browser/views/frame/browser_frame.h
+++ b/chrome/browser/views/frame/browser_frame.h
@@ -33,8 +33,7 @@ class BrowserFrame {
   // Sizes the frame assuming the contents view's bounds are as specified.
   virtual void SizeToContents(const gfx::Rect& contents_bounds) = 0;
 
-  // Returns the bounds that should be used to size and position the specified
-  // TabStrip. The coordinate system is that of the window's client area.
+  // Retrieve the bounds for the specified |tabstrip|, in window coordinates.
   virtual gfx::Rect GetBoundsForTabStrip(TabStrip* tabstrip) const = 0;
 
   // Updates the current frame of the Throbber animation, if applicable.
diff --git a/chrome/browser/views/frame/browser_view.cc b/chrome/browser/views/frame/browser_view.cc
index ab15b9c..9d2a387 100644
--- a/chrome/browser/views/frame/browser_view.cc
+++ b/chrome/browser/views/frame/browser_view.cc
@@ -769,15 +769,13 @@ bool BrowserView::GetSavedWindowBounds(gfx::Rect* bounds) const {
     }
 
     gfx::Rect window_rect = frame_->GetWindowBoundsForClientBounds(*bounds);
-    window_rect.set_origin(gfx::Point(bounds->x(), bounds->y()));
+    window_rect.set_origin(bounds->origin());
 
     // When we are given x/y coordinates of 0 on a created popup window,
     // assume none were given by the window.open() command.
     if (window_rect.x() == 0 && window_rect.y() == 0) {
-      gfx::Point origin = GetNormalBounds().origin();
-      origin.set_x(origin.x() + kWindowTilePixels);
-      origin.set_y(origin.y() + kWindowTilePixels);
-      window_rect.set_origin(origin);
+      window_rect.set_origin(GetNormalBounds().origin());
+      window_rect.Offset(kWindowTilePixels, kWindowTilePixels);
     }
 
     *bounds = window_rect;
@@ -1036,6 +1034,9 @@ views::DropTargetEvent* BrowserView::MapEventToTabStrip(
 int BrowserView::LayoutTabStrip() {
   if (IsTabStripVisible()) {
     gfx::Rect tabstrip_bounds = frame_->GetBoundsForTabStrip(tabstrip_);
+    gfx::Point tabstrip_origin = tabstrip_bounds.origin();
+    ConvertPointToView(GetParent(), this, &tabstrip_origin);
+    tabstrip_bounds.set_origin(tabstrip_origin);
     tabstrip_->SetBounds(tabstrip_bounds.x(), tabstrip_bounds.y(),
                          tabstrip_bounds.width(), tabstrip_bounds.height());
     return tabstrip_bounds.bottom();
diff --git a/chrome/browser/views/frame/opaque_non_client_view.cc b/chrome/browser/views/frame/opaque_non_client_view.cc
index c79fbfa..251f2a4 100644
--- a/chrome/browser/views/frame/opaque_non_client_view.cc
+++ b/chrome/browser/views/frame/opaque_non_client_view.cc
@@ -375,9 +375,15 @@ static const int kOTRBottomSpacing = 2;
 // The number of pixels to crop off the top of the OTR image when the window is
 // maximized.
 static const int kOTRZoomedTopCrop = 4;
+// Horizontal distance between the right edge of the OTR avatar icon and the
+// left edge of the tabstrip.
+static const int kOTRTabStripSpacing = 2;
+// Horizontal distance between the right edge of the new tab icon and the left
+// edge of the window minimize icon when the window is restored.
+static const int kNewTabIconMinimizeSpacing = 5;
 // Horizontal distance between the right edge of the new tab icon and the left
 // edge of the window minimize icon when the window is maximized.
-static const int kNewTabIconWindowControlsSpacing = 10;
+static const int kNewTabIconMinimizeZoomedSpacing = 16;
 
 ///////////////////////////////////////////////////////////////////////////////
 // OpaqueNonClientView, public:
@@ -481,8 +487,7 @@ OpaqueNonClientView::~OpaqueNonClientView() {
 gfx::Rect OpaqueNonClientView::GetWindowBoundsForClientBounds(
     const gfx::Rect& client_bounds) {
   int top_height = CalculateNonClientTopHeight();
-  int horizontal_border = frame_->IsMaximized() ?
-      kWindowHorizontalBorderZoomedSize : kWindowHorizontalBorderSize;
+  int horizontal_border = HorizontalBorderSize();
   int window_x = std::max(0, client_bounds.x() - horizontal_border);
   int window_y = std::max(0, client_bounds.y() - top_height);
   int window_w = client_bounds.width() + (2 * horizontal_border);
@@ -492,12 +497,14 @@ gfx::Rect OpaqueNonClientView::GetWindowBoundsForClientBounds(
 }
 
 gfx::Rect OpaqueNonClientView::GetBoundsForTabStrip(TabStrip* tabstrip) {
-  int tabstrip_x = otr_avatar_bounds_.right();
-  int tabstrip_width = minimize_button_->x() - tabstrip_x;
-  if (frame_->IsMaximized())
-    tabstrip_width -= kNewTabIconWindowControlsSpacing;
-  return gfx::Rect(tabstrip_x, 0, std::max(0, tabstrip_width),
-                   tabstrip->GetPreferredHeight());
+  int tabstrip_x = browser_view_->ShouldShowOffTheRecordAvatar() ?
+      (otr_avatar_bounds_.right() + kOTRTabStripSpacing) :
+      HorizontalBorderSize();
+  int tabstrip_width = minimize_button_->x() - tabstrip_x -
+      (frame_->IsMaximized() ?
+      kNewTabIconMinimizeZoomedSpacing : kNewTabIconMinimizeSpacing);
+  return gfx::Rect(tabstrip_x, CalculateNonClientTopHeight(),
+                   std::max(0, tabstrip_width), tabstrip->GetPreferredHeight());
 }
 
 void OpaqueNonClientView::UpdateWindowIcon() {
@@ -541,8 +548,7 @@ void OpaqueNonClientView::ButtonPressed(views::BaseButton* sender) {
 gfx::Rect OpaqueNonClientView::CalculateClientAreaBounds(int width,
                                                          int height) const {
   int top_margin = CalculateNonClientTopHeight();
-  int horizontal_border = frame_->IsMaximized() ?
-      kWindowHorizontalBorderZoomedSize : kWindowHorizontalBorderSize;
+  int horizontal_border = HorizontalBorderSize();
   return gfx::Rect(horizontal_border, top_margin,
       std::max(0, width - (2 * horizontal_border)),
       std::max(0, height - top_margin - kWindowVerticalBorderBottomSize));
@@ -551,9 +557,7 @@ gfx::Rect OpaqueNonClientView::CalculateClientAreaBounds(int width,
 gfx::Size OpaqueNonClientView::CalculateWindowSizeForClientSize(
     int width,
     int height) const {
-  int horizontal_border = frame_->IsMaximized() ?
-      kWindowHorizontalBorderZoomedSize : kWindowHorizontalBorderSize;
-  return gfx::Size(width + (2 * horizontal_border),
+  return gfx::Size(width + (2 * HorizontalBorderSize()),
       height + CalculateNonClientTopHeight() + kWindowVerticalBorderBottomSize);
 }
 
@@ -586,9 +590,7 @@ int OpaqueNonClientView::NonClientHitTest(const gfx::Point& point) {
       window_icon_->GetBounds(APPLY_MIRRORING_TRANSFORMATION).Contains(point))
     return HTSYSMENU;
 
-  int horizontal_border = frame_->IsMaximized() ?
-      kWindowHorizontalBorderZoomedSize : kWindowHorizontalBorderSize;
-  int window_component = GetHTComponentForFrame(point, horizontal_border,
+  int window_component = GetHTComponentForFrame(point, HorizontalBorderSize(),
       kResizeAreaCornerSize, kWindowVerticalBorderTopSize,
       frame_->window_delegate()->CanResize());
   // Fall back to the caption if no other component matches.
@@ -665,10 +667,8 @@ void OpaqueNonClientView::Layout() {
 }
 
 gfx::Size OpaqueNonClientView::GetPreferredSize() {
-  int horizontal_border = frame_->IsMaximized() ?
-      kWindowHorizontalBorderZoomedSize : kWindowHorizontalBorderSize;
   gfx::Size prefsize(frame_->client_view()->GetPreferredSize());
-  prefsize.Enlarge(2 * horizontal_border,
+  prefsize.Enlarge(2 * HorizontalBorderSize(),
       CalculateNonClientTopHeight() + kWindowVerticalBorderBottomSize);
   return prefsize;
 }
@@ -735,6 +735,11 @@ int OpaqueNonClientView::CalculateNonClientTopHeight() const {
   return frame_->IsMaximized() ? kNoTitleZoomedTopSpacing : kNoTitleTopSpacing;
 }
 
+int OpaqueNonClientView::HorizontalBorderSize() const {
+  return frame_->IsMaximized() ?
+      kWindowHorizontalBorderZoomedSize : kWindowHorizontalBorderSize;
+}
+
 void OpaqueNonClientView::PaintFrameBorder(ChromeCanvas* canvas) {
   SkBitmap* top_left_corner =
       resources()->GetPartBitmap(FRAME_TOP_LEFT_CORNER);
@@ -839,8 +844,8 @@ void OpaqueNonClientView::PaintToolbarBackground(ChromeCanvas* canvas) {
 void OpaqueNonClientView::PaintOTRAvatar(ChromeCanvas* canvas) {
   if (browser_view_->ShouldShowOffTheRecordAvatar()) {
     int src_y = frame_->IsMaximized() ? kOTRZoomedTopCrop : 0;
-    canvas->DrawBitmapInt(browser_view_->GetOTRAvatarIcon(),
-        0, src_y, otr_avatar_bounds_.width(), otr_avatar_bounds_.height(), 
+    canvas->DrawBitmapInt(browser_view_->GetOTRAvatarIcon(), 0, src_y,
+        otr_avatar_bounds_.width(), otr_avatar_bounds_.height(), 
         MirroredLeftPointForRect(otr_avatar_bounds_), otr_avatar_bounds_.y(),
         otr_avatar_bounds_.width(), otr_avatar_bounds_.height(), false);
   }
@@ -1005,11 +1010,6 @@ void OpaqueNonClientView::LayoutTitleBar() {
 }
 
 void OpaqueNonClientView::LayoutOTRAvatar() {
-  if (!browser_view_->ShouldShowOffTheRecordAvatar()) {
-    otr_avatar_bounds_.SetRect(0, 0, 0, 0);
-    return;
-  }
-
   SkBitmap otr_avatar_icon = browser_view_->GetOTRAvatarIcon();
   int non_client_height = CalculateNonClientTopHeight();
   int otr_bottom = non_client_height + browser_view_->GetTabStripHeight() -
diff --git a/chrome/browser/views/frame/opaque_non_client_view.h b/chrome/browser/views/frame/opaque_non_client_view.h
index e584d84..f6f3486 100644
--- a/chrome/browser/views/frame/opaque_non_client_view.h
+++ b/chrome/browser/views/frame/opaque_non_client_view.h
@@ -31,8 +31,8 @@ class OpaqueNonClientView : public views::NonClientView,
   // Retrieve the bounds of the window for the specified contents bounds.
   gfx::Rect GetWindowBoundsForClientBounds(const gfx::Rect& client_bounds);
 
-  // Retrieve the bounds (in ClientView coordinates) that the specified
-  // |tabstrip| will be laid out within.
+  // Retrieve the bounds for the specified |tabstrip|, in the coordinate system
+  // of the non-client view (which whould be window coordinates).
   gfx::Rect GetBoundsForTabStrip(TabStrip* tabstrip);
 
   // Updates the window icon/throbber.
@@ -74,6 +74,10 @@ class OpaqueNonClientView : public views::NonClientView,
   // title bar, etc).
   int CalculateNonClientTopHeight() const;
 
+  // Returns the current thickness of the horizontal border that makes up the
+  // window edge.
+  int HorizontalBorderSize() const;
+
   // Paint various sub-components of this view.
   void PaintFrameBorder(ChromeCanvas* canvas);
   void PaintMaximizedFrameBorder(ChromeCanvas* canvas);