Makes pinned tab service save pinned tabs if you close a normal

browser and there are only non-normal browsers open.

BUG=40293
TEST=see bug, also covered by unit test
R=ben@chromium.org,jcivelli@chromium.org

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

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

6 files changed, 162 insertions, 28 deletions

diff --git a/chrome/browser/tabs/pinned_tab_codec_unittest.cc b/chrome/browser/tabs/pinned_tab_codec_unittest.cc
index 7030b18..9fd33c1 100644
--- a/chrome/browser/tabs/pinned_tab_codec_unittest.cc
+++ b/chrome/browser/tabs/pinned_tab_codec_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -6,35 +6,15 @@
 #include <vector>
 
 #include "chrome/browser/tabs/pinned_tab_codec.h"
+#include "chrome/browser/tabs/pinned_tab_test_utils.h"
 #include "chrome/browser/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/test/browser_with_test_window_test.h"
 #include "chrome/test/testing_profile.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-typedef BrowserInit::LaunchWithProfile::Tab Tab;
-
 typedef BrowserWithTestWindowTest PinnedTabCodecTest;
 
-namespace {
-
-std::string TabToString(const Tab& tab) {
-  return tab.url.spec() + ":" + (tab.is_app ? "app" : "") + ":" +
-      (tab.is_pinned ? "pinned" : "") + ":" + tab.app_id;
-}
-
-std::string TabsToString(const std::vector<Tab>& values) {
-  std::string result;
-  for (size_t i = 0; i < values.size(); ++i) {
-    if (i != 0)
-      result += " ";
-    result += TabToString(values[i]);
-  }
-  return result;
-}
-
-}  // namespace
-
 // Make sure nothing is restored when the browser has no pinned tabs.
 TEST_F(PinnedTabCodecTest, NoPinnedTabs) {
   GURL url1("http://www.google.com");
@@ -42,7 +22,8 @@ TEST_F(PinnedTabCodecTest, NoPinnedTabs) {
 
   PinnedTabCodec::WritePinnedTabs(profile());
 
-  std::string result = TabsToString(PinnedTabCodec::ReadPinnedTabs(profile()));
+  std::string result = PinnedTabTestUtils::TabsToString(
+      PinnedTabCodec::ReadPinnedTabs(profile()));
   EXPECT_EQ("", result);
 }
 
@@ -60,6 +41,7 @@ TEST_F(PinnedTabCodecTest, PinnedAndNonPinned) {
 
   PinnedTabCodec::WritePinnedTabs(profile());
 
-  std::string result = TabsToString(PinnedTabCodec::ReadPinnedTabs(profile()));
+  std::string result = PinnedTabTestUtils::TabsToString(
+      PinnedTabCodec::ReadPinnedTabs(profile()));
   EXPECT_EQ("http://www.google.com/::pinned:", result);
 }
diff --git a/chrome/browser/tabs/pinned_tab_service.cc b/chrome/browser/tabs/pinned_tab_service.cc
index e1bef5d..98e4410 100644
--- a/chrome/browser/tabs/pinned_tab_service.cc
+++ b/chrome/browser/tabs/pinned_tab_service.cc
@@ -4,15 +4,30 @@
 
 #include "chrome/browser/tabs/pinned_tab_service.h"
 
+#include "chrome/browser/browser_list.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/tabs/pinned_tab_codec.h"
 #include "chrome/browser/ui/browser.h"
 #include "content/common/notification_service.h"
 #include "content/common/notification_type.h"
 
+static bool IsLastNormalBrowser(Browser* browser) {
+  for (BrowserList::const_iterator i = BrowserList::begin();
+       i != BrowserList::end(); ++i) {
+    if (*i != browser && (*i)->type() == Browser::TYPE_NORMAL &&
+        (*i)->profile() == browser->profile()) {
+      return false;
+    }
+  }
+  return true;
+}
+
 PinnedTabService::PinnedTabService(Profile* profile)
     : profile_(profile),
-      got_exiting_(false) {
+      got_exiting_(false),
+      has_normal_browser_(false) {
+  registrar_.Add(this, NotificationType::BROWSER_OPENED,
+                 NotificationService::AllSources());
   registrar_.Add(this, NotificationType::BROWSER_CLOSING,
                  NotificationService::AllSources());
   registrar_.Add(this, NotificationType::APP_EXITING,
@@ -26,15 +41,31 @@ void PinnedTabService::Observe(NotificationType type,
     return;
 
   switch (type.value) {
+    case NotificationType::BROWSER_OPENED: {
+      Browser* browser = Source<Browser>(source).ptr();
+      if (!has_normal_browser_ && browser->type() == Browser::TYPE_NORMAL &&
+          browser->profile() == profile_) {
+        has_normal_browser_ = true;
+      }
+      break;
+    }
+
     case NotificationType::BROWSER_CLOSING: {
       Browser* browser = Source<Browser>(source).ptr();
-      if (browser->profile() == profile_ && *(Details<bool>(details)).ptr())
-        GotExit();
+      if (has_normal_browser_ && browser->profile() == profile_) {
+        if (*(Details<bool>(details)).ptr()) {
+          GotExit();
+        } else if (IsLastNormalBrowser(browser)) {
+          has_normal_browser_ = false;
+          PinnedTabCodec::WritePinnedTabs(profile_);
+        }
+      }
       break;
     }
 
     case NotificationType::APP_EXITING: {
-      GotExit();
+      if (has_normal_browser_)
+        GotExit();
       break;
     }
 
diff --git a/chrome/browser/tabs/pinned_tab_service.h b/chrome/browser/tabs/pinned_tab_service.h
index 3c60814..8eff427 100644
--- a/chrome/browser/tabs/pinned_tab_service.h
+++ b/chrome/browser/tabs/pinned_tab_service.h
@@ -33,6 +33,9 @@ class PinnedTabService : public NotificationObserver {
   // triggers an exit) and can ignore all other events.
   bool got_exiting_;
 
+  // True if there is at least one normal browser for our profile.
+  bool has_normal_browser_;
+
   NotificationRegistrar registrar_;
 
   DISALLOW_COPY_AND_ASSIGN(PinnedTabService);
diff --git a/chrome/browser/tabs/pinned_tab_service_unittest.cc b/chrome/browser/tabs/pinned_tab_service_unittest.cc
new file mode 100644
index 0000000..9a12a18
--- /dev/null
+++ b/chrome/browser/tabs/pinned_tab_service_unittest.cc
@@ -0,0 +1,63 @@
+// Copyright (c) 2011 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 <string>
+#include <vector>
+
+#include "chrome/browser/tabs/pinned_tab_codec.h"
+#include "chrome/browser/tabs/pinned_tab_service.h"
+#include "chrome/browser/tabs/pinned_tab_test_utils.h"
+#include "chrome/browser/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/test/browser_with_test_window_test.h"
+#include "chrome/test/testing_profile.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+class PinnedTabServiceTest : public BrowserWithTestWindowTest {
+ public:
+  PinnedTabServiceTest() {}
+
+ protected:
+  virtual TestingProfile* CreateProfile() OVERRIDE {
+    TestingProfile* profile = BrowserWithTestWindowTest::CreateProfile();
+    pinned_tab_service_.reset(new PinnedTabService(profile));
+    return profile;
+  }
+
+ private:
+  scoped_ptr<PinnedTabService> pinned_tab_service_;
+
+  DISALLOW_COPY_AND_ASSIGN(PinnedTabServiceTest);
+};
+
+// Makes sure closing a popup triggers writing pinned tabs.
+TEST_F(PinnedTabServiceTest, Popup) {
+  GURL url("http://www.google.com");
+  AddTab(browser(), url);
+  browser()->tabstrip_model()->SetTabPinned(0, true);
+
+  // Create a popup.
+  scoped_ptr<Browser> popup(new Browser(Browser::TYPE_POPUP, profile()));
+  scoped_ptr<TestBrowserWindow> popup_window(
+      new TestBrowserWindow(popup.get()));
+  popup->set_window(popup_window.get());
+
+  // Close the browser. This should trigger saving the tabs.
+  browser()->OnWindowClosing();
+  DestroyBrowser();
+  std::string result = PinnedTabTestUtils::TabsToString(
+      PinnedTabCodec::ReadPinnedTabs(profile()));
+  EXPECT_EQ("http://www.google.com/::pinned:", result);
+
+  // Close the popup. This shouldn't reset the saved state.
+  popup->CloseAllTabs();
+  popup.reset(NULL);
+  popup_window.reset(NULL);
+
+  // Check the state to make sure it hasn't changed.
+  result = PinnedTabTestUtils::TabsToString(
+      PinnedTabCodec::ReadPinnedTabs(profile()));
+  EXPECT_EQ("http://www.google.com/::pinned:", result);
+}
+
diff --git a/chrome/browser/tabs/pinned_tab_test_utils.cc b/chrome/browser/tabs/pinned_tab_test_utils.cc
new file mode 100644
index 0000000..087df64
--- /dev/null
+++ b/chrome/browser/tabs/pinned_tab_test_utils.cc
@@ -0,0 +1,28 @@
+// Copyright (c) 2011 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 "chrome/browser/tabs/pinned_tab_test_utils.h"
+
+typedef BrowserInit::LaunchWithProfile::Tab Tab;
+
+namespace {
+
+std::string TabToString(const Tab& tab) {
+  return tab.url.spec() + ":" + (tab.is_app ? "app" : "") + ":" +
+      (tab.is_pinned ? "pinned" : "") + ":" + tab.app_id;
+}
+
+}  // namespace
+
+// static
+std::string PinnedTabTestUtils::TabsToString(
+    const std::vector<BrowserInit::LaunchWithProfile::Tab>& values) {
+  std::string result;
+  for (size_t i = 0; i < values.size(); ++i) {
+    if (i != 0)
+      result += " ";
+    result += TabToString(values[i]);
+  }
+  return result;
+}
diff --git a/chrome/browser/tabs/pinned_tab_test_utils.h b/chrome/browser/tabs/pinned_tab_test_utils.h
new file mode 100644
index 0000000..ab73e34
--- /dev/null
+++ b/chrome/browser/tabs/pinned_tab_test_utils.h
@@ -0,0 +1,27 @@
+// Copyright (c) 2011 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.
+
+#ifndef CHROME_BROWSER_TABS_PINNED_TAB_TEST_UTILS_H_
+#define CHROME_BROWSER_TABS_PINNED_TAB_TEST_UTILS_H_
+#pragma once
+
+#include <string>
+#include <vector>
+
+#include "base/basictypes.h"
+#include "chrome/browser/ui/browser_init.h"
+
+class PinnedTabTestUtils {
+ public:
+  // Converts a set of Tabs into a string. The format is a space separated list
+  // of urls. If the tab is an app, ':app' is appended, and if the tab is
+  // pinned, ':pinned' is appended.
+  static std::string TabsToString(
+      const std::vector<BrowserInit::LaunchWithProfile::Tab>& values);
+
+ private:
+  DISALLOW_IMPLICIT_CONSTRUCTORS(PinnedTabTestUtils);
+};
+
+#endif  // CHROME_BROWSER_TABS_PINNED_TAB_TEST_UTILS_H_