Relanding this, it was missing the Mac unit-test change, that was breaking the build.

TBR=ben

Closing the last tab with a download in-progress would cause the tab to be
closed and become unusable if the user decides not to proceed with the browser
shutdown.
This is because we check for in-progress downloads when the browser is closed,
and the tab is closed before that, leaving the tab-strip in a bad state.

This CL ensures we also bring-up the confirmation dialog when the last tab is
closed.

BUG=10680
TEST=Start downloading a big file. While the file is downloading, close all
tabs. When closing the last tab, the in-progress download dialog should be
shown. Select the 'Wait for downloads', the download tab should be shown and
the previous tab should still be displayed and functional.
Close all tabs again, this time select the 'Close and cancel downloads'
option, the browser should be closed.
Review URL: http://codereview.chromium.org/100210

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

7 files changed, 43 insertions, 8 deletions

diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc
index f931187..dc86bcb 100644
--- a/chrome/browser/browser.cc
+++ b/chrome/browser/browser.cc
@@ -1525,6 +1525,15 @@ bool Browser::RunUnloadListenerBeforeClosing(TabContents* contents) {
   return false;
 }
 
+bool Browser::CanCloseContentsAt(int index) {
+  if (tabstrip_model_.count() > 1)
+    return true;
+  // We are closing the last tab for this browser. Make sure to check for
+  // in-progress downloads.
+  // Note that the next call when it returns false will ask the user for
+  // confirmation before closing the browser if the user decides so.
+  return CanCloseWithInProgressDownloads();
+}
 
 ///////////////////////////////////////////////////////////////////////////////
 // Browser, TabStripModelObserver implementation:
diff --git a/chrome/browser/browser.h b/chrome/browser/browser.h
index b59d4ff..d89337c 100644
--- a/chrome/browser/browser.h
+++ b/chrome/browser/browser.h
@@ -432,6 +432,7 @@ class Browser : public TabStripModelDelegate,
   virtual void CloseFrameAfterDragSession();
   virtual void CreateHistoricalTab(TabContents* contents);
   virtual bool RunUnloadListenerBeforeClosing(TabContents* contents);
+  virtual bool CanCloseContentsAt(int index);
 
   // Overridden from TabStripModelObserver:
   virtual void TabInsertedAt(TabContents* contents,
diff --git a/chrome/browser/cocoa/tab_strip_controller_unittest.mm b/chrome/browser/cocoa/tab_strip_controller_unittest.mm
index 0a70a98d..f60ab03 100644
--- a/chrome/browser/cocoa/tab_strip_controller_unittest.mm
+++ b/chrome/browser/cocoa/tab_strip_controller_unittest.mm
@@ -49,6 +49,8 @@ class TestTabStripDelegate : public TabStripModelDelegate {
     return true;
   }
   virtual void RestoreTab() { }
+
+  virtual bool CanCloseContentsAt(int index) { return true; }
 };
 
 class TabStripControllerTest : public testing::Test {
diff --git a/chrome/browser/tabs/tab_strip_model.cc b/chrome/browser/tabs/tab_strip_model.cc
index 3815232..038f01c 100644
--- a/chrome/browser/tabs/tab_strip_model.cc
+++ b/chrome/browser/tabs/tab_strip_model.cc
@@ -515,6 +515,9 @@ bool TabStripModel::IsNewTabAtEndOfTabStrip(TabContents* contents) const {
 
 bool TabStripModel::InternalCloseTabContentsAt(int index,
                                                bool create_historical_tab) {
+  if (!delegate_->CanCloseContentsAt(index))
+    return false;
+
   TabContents* detached_contents = GetContentsAt(index);
 
   if (delegate_->RunUnloadListenerBeforeClosing(detached_contents))
diff --git a/chrome/browser/tabs/tab_strip_model.h b/chrome/browser/tabs/tab_strip_model.h
index abc8051..fac1dbe 100644
--- a/chrome/browser/tabs/tab_strip_model.h
+++ b/chrome/browser/tabs/tab_strip_model.h
@@ -109,7 +109,7 @@ class TabStripModelDelegate {
   virtual TabContents* AddBlankTab(bool foreground) = 0;
   virtual TabContents* AddBlankTabAt(int index, bool foreground) = 0;
 
-  // Ask for a new TabStripModel to be created and the given tab contents to
+  // Asks for a new TabStripModel to be created and the given tab contents to
   // be added to it. Its size and position are reflected in |window_bounds|.
   // If |dock_info|'s type is other than NONE, the newly created window should
   // be docked as identified by |dock_info|. Returns the Browser object
@@ -128,7 +128,7 @@ class TabStripModelDelegate {
     TAB_TEAROFF_ACTION = 2
   };
 
-  // Determine what drag actions are possible for the specified strip.
+  // Determines what drag actions are possible for the specified strip.
   virtual int GetDragActions() const = 0;
 
   // Creates an appropriate TabContents for the given URL. This is handled by
@@ -144,10 +144,10 @@ class TabStripModelDelegate {
       bool defer_load,
       SiteInstance* instance) const = 0;
 
-  // Return whether some contents can be duplicated.
+  // Returns whether some contents can be duplicated.
   virtual bool CanDuplicateContentsAt(int index) = 0;
 
-  // Duplicate the contents at the provided index and places it into its own
+  // Duplicates the contents at the provided index and places it into its own
   // window.
   virtual void DuplicateContentsAt(int index) = 0;
 
@@ -171,6 +171,9 @@ class TabStripModelDelegate {
 
   // Restores the last closed tab if CanRestoreTab would return true.
   virtual void RestoreTab() = 0;
+
+  // Returns whether some contents can be closed.
+  virtual bool CanCloseContentsAt(int index) = 0;
 };
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -254,8 +257,9 @@ class TabStripModel : public NotificationObserver {
   // Closes the TabContents at the specified index. This causes the TabContents
   // to be destroyed, but it may not happen immediately (e.g. if it's a
   // WebContents).
-  // Returns true if the TabContents was closed immediately, false if we are
-  // waiting for a response from an onunload handler.
+  // Returns true if the TabContents was closed immediately, false if it was not
+  // closed (we may be waiting for a response from an onunload handler, or
+  // waiting for the user to confirm closure).
   bool CloseTabContentsAt(int index) {
     return InternalCloseTabContentsAt(index, true);
   }
diff --git a/chrome/browser/tabs/tab_strip_model_unittest.cc b/chrome/browser/tabs/tab_strip_model_unittest.cc
index 8e052544..3036ddc 100644
--- a/chrome/browser/tabs/tab_strip_model_unittest.cc
+++ b/chrome/browser/tabs/tab_strip_model_unittest.cc
@@ -22,9 +22,11 @@
 class TabStripDummyDelegate : public TabStripModelDelegate {
  public:
   explicit TabStripDummyDelegate(TabContents* dummy)
-      : dummy_contents_(dummy) {}
+      : dummy_contents_(dummy), can_close_(true) {}
   virtual ~TabStripDummyDelegate() {}
 
+  void set_can_close(bool value) { can_close_ = value; }
+
   // Overridden from TabStripModelDelegate:
   virtual TabContents* AddBlankTab(bool foreground) { return NULL; }
   virtual TabContents* AddBlankTabAt(int index, bool foreground) {
@@ -56,12 +58,16 @@ class TabStripDummyDelegate : public TabStripModelDelegate {
   }
   virtual bool CanRestoreTab() { return false; }
   virtual void RestoreTab() {}
+  virtual bool CanCloseContentsAt(int index) { return can_close_ ; }
 
  private:
   // A dummy TabContents we give to callers that expect us to actually build a
   // Destinations tab for them.
   TabContents* dummy_contents_;
 
+  // Whether tabs can be closed.
+  bool can_close_;
+
   DISALLOW_EVIL_CONSTRUCTORS(TabStripDummyDelegate);
 };
 
@@ -311,7 +317,15 @@ TEST_F(TabStripModelTest, TestBasicAPI) {
 
   // Test CloseTabContentsAt
   {
-    tabstrip.CloseTabContentsAt(2);
+    // Let's test nothing happens when the delegate veto the close.
+    delegate.set_can_close(false);
+    EXPECT_FALSE(tabstrip.CloseTabContentsAt(2));
+    EXPECT_EQ(3, tabstrip.count());
+    EXPECT_EQ(0, observer.GetStateCount());
+
+    // Now let's close for real.
+    delegate.set_can_close(true);
+    EXPECT_TRUE(tabstrip.CloseTabContentsAt(2));
     EXPECT_EQ(2, tabstrip.count());
 
     EXPECT_EQ(3, observer.GetStateCount());
diff --git a/chrome/browser/views/tabs/tab_strip.cc b/chrome/browser/views/tabs/tab_strip.cc
index 51ff414..1b1acac 100644
--- a/chrome/browser/views/tabs/tab_strip.cc
+++ b/chrome/browser/views/tabs/tab_strip.cc
@@ -904,6 +904,8 @@ void TabStrip::CloseTab(Tab* tab) {
     available_width_for_tabs_ = GetAvailableWidthForTabs(last_tab);
     resize_layout_scheduled_ = true;
     AddMessageLoopObserver();
+    // Note that the next call might not close the tab (because of unload
+    // hanlders or if the delegate veto the close).
     model_->CloseTabContentsAt(tab_index);
   }
 }