Fixes bug where we weren't sending out TabClosingAt when removing a

tab as a result of uninstalling an extension.

BUG=none
TEST=none

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

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

3 files changed, 82 insertions, 14 deletions

diff --git a/chrome/browser/browser_browsertest.cc b/chrome/browser/browser_browsertest.cc
index cfd54bf..723e175 100644
--- a/chrome/browser/browser_browsertest.cc
+++ b/chrome/browser/browser_browsertest.cc
@@ -23,7 +23,6 @@
 #include "chrome/common/page_transition_types.h"
 #include "chrome/test/in_process_browser_test.h"
 #include "chrome/test/ui_test_utils.h"
-#include "testing/gtest/include/gtest/gtest.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 #include "net/base/mock_host_resolver.h"
@@ -63,6 +62,22 @@ int CountRenderProcessHosts() {
   return result;
 }
 
+class MockTabStripModelObserver : public TabStripModelObserver {
+ public:
+  MockTabStripModelObserver() : closing_count_(0) {}
+
+  virtual void TabClosingAt(TabContents* contents, int index) {
+    closing_count_++;
+  }
+
+  int closing_count() const { return closing_count_; }
+
+ private:
+  int closing_count_;
+
+  DISALLOW_COPY_AND_ASSIGN(MockTabStripModelObserver);
+};
+
 }  // namespace
 
 class BrowserTest : public ExtensionBrowserTest {
@@ -380,6 +395,43 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, RevivePhantomTab) {
   EXPECT_FALSE(model->IsPhantomTab(0));
 }
 
+// Makes sure TabClosing is sent when uninstalling an extension that is an app
+// tab.
+IN_PROC_BROWSER_TEST_F(BrowserTest, TabClosingWhenRemovingExtension) {
+  HTTPTestServer* server = StartHTTPServer();
+  ASSERT_TRUE(server);
+  host_resolver()->AddRule("www.example.com", "127.0.0.1");
+  GURL url(server->TestServerPage("empty.html"));
+  TabStripModel* model = browser()->tabstrip_model();
+
+  ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII("app/")));
+
+  Extension* app_extension = GetExtension();
+
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  TabContents* app_contents = new TabContents(browser()->profile(), NULL,
+                                              MSG_ROUTING_NONE, NULL);
+  app_contents->SetAppExtension(app_extension);
+
+  model->AddTabContents(app_contents, 0, false, 0, false);
+  model->SetTabPinned(0, true);
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  MockTabStripModelObserver observer;
+  model->AddObserver(&observer);
+
+  // Uninstall the extension and make sure TabClosing is sent.
+  ExtensionsService* service = browser()->profile()->GetExtensionsService();
+  service->UninstallExtension(GetExtension()->id(), false);
+  EXPECT_EQ(1, observer.closing_count());
+
+  model->RemoveObserver(&observer);
+
+  // There should only be one tab now.
+  ASSERT_EQ(1, browser()->tab_count());
+}
+
 IN_PROC_BROWSER_TEST_F(BrowserTest, AppTabRemovedWhenExtensionUninstalled) {
   PhantomTabTest();
 
diff --git a/chrome/browser/tabs/tab_strip_model.cc b/chrome/browser/tabs/tab_strip_model.cc
index 1d9f544..5a4368f 100644
--- a/chrome/browser/tabs/tab_strip_model.cc
+++ b/chrome/browser/tabs/tab_strip_model.cc
@@ -701,12 +701,14 @@ void TabStripModel::Observe(NotificationType type,
       Extension* extension = Details<Extension>(details).ptr();
       // Iterate backwards as we may remove items while iterating.
       for (int i = count() - 1; i >= 0; i--) {
-        if (GetTabContentsAt(i)->app_extension() == extension) {
+        TabContents* contents = GetTabContentsAt(i);
+        if (contents->app_extension() == extension) {
           // The extension an app tab was created from has been nuked. Delete
           // the TabContents. Deleting a TabContents results in a notification
           // of type TAB_CONTENTS_DESTROYED; we do the necessary cleanup in
           // handling that notification.
-          delete GetTabContentsAt(i);
+
+          InternalCloseTab(contents, i, false);
         }
       }
       break;
@@ -774,22 +776,28 @@ bool TabStripModel::InternalCloseTabs(std::vector<int> indices,
       continue;
     }
 
-    FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
-        TabClosingAt(detached_contents, indices[i]));
-
-    // Ask the delegate to save an entry for this tab in the historical tab
-    // database if applicable.
-    if (create_historical_tabs)
-      delegate_->CreateHistoricalTab(detached_contents);
-
-    // Deleting the TabContents will call back to us via NotificationObserver
-    // and detach it.
-    delete detached_contents;
+    InternalCloseTab(detached_contents, indices[i], create_historical_tabs);
   }
 
   return retval;
 }
 
+void TabStripModel::InternalCloseTab(TabContents* contents,
+                                     int index,
+                                     bool create_historical_tabs) {
+  FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
+                    TabClosingAt(contents, index));
+
+  // Ask the delegate to save an entry for this tab in the historical tab
+  // database if applicable.
+  if (create_historical_tabs)
+    delegate_->CreateHistoricalTab(contents);
+
+  // Deleting the TabContents will call back to us via NotificationObserver
+  // and detach it.
+  delete contents;
+}
+
 TabContents* TabStripModel::GetContentsAt(int index) const {
   CHECK(ContainsIndex(index)) <<
       "Failed to find: " << index << " in: " << count() << " entries.";
diff --git a/chrome/browser/tabs/tab_strip_model.h b/chrome/browser/tabs/tab_strip_model.h
index 67a5243..52bb3d6 100644
--- a/chrome/browser/tabs/tab_strip_model.h
+++ b/chrome/browser/tabs/tab_strip_model.h
@@ -602,6 +602,14 @@ class TabStripModel : public NotificationObserver {
   bool InternalCloseTabs(std::vector<int> indices,
                          bool create_historical_tabs);
 
+  // Invoked from InternalCloseTabs and when an extension is removed for an app
+  // tab. Notifies observers of TabClosingAt and deletes |contents|. If
+  // |create_historical_tabs| is true, CreateHistoricalTab is invoked on the
+  // delegate.
+  void InternalCloseTab(TabContents* contents,
+                        int index,
+                        bool create_historical_tabs);
+
   TabContents* GetContentsAt(int index) const;
 
   // The actual implementation of SelectTabContentsAt. Takes the previously