Make session restore understand that tabs have multiple SessionStorageNamespaces.

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 caf5475a627844324d056f94c845f3d3cb57e56a..63fcf3ee8d33f260668bd1ccf4ccf1f21e9cc49a 100644
--- a/chrome/browser/sessions/session_service.cc
+++ b/chrome/browser/sessions/session_service.cc
@@ -603,15 +603,19 @@ void SessionService::Observe(int type,
             tab->extension_tab_helper()->extension_app()->id());
       }
 
-      // Record the association between the SessionStorageNamespace and the
-      // tab.
-      content::SessionStorageNamespace* session_storage_namespace =
-          tab->web_contents()->GetController().GetSessionStorageNamespaceMap()
-              .find("")->second;
-      ScheduleCommand(CreateSessionStorageAssociatedCommand(
-          tab->restore_tab_helper()->session_id(),
-          session_storage_namespace->persistent_id()));
-      session_storage_namespace->SetShouldPersist(true);
+      // Record the association between all the SessionStorageNamespaces and
+      // their containing tab.
+      const content::SessionStorageNamespaceMap& session_storage_namespace_map =
+          tab->web_contents()->GetController().GetSessionStorageNamespaceMap();
+      for (content::SessionStorageNamespaceMap::const_iterator it =
+               session_storage_namespace_map.begin();
+           it != session_storage_namespace_map.end();
+           ++it) {
+        ScheduleCommand(CreateSessionStorageAssociatedCommand(
+            tab->restore_tab_helper()->session_id(),
+            it->second->persistent_id(), it->first));
+        it->second->SetShouldPersist(true);
+      }
       break;
     }
 
@@ -619,12 +623,16 @@ 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 =
-          tab->web_contents()->GetController().GetSessionStorageNamespaceMap()
-              .find("")->second;
-      session_storage_namespace->SetShouldPersist(false);
+      // Allow the associated SessionStorageNamespaces to be deleted; they won't
+      // be needed in the session restore.
+      const content::SessionStorageNamespaceMap& session_storage_namespace_map =
+          tab->web_contents()->GetController().GetSessionStorageNamespaceMap();
+      for (content::SessionStorageNamespaceMap::const_iterator it =
+               session_storage_namespace_map.begin();
+           it != session_storage_namespace_map.end();
+           ++it) {
+        it->second->SetShouldPersist(false);
+      }
       TabClosed(tab->restore_tab_helper()->window_id(),
                 tab->restore_tab_helper()->session_id(),
                 tab->web_contents()->GetClosedByUserGesture());
@@ -859,10 +867,12 @@ SessionCommand* SessionService::CreatePinnedStateCommand(
 
 SessionCommand* SessionService::CreateSessionStorageAssociatedCommand(
     const SessionID& tab_id,
-    const std::string& session_storage_persistent_id) {
+    const std::string& session_storage_persistent_id,
+    const std::string& partition_id) {
   Pickle pickle;
   pickle.WriteInt(tab_id.id());
   pickle.WriteString(session_storage_persistent_id);
+  pickle.WriteString(partition_id);
   return new SessionCommand(kCommandSessionStorageAssociated, pickle);
 }
 
@@ -1266,13 +1276,19 @@ bool SessionService::CreateTabsAndWindows(
         scoped_ptr<Pickle> command_pickle(command->PayloadAsPickle());
         SessionID::id_type command_tab_id;
         std::string session_storage_persistent_id;
+        std::string partition_id;
         PickleIterator iter(*command_pickle.get());
+        // TODO(ajwong): Is it okay to just skip this command and proceed with
+        // the rest of session restore?
         if (!command_pickle->ReadInt(&iter, &command_tab_id) ||
-            !command_pickle->ReadString(&iter, &session_storage_persistent_id))
-          return true;
+            !command_pickle->ReadString(&iter,
+                                        &session_storage_persistent_id) ||
+            !command_pickle->ReadString(&iter, &partition_id))
+          continue;  // If the storage is corrupt, just ignore it.

[marja, 2012/08/06 14:38:34]

When an user first launches the version of chromium which has this code, it's
unable to read and utilize the old sessionStorageAssociated commands, because
they're missing the partition IDs. So, the sessionStorages end up getting lost
in that relaunch.

Can you / should you handle this gracefully (by associating the session storages
without a partition id into the default partition, or something like that)?

[marja, 2012/08/06 14:53:21]

Actually, depending on when this CL lands it might or might not matter. Atm it
doesn't matter, since the sessionStorage data is not yet saved. So maybe it's
simpler to just leave it like this. (Even if the sessionStorages get lost for
one restart, the impact is really small, taken into account that they're now
lost on every restart :)

[awong, 2012/08/07 00:38:48]

I think we're safe for now.  Like you said, we'd lose the SSNs for just one
restart.  

As to your other question about whether or not we could/should do something
about associating to the default partition, that would actually be unsafe.

Over a restart, someone may go in and mess with which URLs are isolated (imagine
a policy push).  Thus, it's not clear that the set of associated partition IDs
that were written are the same that should be in the tab on restore.

To handle the case where partitions become invalid, before we try to construct
the real TabContents, we check if the partition_id is valid. If not, then we
just elect not to insert the associated SSN into the tab contents.  They will
then auto-magically pull up the default SSN via the
NavControllerImpl::GetSessionStorageNamespace() method.

For the case where some NavEntries inside the tab have become "partitioned" we
actually don't need to do anything and they will mint themselves a fresh new SSN
on first access, again using NavControllerImpl::GetSessionStorageNamespace().

I think this all works.  The only thing we might want to do is when we notice a
partition is invalid is somehow mark that the persisted SSN can be cleaned up.

         // Associate the session storage back.
-        GetTab(command_tab_id, tabs)->session_storage_persistent_id =
-            session_storage_persistent_id;
+        GetTab(command_tab_id, tabs)->
+            session_storage_persistent_id_map[partition_id] =
+                session_storage_persistent_id;
         break;
       }
 
@@ -1346,13 +1362,18 @@ void SessionService::BuildCommandsForTab(
         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().GetSessionStorageNamespaceMap()
-          .find("")->second;
-  ScheduleCommand(CreateSessionStorageAssociatedCommand(
-      tab->restore_tab_helper()->session_id(),
-      session_storage_namespace->persistent_id()));
+  // Record the association between the SessionStorageNamespaces and the tab.
+  const content::SessionStorageNamespaceMap& session_storage_namespace_map =
+      tab->web_contents()->GetController().GetSessionStorageNamespaceMap();
+  for (content::SessionStorageNamespaceMap::const_iterator it =
+           session_storage_namespace_map.begin();
+       it != session_storage_namespace_map.end();
+       ++it) {
+    it->second->SetShouldPersist(true);
+    ScheduleCommand(CreateSessionStorageAssociatedCommand(
+        tab->restore_tab_helper()->session_id(),
+        it->second->persistent_id(), it->first));
+  }
 }
 
 void SessionService::BuildCommandsForBrowser(