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..7eb5b1231ec84ed3c5cea84addb837da97b6a2ba 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 IsLastNormalBrowser(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,25 +41,31 @@ 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 if a browser window is opened again, which may happen
+  // if the close is cancelled from an onbeforeunload event and the user
+  // continues their session, or if the process is kept alive by the background

[benwells, 2012/07/19 11:16:28]

Is this logic too tricky? I'm not sure if it's the right way to handle
onbeforeunload cancelling a shutdown.

[sky, 2012/07/19 16:31:27]

Can you instead listen for NOTIFICATION_BROWSER_CLOSE_CANCELLED ?

[benwells, 2012/07/20 01:41:57]

I could, I'm not sure if it's right or not. I've updated the logic a bit and
expanded the comment. Let me know if this makes sense the way it is.

+  // mode or packaged apps.
   switch (type) {
     case chrome::NOTIFICATION_BROWSER_OPENED: {
       Browser* browser = content::Source<Browser>(source).ptr();
       if (!has_normal_browser_ && browser->is_type_tabbed() &&
           browser->profile() == profile_) {
         has_normal_browser_ = true;
+        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 (IsLastNormalBrowser(browser)) {
           has_normal_browser_ = false;
           PinnedTabCodec::WritePinnedTabs(profile_);
         }
@@ -64,8 +74,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 +85,3 @@ void PinnedTabService::Observe(int type,
       NOTREACHED();
   }
 }
-
-void PinnedTabService::GotExit() {
-  DCHECK(!got_exiting_);
-  got_exiting_ = true;
-  PinnedTabCodec::WritePinnedTabs(profile_);
-}