Remove details from BROWSER_CLOSING and BROWSER_CLOSED notifications.

chrome/browser/ui/tabs/pinned_tab_service.cc

Index: chrome/browser/ui/tabs/pinned_tab_service.cc
diff --git a/chrome/browser/ui/tabs/pinned_tab_service.cc b/chrome/browser/ui/tabs/pinned_tab_service.cc
index 39de47d6202f8e5f73bd56300ac5b4be92d12a50..bae6e689d2c92c2550300034507233807d7f0c0b 100644
--- a/chrome/browser/ui/tabs/pinned_tab_service.cc
+++ b/chrome/browser/ui/tabs/pinned_tab_service.cc
@@ -11,7 +11,9 @@
 #include "chrome/common/chrome_notification_types.h"
 #include "content/public/browser/notification_service.h"
 
-static bool IsLastNormalBrowser(Browser* browser) {
+namespace {
+
+bool IsOnlyNormalBrowser(Browser* browser) {
   for (BrowserList::const_iterator i = BrowserList::begin();
        i != BrowserList::end(); ++i) {
     if (*i != browser && (*i)->is_type_tabbed() &&
@@ -22,9 +24,11 @@ static bool IsLastNormalBrowser(Browser* browser) {
   return true;
 }
 
+}  // namespace
+
 PinnedTabService::PinnedTabService(Profile* profile)
     : profile_(profile),
-      got_exiting_(false),
+      save_pinned_tabs_(true),
       has_normal_browser_(false) {
   registrar_.Add(this, chrome::NOTIFICATION_BROWSER_OPENED,
                  content::NotificationService::AllBrowserContextsAndSources());
@@ -37,9 +41,24 @@ PinnedTabService::PinnedTabService(Profile* profile)
 void PinnedTabService::Observe(int type,
                                const content::NotificationSource& source,
                                const content::NotificationDetails& details) {
-  if (got_exiting_)
-    return;
-
+  // Saving of tabs happens when saving is enabled, and when either the user
+  // exits the application or closes the last browser window.
+  // Saving is disabled when the user exits the application to prevent the
+  // pin state of all the open browsers being overwritten by the state of the
+  // last browser window to close.
+  // Saving is re-enabled when the first browser window is opened again. So,
+  // if background mode or a packaged app keeps the process alive after a user
+  // exit, when a new window is opened pinned tab saving is restored.
+  // Note, cancelling a shutdown (via onbeforeunload) will not re-enable pinned
+  // tab saving, to prevent this situation:
+  //   * two windows are open, one with pinned tabs
+  //   * user exits
+  //   * pinned tabs are saved
+  //   * window with pinned tabs is closed
+  //   * other window blocks close with onbeforeunload
+  //   * user saves work, etc. then closes the window
+  //   * pinned tabs are saved, without the window with the pinned tabs,
+  //     over-writing the correct state.

[sky, 2012/07/20 16:06:40]

The problem with this, and why I suggested NOTIFICATION_BROWSER_CLOSE_CANCELLED,
is that you're assuming the user exits immediately. We don't necessarily know
that is going to happen. A new browser window is a good indication, but I tend
to think a new tab created is better.

[benwells, 2012/07/23 01:22:12]

Yep, they won't always exit immediately. I've put in your suggestion and I
reckon it's better, especially in the case where there is only one window open.

   switch (type) {
     case chrome::NOTIFICATION_BROWSER_OPENED: {
       Browser* browser = content::Source<Browser>(source).ptr();
@@ -47,15 +66,16 @@ void PinnedTabService::Observe(int type,
           browser->profile() == profile_) {
         has_normal_browser_ = true;
       }
+      if (IsOnlyNormalBrowser(browser))
+        save_pinned_tabs_ = true;
       break;
     }
 
     case chrome::NOTIFICATION_BROWSER_CLOSING: {
       Browser* browser = content::Source<Browser>(source).ptr();
-      if (has_normal_browser_ && browser->profile() == profile_) {
-        if (*(content::Details<bool>(details)).ptr()) {
-          GotExit();
-        } else if (IsLastNormalBrowser(browser)) {
+      if (has_normal_browser_ && save_pinned_tabs_ &&
+          browser->profile() == profile_) {
+        if (IsOnlyNormalBrowser(browser)) {
           has_normal_browser_ = false;
           PinnedTabCodec::WritePinnedTabs(profile_);
         }
@@ -64,8 +84,10 @@ void PinnedTabService::Observe(int type,
     }
 
     case chrome::NOTIFICATION_CLOSE_ALL_BROWSERS_REQUEST: {
-      if (has_normal_browser_)
-        GotExit();
+      if (has_normal_browser_ && save_pinned_tabs_) {
+        PinnedTabCodec::WritePinnedTabs(profile_);
+        save_pinned_tabs_ = false;
+      }
       break;
     }
 
@@ -73,9 +95,3 @@ void PinnedTabService::Observe(int type,
       NOTREACHED();
   }
 }
-
-void PinnedTabService::GotExit() {
-  DCHECK(!got_exiting_);
-  got_exiting_ = true;
-  PinnedTabCodec::WritePinnedTabs(profile_);
-}