Session restore: Store and restore persistent IDs for sessionStorage.

chrome/browser/sessions/session_service.cc

Index: chrome/browser/sessions/session_service.cc
diff --git a/chrome/browser/sessions/session_service.cc b/chrome/browser/sessions/session_service.cc
index b97b6a2800879417d78e3fbb9501b78c20e221ef..be3d700f0bea5893d3a11ffae706b60865202b78 100644
--- a/chrome/browser/sessions/session_service.cc
+++ b/chrome/browser/sessions/session_service.cc
@@ -37,6 +37,7 @@
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_service.h"
+#include "content/public/browser/session_storage_namespace.h"
 #include "content/public/browser/web_contents.h"
 
 #if defined(OS_MACOSX)
@@ -72,6 +73,7 @@ static const SessionCommand::id_type kCommandSetWindowAppName = 15;
 static const SessionCommand::id_type kCommandTabClosed = 16;
 static const SessionCommand::id_type kCommandWindowClosed = 17;
 static const SessionCommand::id_type kCommandSetTabUserAgentOverride = 18;
+static const SessionCommand::id_type kCommandSessionStorageAssociated = 19;
 
 // Every kWritesPerReset commands triggers recreating the file.
 static const int kWritesPerReset = 250;
@@ -599,6 +601,15 @@ void SessionService::Observe(int type,
             tab->restore_tab_helper()->session_id(),
             tab->extension_tab_helper()->extension_app()->id());
       }
+
+      // Record the association between the sessionStorage namespace and the

[sky, 2012/06/19 14:11:59]

Should that be SessionStorage?

[marja, 2012/06/20 16:54:32]

Done.

+      // tab.
+      content::SessionStorageNamespace* session_storage_namespace =
+          tab->web_contents()->GetController().GetSessionStorageNamespace();
+      ScheduleCommand(CreateSessionStorageAssociatedCommand(
+          tab->restore_tab_helper()->session_id(),
+          session_storage_namespace->persistent_id()));
+      session_storage_namespace->SetShouldPersist(true);
       break;
     }
 
@@ -606,6 +617,11 @@ void SessionService::Observe(int type,
       TabContents* tab = content::Source<TabContents>(source).ptr();
       if (!tab || tab->profile() != profile())
         return;
+      // Allow the associated sessionStorage to get deleted; it won't be needed
+      // in the session restore.
+      content::SessionStorageNamespace* session_storage_namespace =

[sky, 2012/06/19 14:11:59]

If we shutdown the session service before the tabcontents will the session
storage leak?

[michaeln, 2012/06/20 04:01:59]

The question about order at shutdown is interesting. During shutdown with the
'continue where i left off' setting turned on, are the
NOTIFICATION_TAB_CONTENTS_DESTROYED processed when tabs that will be restored
upon restart are cleaned up? How does the session service ignore state changes
it wants to ignore near the end of time? Does it skip saving SessionCommands
generatred near the end, or does it pull the plug on the notifications such that
no commands are generated near the end?

The call to SetShouldPersist(false) will take effect even if a
TabClosedSessionCommand created by TabClosed is not put in the session restore
journal.

Hopefully you two know more about the session restore stuff than i do :) I'm not
so familiar with this chrome/browser code.

[marja, 2012/06/20 16:54:32]

It's part of the normal operation that for some tabs, this code won't be
executed, and the "should persist" bool will be true. Those sessionStorages will
be persisted, and they will possibly be restored in the next startup.

When the data is saved, there will also be cleanup code to delete data which was
not used by session restore (e.g., if we crash and don't restore).

Iow, nothing should leak if session_storage_namespace->SetShouldPersist(false);
is not called for some tabs before exit. Was this what you meant?

[marja, 2012/06/20 16:54:32]

Afaics (empirically), when we're exiting the browser, this notification is not
sent for tabs that get closed because of the browser exit. So, those won't get
destroyed, the data will be persisted, and the session restore can work.

[michaeln, 2012/06/20 18:09:11]

Great, this works out well then.

+          tab->web_contents()->GetController().GetSessionStorageNamespace();
+      session_storage_namespace->SetShouldPersist(false);
       TabClosed(tab->restore_tab_helper()->window_id(),
                 tab->restore_tab_helper()->session_id(),
                 tab->web_contents()->GetClosedByUserGesture());
@@ -838,6 +854,15 @@ SessionCommand* SessionService::CreatePinnedStateCommand(
   return command;
 }
 
+SessionCommand* SessionService::CreateSessionStorageAssociatedCommand(
+    const SessionID& tab_id,
+    const std::string& session_storage_persistent_id) {
+  Pickle pickle;
+  pickle.WriteInt(tab_id.id());
+  pickle.WriteString(session_storage_persistent_id);

[sky, 2012/06/19 14:11:59]

Can session_storage_persistent_id be bigger than bytes available?

[marja, 2012/06/20 16:54:32]

Persistent ID is a guid (%08X-%04X-%04X-%04X-%012llX), so shouldn't be a problem
here.

[sky, 2012/06/20 21:57:45]

Please add a comment, and preferably a DCHECK.

+  return new SessionCommand(kCommandSessionStorageAssociated, pickle);
+}
+
 void SessionService::OnGotSessionCommands(
     Handle handle,
     scoped_refptr<InternalGetCommandsRequest> request) {
@@ -1234,6 +1259,19 @@ bool SessionService::CreateTabsAndWindows(
         break;
       }
 
+      case kCommandSessionStorageAssociated: {
+        scoped_ptr<Pickle> pickle(command->PayloadAsPickle());
+        SessionID::id_type tab_id;
+        std::string session_storage_persistent_id;
+        PickleIterator iter(*pickle.get());
+        pickle->ReadInt(&iter, &tab_id);

[sky, 2012/06/19 14:11:59]

Check return value of these two.

[marja, 2012/06/20 16:54:32]

Done.

+        pickle->ReadString(&iter, &session_storage_persistent_id);
+        // Associate the session storage back.
+        GetTab(tab_id, tabs)->session_storage_persistent_id =
+            session_storage_persistent_id;
+        break;
+      }
+
       default:
         VLOG(1) << "Failed reading an unknown command " << command->id();
         return true;
@@ -1303,6 +1341,13 @@ void SessionService::BuildCommandsForTab(
     commands->push_back(
         CreateSetTabIndexInWindowCommand(session_id, index_in_window));
   }
+
+  // Record the association between the sessionStorage namespace and the tab.
+  content::SessionStorageNamespace* session_storage_namespace =
+      tab->web_contents()->GetController().GetSessionStorageNamespace();
+  ScheduleCommand(CreateSessionStorageAssociatedCommand(
+      tab->restore_tab_helper()->session_id(),
+      session_storage_namespace->persistent_id()));
 }
 
 void SessionService::BuildCommandsForBrowser(