Streamline the layout of the BrowserView's children TabContents views....

views/controls/single_split_view.cc

 // Copyright (c) 2010 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 "views/controls/single_split_view.h"
 
 #if defined(OS_LINUX)
 #include <gdk/gdk.h>
 #endif
 
+#include "base/logging.h"
 #include "gfx/canvas.h"
 #include "skia/ext/skia_utils_win.h"
 #include "views/background.h"
 
 #if defined(OS_LINUX)
 #include "gfx/gtk_util.h"
 #endif
 
 namespace views {
 
 // Size of the divider in pixels.
 static const int kDividerSize = 4;
 
 SingleSplitView::SingleSplitView(View* leading,
                                  View* trailing,
-                                 Orientation orientation)
+                                 Orientation orientation,
+                                 Observer* observer)
     : is_horizontal_(orientation == HORIZONTAL_SPLIT),
       divider_offset_(-1),
-      resize_leading_on_bounds_change_(true) {
+      resize_leading_on_bounds_change_(true),
+      observer_(observer) {
+  DCHECK(observer_);
   AddChildView(leading);
   AddChildView(trailing);
 #if defined(OS_WIN)
   set_background(
       views::Background::CreateSolidBackground(
           skia::COLORREFToSkColor(GetSysColor(COLOR_3DFACE))));
 #endif
 }
 
 void SingleSplitView::DidChangeBounds(const gfx::Rect& previous,
@@ skipping 11 matching lines @@
         divider_offset_ += current.height() - previous.height();
 
       if (divider_offset_ < 0)
         divider_offset_ = kDividerSize;
     }
   }
   View::DidChangeBounds(previous, current);
 }
 
 void SingleSplitView::Layout() {
-  if (GetChildViewCount() != 2)
+  if (GetChildViewCount() != 2) {

[sky, 2011/01/06 17:55:02]

This code is fairly finicky. Please write some tests so that we have coverage.

[Aleksey Shlyapnikov, 2011/01/07 19:07:23]

Reworked this code to reduce finickiness.

On 2011/01/06 17:55:02, sky wrote:

+    // Special case, if there's only one view and it's visible, maximize it.
+    // Otherwise don't touch these rects to avoid unnecessary resizes.

[sky, 2011/01/06 17:55:02]

I don't understand this comment. Shouldn't you always reset the leading/trailing
rects here (and at 77) so that callers don't get stale data?

[Aleksey Shlyapnikov, 2011/01/07 19:07:23]

Done.

+    if (GetChildViewCount() == 1 && GetChildViewAt(0)->IsVisible())
+      leading_view_rect_ = gfx::Rect(0, 0, width(), height());
     return;
+  }
 
   View* leading = GetChildViewAt(0);
   View* trailing = GetChildViewAt(1);
 
   if (!leading->IsVisible() && !trailing->IsVisible())
     return;
 
   if (width() == 0 || height() == 0) {
     // We are most likely minimized - do not touch divider offset.
     return;
-  } else if (!trailing->IsVisible()) {
-    leading->SetBounds(0, 0, width(), height());
+  }
+
+  int divide_at;

[sky, 2011/01/06 17:55:02]

dividier_at

[Aleksey Shlyapnikov, 2011/01/07 19:07:23]

Done.

+  int divider_size =
+      !leading->IsVisible() || !trailing->IsVisible() ? 0 : kDividerSize;
+
+  if (!trailing->IsVisible()) {
+    divide_at = GetPrimaryAxisSize();
   } else if (!leading->IsVisible()) {
-    trailing->SetBounds(0, 0, width(), height());
+    divide_at = 0;
   } else {
-    if (divider_offset_ < 0)
+    if (divider_offset_ < 0) {
       divider_offset_ = (GetPrimaryAxisSize() - kDividerSize) / 2;
-    else
+    } else {
       divider_offset_ = std::min(divider_offset_,
                                  GetPrimaryAxisSize() - kDividerSize);
-
-    if (is_horizontal_) {
-      leading->SetBounds(0, 0, divider_offset_, height());
-      trailing->SetBounds(divider_offset_ + kDividerSize, 0,
-                          width() - divider_offset_ - kDividerSize, height());
-    } else {
-      leading->SetBounds(0, 0, width(), divider_offset_);
-      trailing->SetBounds(0, divider_offset_ + kDividerSize,
-                          width(), height() - divider_offset_ - kDividerSize);
     }
+    divide_at = divider_offset_;
   }
 
-  SchedulePaint();
+  if (is_horizontal_) {
+    leading_view_rect_ = gfx::Rect(0, 0, divide_at, height());
+    trailing_view_rect_ =
+        gfx::Rect(divide_at + divider_size, 0,
+                  width() - divide_at - divider_size, height());
+  } else {
+    leading_view_rect_ = gfx::Rect(0, 0, width(), divide_at);
+    trailing_view_rect_ =
+        gfx::Rect(0, divide_at + divider_size,
+                  width(), height() - divide_at - divider_size);
+  }
 
   // Invoke super's implementation so that the children are layed out.

[sky, 2011/01/06 17:55:02]

If Layout was invoked because the bounds of the singlesplitview changed, then at
this point the two views have not been correctly resized.

You should refactor the code so that you have a method that will tell you where
to layout things out, perhaps:

void CalculateChildrenBounds(gfx::Rect* leading_bounds, gfx::Rect*
trailing_bounds);

Layout would call this and set the bounds. BrowserViewLayout would also call
this method if it needs the bounds. When the splitter is moved conditionally
call Layout (BrowserView would return a value indicating no Layout).

[Aleksey Shlyapnikov, 2011/01/07 19:07:23]

Done.

   View::Layout();
 }
 
 AccessibilityTypes::Role SingleSplitView::GetAccessibleRole() {
   return AccessibilityTypes::ROLE_GROUPING;
 }
 
 gfx::Size SingleSplitView::GetPreferredSize() {
   int width = 0;
   int height = 0;
@@ skipping 25 matching lines @@
     return is_horizontal_ ? we_resize_cursor : ns_resize_cursor;
 #elif defined(OS_LINUX)
     return gfx::GetCursor(is_horizontal_ ?
                               GDK_SB_H_DOUBLE_ARROW :
                               GDK_SB_V_DOUBLE_ARROW);
 #endif
   }
   return NULL;
 }
 
+void SingleSplitView::ResizeViews() {
+  if (GetChildViewCount() > 0) {
+    GetChildViewAt(0)->SetBounds(leading_view_rect());

[sky, 2011/01/06 17:55:02]

Only do this if the views are visible.

[Aleksey Shlyapnikov, 2011/01/07 19:07:23]

Done.

+    if (GetChildViewCount() > 1)
+      GetChildViewAt(1)->SetBounds(trailing_view_rect());
+  }
+}
+
 bool SingleSplitView::OnMousePressed(const MouseEvent& event) {
   if (!IsPointInDivider(event.location()))
     return false;
   drag_info_.initial_mouse_offset = GetPrimaryAxisSize(event.x(), event.y());
   drag_info_.initial_divider_offset = divider_offset_;
   return true;
 }
 
 bool SingleSplitView::OnMouseDragged(const MouseEvent& event) {
   if (GetChildViewCount() < 2)
     return false;
 
   int delta_offset = GetPrimaryAxisSize(event.x(), event.y()) -
       drag_info_.initial_mouse_offset;
   if (is_horizontal_ && base::i18n::IsRTL())
     delta_offset *= -1;
   // Honor the minimum size when resizing.
   gfx::Size min = GetChildViewAt(0)->GetMinimumSize();
   int new_size = std::max(GetPrimaryAxisSize(min.width(), min.height()),
                           drag_info_.initial_divider_offset + delta_offset);
 
   // And don't let the view get bigger than our width.
   new_size = std::min(GetPrimaryAxisSize() - kDividerSize, new_size);
 
   if (new_size != divider_offset_) {
     set_divider_offset(new_size);
     Layout();
+    observer_->SplitHandleMoved(this);
   }
   return true;
 }
 
 void SingleSplitView::OnMouseReleased(const MouseEvent& event, bool canceled) {
   if (GetChildViewCount() < 2)
     return;
 
   if (canceled && drag_info_.initial_divider_offset != divider_offset_) {
     set_divider_offset(drag_info_.initial_divider_offset);
     Layout();
+    observer_->SplitHandleMoved(this);
   }
 }
 
 bool SingleSplitView::IsPointInDivider(const gfx::Point& p) {
   if (GetChildViewCount() < 2)
     return false;
 
   if (!GetChildViewAt(0)->IsVisible() || !GetChildViewAt(1)->IsVisible())
     return false;
 
   int divider_relative_offset;
   if (is_horizontal_) {
     divider_relative_offset =
         p.x() - GetChildViewAt(base::i18n::IsRTL() ? 1 : 0)->width();
   } else {
     divider_relative_offset = p.y() - GetChildViewAt(0)->height();
   }
   return (divider_relative_offset >= 0 &&
       divider_relative_offset < kDividerSize);
 }
 
 }  // namespace views