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 "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) {
   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,
                                       const gfx::Rect& current) {
   if (resize_leading_on_bounds_change_) {
     // We do not update divider_offset_ on minimize (to zero) and on restore
     // (to largest value). As a result we get back to the original value upon
     // window restore.
     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
         divider_offset_ += current.height() - previous.height();
 
       if (divider_offset_ < 0)
         divider_offset_ = kDividerSize;
     }
   }
   View::DidChangeBounds(previous, current);
 }
 
 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;
-
-  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);
-    }
-  }
+  ResizeChildren(leading_bounds, trailing_bounds);
 
   SchedulePaint();
 
   // Invoke super's implementation so that the children are layed out.
   View::Layout();
 }
 
 AccessibilityTypes::Role SingleSplitView::GetAccessibleRole() {
   return AccessibilityTypes::ROLE_GROUPING;
 }
@@ skipping 29 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::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;
 }
 
 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();
+    if (!observer_)
+      Layout();
+    else
+      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();
+    if (!observer_)
+      Layout();
+    else
+      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);
 }
 
+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