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 70475)
+++ views/controls/single_split_view.cc	(working copy)
@@ -23,10 +23,12 @@
 
 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) {
   AddChildView(leading);
   AddChildView(trailing);
 #if defined(OS_WIN)
@@ -45,6 +47,7 @@
     bool is_minimize_or_restore =
         previous.height() == 0 || current.height() == 0;
     if (!is_minimize_or_restore) {
+      divider_offset_ = NormalizeDividerOffset(divider_offset_);

[sky, 2011/01/07 21:35:40]

I think this'll do the wrong thing on a large contraction. For example, if the
width is 1000, divider at 800 and you resize to 400 you want the divider at 200,
but I believe this makes it go to 0.

[Aleksey Shlyapnikov, 2011/01/07 23:06:00]

Thanks for catching this, fixed.

       if (is_horizontal_)
         divider_offset_ += current.width() - previous.width();
       else
@@ -58,40 +61,17 @@
 }
 
 void SingleSplitView::Layout() {
-  if (GetChildViewCount() != 2)
+  // When provided, it's an observer's responsiblity to layout
+  // children views, SingleSplitView should not interfere in this case.
+  if (observer_)

[sky, 2011/01/07 21:35:40]

I'm confused by this. Why isn't layout always doing a layout? I thought what you
cared about was having the observer control where layout happens, but this
should still do a layout.

[Aleksey Shlyapnikov, 2011/01/07 23:06:00]

Correct, but it's a bit more complex than that. BrowserViewLayout first resizes
the split views, inquires its children future bounds, adjusts reserved rects and
only then resizes children views. Just calling Layout there (basically, resizing
the split view and letting it to resize its children) will require another const
function to be added to SingleSplitView,
CalculateChildrenBoundsForSplitViewBounds combining DidChangeBounds and
CalculateChildrenBounds functionality, call it for both split views first,
update contents reserved rects based on returned future bounds with a bit more
calculations than we do now since split views themselves are not moved yet and
only then resize split views.
My initial idea was to explicitly set all views bounds in BrowserViewLayout, let
me try the alternative approach to and see whether it's more natural.

[Aleksey Shlyapnikov, 2011/01/08 23:54:41]

Please give it another look, I uploaded the alternative implementation.

     return;
 
-  View* leading = GetChildViewAt(0);
-  View* trailing = GetChildViewAt(1);
+  gfx::Rect leading_bounds;
+  gfx::Rect trailing_bounds;
+  CalculateChildrenBounds(&leading_bounds, &trailing_bounds);
 
-  if (!leading->IsVisible() && !trailing->IsVisible())
-    return;
+  ResizeChildren(leading_bounds, trailing_bounds);
 
-  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());
-  } else if (!leading->IsVisible()) {
-    trailing->SetBounds(0, 0, width(), height());
-  } else {
-    if (divider_offset_ < 0)
-      divider_offset_ = (GetPrimaryAxisSize() - kDividerSize) / 2;
-    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);
-    }
-  }
-
   SchedulePaint();
 
   // Invoke super's implementation so that the children are layed out.
@@ -140,11 +120,61 @@
   return NULL;
 }
 
+void SingleSplitView::CalculateChildrenBounds(
+    gfx::Rect* leading_bounds, gfx::Rect* trailing_bounds) const {
+  bool is_leading_visible =
+      GetChildViewCount() > 0 && GetChildViewAt(0)->IsVisible();
+  bool is_trailing_visible =
+      GetChildViewCount() > 1 && GetChildViewAt(1)->IsVisible();
+
+  if (!is_leading_visible && !is_trailing_visible) {
+    *leading_bounds = gfx::Rect();

[sky, 2011/01/07 21:35:40]

nit: set the bounds to empty first thing. That way you don't care about someone
forgetting to reset them later on.

[Aleksey Shlyapnikov, 2011/01/07 23:06:00]

This is a very rare case, why should we penalize the normal code path by adding
unnecessary assignments?

+    *trailing_bounds = gfx::Rect();
+    return;
+  }
+
+  int divider_at;
+  int divider_size =
+      !is_leading_visible || !is_trailing_visible ? 0 : kDividerSize;
+
+  if (!is_trailing_visible) {
+    divider_at = GetPrimaryAxisSize();
+  } else if (!is_leading_visible) {
+    divider_at = 0;
+  } else {
+    divider_at = NormalizeDividerOffset(divider_offset_);
+  }
+
+  if (is_horizontal_) {
+    *leading_bounds = gfx::Rect(0, 0, divider_at, height());
+    *trailing_bounds =
+        gfx::Rect(divider_at + divider_size, 0,
+                  width() - divider_at - divider_size, height());
+  } else {
+    *leading_bounds = gfx::Rect(0, 0, width(), divider_at);
+    *trailing_bounds =
+        gfx::Rect(0, divider_at + divider_size,
+                  width(), height() - divider_at - divider_size);
+  }
+}
+
+void SingleSplitView::ResizeChildren(const gfx::Rect& leading_bounds,
+                                     const gfx::Rect& trailing_bounds) const {
+  if (GetChildViewCount() > 0) {
+    if (GetChildViewAt(0)->IsVisible())
+      GetChildViewAt(0)->SetBounds(leading_bounds);
+    if (GetChildViewCount() > 1) {
+      if (GetChildViewAt(1)->IsVisible())
+        GetChildViewAt(1)->SetBounds(trailing_bounds);
+    }
+  }
+}
+
 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_;
+  drag_info_.initial_divider_offset = NormalizeDividerOffset(divider_offset_);
   return true;
 }
 
@@ -166,7 +196,10 @@
 
   if (new_size != divider_offset_) {
     set_divider_offset(new_size);
-    Layout();
+    if (!observer_)
+      Layout();
+    else
+      observer_->SplitHandleMoved(this);
   }
   return true;
 }
@@ -177,7 +210,10 @@
 
   if (canceled && drag_info_.initial_divider_offset != divider_offset_) {
     set_divider_offset(drag_info_.initial_divider_offset);
-    Layout();
+    if (!observer_)
+      Layout();
+    else
+      observer_->SplitHandleMoved(this);
   }
 }
 
@@ -199,4 +235,11 @@
       divider_relative_offset < kDividerSize);
 }
 
+int SingleSplitView::NormalizeDividerOffset(int divider_offset) const {
+  if (divider_offset < 0)
+    return (GetPrimaryAxisSize() - kDividerSize) / 2;
+  return std::min(divider_offset,
+                  std::max(GetPrimaryAxisSize() - kDividerSize, 0));
+}
+
 }  // namespace views