Fix bug where a shelf or infobar would not be closed correctly when closing a tab.

The issue was because ShelfVisibilityChangedImpl is called twice, once with a null current_tab for the closing tab (via TabClosingAt), and once on selection of the new tab (via TabSelectedAt) - the first call nulls the shelves without doing a Layout, the nulled shelves then don't show up in the second call, so Layout is never called.

http://crbug.com/2069

BUG=2069

Review URL: http://codereview.chromium.org/7155

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@3528 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 21 insertions, 16 deletions

diff --git a/chrome/browser/views/old_frames/vista_frame.cc b/chrome/browser/views/old_frames/vista_frame.cc
index ce8ff70..ed6cfe3 100644
--- a/chrome/browser/views/old_frames/vista_frame.cc
+++ b/chrome/browser/views/old_frames/vista_frame.cc
@@ -127,6 +127,7 @@ VistaFrame::VistaFrame(Browser* browser)
       shelf_view_(NULL),
       bookmark_bar_view_(NULL),
       info_bar_view_(NULL),
+      needs_layout_(false),
       is_off_the_record_(false),
       off_the_record_image_(NULL),
       distributor_logo_(NULL),
@@ -384,6 +385,7 @@ void VistaFrame::Layout() {
   browser_view_->LayoutStatusBubble(browser_y + browser_h);
 
   frame_view_->SchedulePaint();
+  needs_layout_ = false;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -1595,30 +1597,28 @@ void VistaFrame::DestroyBrowser() {
 
 void VistaFrame::ShelfVisibilityChangedImpl(TabContents* current_tab) {
   // Coalesce layouts.
-  bool changed = false;
-
   views::View* new_shelf = NULL;
   if (current_tab && current_tab->IsDownloadShelfVisible())
     new_shelf = current_tab->GetDownloadShelfView();
-  changed |= UpdateChildViewAndLayout(new_shelf, &shelf_view_);
+  needs_layout_ |= UpdateChildViewAndLayout(new_shelf, &shelf_view_);
 
   views::View* new_info_bar = NULL;
   WebContents* web_contents = current_tab ? current_tab->AsWebContents() : NULL;
   if (web_contents && web_contents->view()->IsInfoBarVisible())
     new_info_bar = web_contents->view()->GetInfoBarView();
-  changed |= UpdateChildViewAndLayout(new_info_bar, &info_bar_view_);
+  needs_layout_ |= UpdateChildViewAndLayout(new_info_bar, &info_bar_view_);
 
   views::View* new_bookmark_bar_view = NULL;
   if (SupportsBookmarkBar())
     new_bookmark_bar_view = GetBookmarkBarView();
-  changed |= UpdateChildViewAndLayout(new_bookmark_bar_view,
-                                      &active_bookmark_bar_);
+  needs_layout_ |= UpdateChildViewAndLayout(new_bookmark_bar_view,
+                                            &active_bookmark_bar_);
 
   // Only do a layout if the current contents is non-null. We assume that if the
   // contents is NULL, we're either being destroyed, or ShowTabContents is going
   // to be invoked with a non-null TabContents again so that there is no need
   // in doing a layout now (and would result in extra work/invalidation on
   // tab switches).
-  if (changed && current_tab)
+  if (needs_layout_ && current_tab)
     Layout();
 }
diff --git a/chrome/browser/views/old_frames/vista_frame.h b/chrome/browser/views/old_frames/vista_frame.h
index 3b105b5..10d654f 100644
--- a/chrome/browser/views/old_frames/vista_frame.h
+++ b/chrome/browser/views/old_frames/vista_frame.h
@@ -378,6 +378,9 @@ class VistaFrame : public BrowserWindow,
   // A mapping between accelerators and commands.
   scoped_ptr<std::map<views::Accelerator, int>> accelerator_table_;
 
+  // Whether this frame needs a layout or not.
+  bool needs_layout_;
+
   // Whether this frame represents an off the record session.
   bool is_off_the_record_;
 
diff --git a/chrome/browser/views/old_frames/xp_frame.cc b/chrome/browser/views/old_frames/xp_frame.cc
index 84d9aa77..9ca65f1 100644
--- a/chrome/browser/views/old_frames/xp_frame.cc
+++ b/chrome/browser/views/old_frames/xp_frame.cc
@@ -339,6 +339,7 @@ XPFrame::XPFrame(Browser* browser)
       is_active_(false),
       is_off_the_record_(false),
       title_bar_height_(0),
+      needs_layout_(false),
       off_the_record_image_(NULL),
       distributor_logo_(NULL),
       ignore_ncactivate_(false),
@@ -834,6 +835,7 @@ void XPFrame::Layout() {
   browser_view_->LayoutStatusBubble(last_y + browser_h);
 
   frame_view_->SchedulePaint();
+  needs_layout_ = false;
 }
 
 // This is called when we receive WM_ENDSESSION. We have 5 seconds to quit
@@ -2474,31 +2476,28 @@ void XPFrame::DestroyBrowser() {
 
 void XPFrame::ShelfVisibilityChangedImpl(TabContents* current_tab) {
   // Coalesce layouts.
-  bool changed = false;
-
   views::View* new_shelf = NULL;
   if (current_tab && current_tab->IsDownloadShelfVisible())
     new_shelf = current_tab->GetDownloadShelfView();
-  changed |= UpdateChildViewAndLayout(new_shelf, &shelf_view_);
+  needs_layout_ |= UpdateChildViewAndLayout(new_shelf, &shelf_view_);
 
   views::View* new_info_bar = NULL;
   WebContents* web_contents = current_tab ? current_tab->AsWebContents() : NULL;
   if (web_contents && web_contents->view()->IsInfoBarVisible())
     new_info_bar = web_contents->view()->GetInfoBarView();
-  changed |= UpdateChildViewAndLayout(new_info_bar, &info_bar_view_);
+  needs_layout_ |= UpdateChildViewAndLayout(new_info_bar, &info_bar_view_);
 
   views::View* new_bookmark_bar_view = NULL;
   if (SupportsBookmarkBar())
     new_bookmark_bar_view = GetBookmarkBarView();
-  changed |= UpdateChildViewAndLayout(new_bookmark_bar_view,
-                                      &active_bookmark_bar_);
+  needs_layout_ |= UpdateChildViewAndLayout(new_bookmark_bar_view,
+                                            &active_bookmark_bar_);
 
   // Only do a layout if the current contents is non-null. We assume that if the
   // contents is NULL, we're either being destroyed, or ShowTabContents is going
   // to be invoked with a non-null TabContents again so that there is no need
   // in doing a layout now (and would result in extra work/invalidation on
   // tab switches).
-  if (changed && current_tab)
+  if (needs_layout_ && current_tab)
     Layout();
-}
-
+}
\ No newline at end of file
diff --git a/chrome/browser/views/old_frames/xp_frame.h b/chrome/browser/views/old_frames/xp_frame.h
index 051ba83..49a2c9f 100644
--- a/chrome/browser/views/old_frames/xp_frame.h
+++ b/chrome/browser/views/old_frames/xp_frame.h
@@ -503,6 +503,9 @@ class XPFrame : public BrowserWindow,
   // Set during layout. Total height of the title bar.
   int title_bar_height_;
 
+  // Whether this frame needs a layout or not.
+  bool needs_layout_;
+
   static bool g_initialized;
   static HCURSOR g_resize_cursors[4];
   static SkBitmap** g_bitmaps;