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

views/controls/single_split_view.cc

Index: views/controls/single_split_view.cc
===================================================================
--- views/controls/single_split_view.cc	(revision 68381)
+++ views/controls/single_split_view.cc	(working copy)
@@ -8,6 +8,7 @@
 #include <gdk/gdk.h>
 #endif
 
+#include "base/logging.h"
 #include "gfx/canvas.h"
 #include "skia/ext/skia_utils_win.h"
 #include "views/background.h"
@@ -23,10 +24,13 @@
 
 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)
@@ -58,8 +62,13 @@
 }
 
 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);
@@ -70,29 +79,37 @@
   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();
@@ -140,6 +157,14 @@
   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;
@@ -167,6 +192,7 @@
   if (new_size != divider_offset_) {
     set_divider_offset(new_size);
     Layout();
+    observer_->SplitHandleMoved(this);
   }
   return true;
 }
@@ -178,6 +204,7 @@
   if (canceled && drag_info_.initial_divider_offset != divider_offset_) {
     set_divider_offset(drag_info_.initial_divider_offset);
     Layout();
+    observer_->SplitHandleMoved(this);
   }
 }