[linux_views][Win] spoof proof redesign infobar extension with tab.

chrome/browser/ui/views/infobars/infobar_container.cc

Index: chrome/browser/ui/views/infobars/infobar_container.cc
diff --git a/chrome/browser/ui/views/infobars/infobar_container.cc b/chrome/browser/ui/views/infobars/infobar_container.cc
index 20761aad61ac91716f461a952311662aa0e08faf..f005f87dd584a76cdb3898e622aceb8bddbf3cc3 100644
--- a/chrome/browser/ui/views/infobars/infobar_container.cc
+++ b/chrome/browser/ui/views/infobars/infobar_container.cc
@@ -33,6 +33,44 @@ InfoBarContainer::~InfoBarContainer() {
   ChangeTabContents(NULL);
 }
 
+void InfoBarContainer::LayoutHelper(bool set_bounds,

[Peter Kasting, 2011/03/07 20:14:30]

Nit: Function definition order should match declaration order.

[Sheridan Rawlins, 2011/03/08 01:38:19]

Done.

+                                    int* vertical_overlap,
+                                    int* height) const {
+  int minimum_top = 0;
+  int top = 0;
+
+  if (set_bounds) {

[Peter Kasting, 2011/03/07 22:23:07]

This recursive method works correctly, but it has two problems:

(1) When |set_bounds| is true, LayoutHelper() returns 0 in |vertical_overlap|. 
This doesn't matter to you now, but maybe someone will write code someday where
it does.
(2) It's confusing.  It took me a long time to understand why using recursion
here produced the right answer to the problem scenario you describe.

Therefore, I suggest you do the following:

(1) Make InfoBarView's functions for tab height and bar height both public.  For
the sake of argument let's call them GetTabHeight() and GetBarHeight().
(2) Convert this function to the following:

int InfoBarContainer::GetVerticalOverlap(int* height_without_overlap) const {
  int height_temp;
  if (!height_without_overlap)
    height_without_overlap = &height_temp;
  int vertical_overlap = 0;
  *height_without_overlap = 0;
  for (int i = 0; i < child_count(); ++i) {
    const InfoBarView* child =
        static_cast<const InfoBarView*>(GetChildViewAt(i));
    vertical_overlap = std::max(vertical_overlap,
        child->GetTabHeight() - *height_without_overlap);
    *height_without_overlap += child->GetBarHeight();
  }
  return vertical_overlap;
}

(3) Rewrite these three functions as follows:

int InfoBarContainer::VerticalOverlap() const {
  return GetVerticalOverlap(NULL);
}

gfx::Size InfoBarContainer::GetPreferredSize() {
  int height_without_overlap;
  int vertical_overlap = GetVerticalOverlap(&height_without_overlap);
  return gfx::Size(0, vertical_overlap + height_without_overlap);
}

void InfoBarContainer::Layout() {
  int next_child_bar_y = GetVerticalOverlap(NULL);
  for (int i = 0; i < child_count(); ++i) {
    InfoBarView* child = static_cast<InfoBarView*>(GetChildViewAt(i));
    child->SetBounds(0, next_child_bar_y - child->GetTabHeight(), width(),
                     child->GetPreferredSize().height());
    next_child_bar_y = child->GetMirroredBounds().bottom();
  }
}

I realize that to some degree this represents moving back towards the
GetHeights() code you had before.  I'm sorry that at first I didn't realize that
the net vertical overlap of the container might not be the vertical overlap of
the first child, which makes things much more complicated.

[Sheridan Rawlins, 2011/03/08 01:38:19]

Slightly different but in the same spirit.

Done.

+    // Because multiple infobars may be animating, the tab of a
+    // vertically lower InfoBarView may be "higher", and the first
+    // InfoBarView may start "lower" than 0.  Call once recursively to
+    // set the top to the overlap amount before laying out.
+    LayoutHelper(false, &top, height);
+  }
+
+  for (int i = 0; i < child_count(); ++i) {
+    View* child = const_cast<View*>(GetChildViewAt(i));
+    gfx::Size ps = child->GetPreferredSize();
+    int overlapped_top = top -
+        static_cast<InfoBarView*>(child)->vertical_overlap();
+    minimum_top = std::min(minimum_top, overlapped_top);
+    if (set_bounds)
+      child->SetBounds(0, overlapped_top, width(), ps.height());
+    top = overlapped_top + ps.height();
+  }
+
+  DCHECK(vertical_overlap);
+  DCHECK(height);
+  *vertical_overlap = -minimum_top;
+  *height = top - minimum_top;
+}
+
+int InfoBarContainer::vertical_overlap() const {
+  int vertical_overlap;
+  int height;
+  LayoutHelper(false, &vertical_overlap, &height);
+  return vertical_overlap;
+}
+
 void InfoBarContainer::ChangeTabContents(TabContents* contents) {
   registrar_.RemoveAll();
 
@@ -80,33 +118,21 @@ void InfoBarContainer::RemoveInfoBar(InfoBarView* infobar) {
   infobars_.erase(infobar);
 }
 
-void InfoBarContainer::PaintInfoBarArrows(gfx::Canvas* canvas,
-                                          View* outer_view,
-                                          int arrow_center_x) {
-  for (int i = 0; i < child_count(); ++i) {
-    InfoBarView* infobar = static_cast<InfoBarView*>(GetChildViewAt(i));
-    infobar->PaintArrow(canvas, outer_view, arrow_center_x);
-  }
-}
-
 gfx::Size InfoBarContainer::GetPreferredSize() {
-  // We do not have a preferred width (we will expand to fit the available width
-  // of the delegate). Our preferred height is the sum of the preferred heights
-  // of the InfoBars contained within us.
-  int height = 0;
-  for (int i = 0; i < child_count(); ++i)
-    height += GetChildViewAt(i)->GetPreferredSize().height();
+  // We do not have a preferred width (we will expand to fit the
+  // available width of the delegate). Our preferred height is the sum

[Peter Kasting, 2011/03/07 20:14:30]

Nit: Why did you rewrap these lines?  They were fine before.

[Sheridan Rawlins, 2011/03/08 01:38:19]

After adding text, I hit M-q.  Emacs' c-fill-paragraph apparently wraps more
aggressively than 80 chars.

http://www.emacswiki.org/emacs/FillParagraph

The right margin is determined by the variable fill-column. You can set it using
C-x f or M-x set-fill-column using an explicit argument. Use C-u followed by a
number to specify a column. Just C-u as argument means to use the current
column. The default value for fill-column is 70. This can be set to 80 in the
InitFile with the command

  (setq-default fill-column 80)

I'll fix this so it shouldn't happen again.

Done.

+  // of the preferred heights of the InfoBars contained within us and
+  // the tab_height.
+  int vertical_overlap;
+  int height;
+  LayoutHelper(false, &vertical_overlap, &height);
   return gfx::Size(0, height);
 }
 
 void InfoBarContainer::Layout() {
-  int top = 0;
-  for (int i = 0; i < child_count(); ++i) {
-    View* child = GetChildViewAt(i);
-    gfx::Size ps = child->GetPreferredSize();
-    child->SetBounds(0, top, width(), ps.height());
-    top += ps.height();
-  }
+  int vertical_overlap;
+  int height;
+  LayoutHelper(true, &vertical_overlap, &height);
 }
 
 AccessibilityTypes::Role InfoBarContainer::GetAccessibleRole() {