[ios] Add free function to (de)serialise WebStateList.

ios/chrome/browser/tabs/tab_model.mm

Index: ios/chrome/browser/tabs/tab_model.mm
diff --git a/ios/chrome/browser/tabs/tab_model.mm b/ios/chrome/browser/tabs/tab_model.mm
index 5490d3e15f0b78383b9100eb8a61fca02772020e..f2ed27028a5c86b680b80038e4bad26faf43d067 100644
--- a/ios/chrome/browser/tabs/tab_model.mm
+++ b/ios/chrome/browser/tabs/tab_model.mm
@@ -47,6 +47,7 @@
 #import "ios/shared/chrome/browser/tabs/web_state_list_fast_enumeration_helper.h"
 #import "ios/shared/chrome/browser/tabs/web_state_list_metrics_observer.h"
 #import "ios/shared/chrome/browser/tabs/web_state_list_observer.h"
+#import "ios/shared/chrome/browser/tabs/web_state_list_serialisation.h"
 #import "ios/shared/chrome/browser/tabs/web_state_opener.h"
 #import "ios/web/navigation/crw_session_certificate_policy_manager.h"
 #import "ios/web/navigation/crw_session_controller.h"
@@ -78,14 +79,6 @@ NSString* const kTabModelOpenInBackgroundKey = @"shouldOpenInBackground";
 
 namespace {
 
-// The key under which the opener Tab ID is stored in the WebState's
-// serializable user data.
-NSString* const kOpenerIDKey = @"OpenerID";
-
-// The key under which the opener navigation index is stored in the WebState's
-// serializable user data.
-NSString* const kOpenerNavigationIndexKey = @"OpenerNavigationIndex";
-
 // Updates CRWSessionCertificatePolicyManager's certificate policy cache.
 void UpdateCertificatePolicyCacheFromWebState(
     const scoped_refptr<web::CertificatePolicyCache>& policy_cache,
@@ -131,34 +124,6 @@ void CleanCertificatePolicyCache(
                  base::Unretained(web_state_list)));
 }
 
-// Internal helper function returning the opener for a given Tab by
-// checking the associated Tab tabId (should be removed once the opener
-// is passed to the insertTab:atIndex: and replaceTab:withTab: methods).
-WebStateOpener GetOpenerForTab(id<NSFastEnumeration> tabs, Tab* tab) {
-  web::SerializableUserDataManager* user_data_manager =
-      web::SerializableUserDataManager::FromWebState(tab.webState);
-
-  NSString* opener_id = base::mac::ObjCCast<NSString>(
-      user_data_manager->GetValueForSerializationKey(kOpenerIDKey));
-  if (!opener_id || ![opener_id length])
-    return WebStateOpener(nullptr);
-
-  NSNumber* boxed_opener_navigation_index = base::mac::ObjCCast<NSNumber>(
-      user_data_manager->GetValueForSerializationKey(
-          kOpenerNavigationIndexKey));
-  if (!boxed_opener_navigation_index)
-    return WebStateOpener(nullptr);
-
-  for (Tab* current_tab in tabs) {
-    if ([opener_id isEqualToString:current_tab.tabId]) {
-      return WebStateOpener(current_tab.webState,
-                            [boxed_opener_navigation_index intValue]);
-    }
-  }
-
-  return WebStateOpener(nullptr);
-}
-
 }  // anonymous namespace
 
 @interface TabModelWebStateProxyFactory : NSObject<WebStateProxyFactory>
@@ -705,31 +670,8 @@ WebStateOpener GetOpenerForTab(id<NSFastEnumeration> tabs, Tab* tab) {
   // be done on a separate thread.
   // TODO(crbug.com/661986): This could get expensive especially since this
   // window may never be saved (if another call comes in before the delay).
-  NSMutableArray<CRWSessionStorage*>* sessions =
-      [NSMutableArray arrayWithCapacity:[self count]];
-
-  for (int index = 0; index < _webStateList->count(); ++index) {
-    web::WebState* webState = _webStateList->GetWebStateAt(index);
-    web::SerializableUserDataManager* userDataManager =
-        web::SerializableUserDataManager::FromWebState(webState);
-
-    WebStateOpener opener = _webStateList->GetOpenerOfWebStateAt(index);
-    if (opener.opener) {
-      Tab* parentTab = LegacyTabHelper::GetTabForWebState(opener.opener);
-      userDataManager->AddSerializableData(parentTab.tabId, kOpenerIDKey);
-      userDataManager->AddSerializableData(@(opener.navigation_index),
-                                           kOpenerNavigationIndexKey);
-    } else {
-      userDataManager->AddSerializableData([NSNull null], kOpenerIDKey);
-      userDataManager->AddSerializableData([NSNull null],
-                                           kOpenerNavigationIndexKey);
-    }
-
-    [sessions addObject:webState->BuildSessionStorage()];
-  }
-
   return [[[SessionWindowIOS alloc]
-      initWithSessions:sessions
+      initWithSessions:SerializeWebStateList(_webStateList.get())
          selectedIndex:[self indexOfTab:self.currentTab]] autorelease];
 }
 
@@ -758,24 +700,23 @@ WebStateOpener GetOpenerForTab(id<NSFastEnumeration> tabs, Tab* tab) {
   DCHECK_GE(oldCount, 0);
 
   web::WebState::CreateParams params(_browserState);
+  int restoredCount =
+      DeserializeWebStateListFrom(_webStateList.get(), params, sessions);
+
+  DCHECK_GE(restoredCount, 0);
+  DCHECK_EQ(sessions.count, static_cast<NSUInteger>(restoredCount));
+  DCHECK_GT(_webStateList->count(), oldCount);

[marq (ping after 24h), 2017/03/24 10:37:17]

restoredCount must be >= 0, so can be 0.
Presumably _webStateList->count() is expected to be oldCount + restoredCount? So
this could be 0, which isn't > oldCount?

(maybe line 706 should be GT and not GE?)

[sdefresne, 2017/03/27 16:14:44]

The method early return (see line 696) if sessions.count == 0, so the
restoredCount must always be > 0, changed the DCHECK_GE to DCHECK_GT instead.

[marq (ping after 24h), 2017/03/28 15:47:24]

Thanks -- a comment saying that because of line 696 we know its > 0 would also
have been fine.

+
   scoped_refptr<web::CertificatePolicyCache> policyCache =
       web::BrowserState::GetCertificatePolicyCache(_browserState);
 
   base::scoped_nsobject<NSMutableArray<Tab*>> restoredTabs(
-      [[NSMutableArray alloc] initWithCapacity:sessions.count]);
-
-  // Recreate all the restored Tabs and add them to the WebStateList without
-  // any opener-opened relationship (as the n-th restored Tab opener may be
-  // at an index larger than n). Then in a second pass fix the openers.
-  for (CRWSessionStorage* session in sessions) {
-    std::unique_ptr<web::WebState> webState =
-        web::WebState::Create(params, session);
-    _webStateList->InsertWebState(_webStateList->count(), std::move(webState));
-  }
+      [[NSMutableArray alloc] initWithCapacity:restoredCount]);
 
   for (int index = oldCount; index < _webStateList->count(); ++index) {
     web::WebState* webState = _webStateList->GetWebStateAt(index);
     Tab* tab = LegacyTabHelper::GetTabForWebState(webState);
+
     tab.webController.webUsageEnabled = webUsageEnabled_;
     tab.webController.usePlaceholderOverlay = YES;
 
@@ -788,17 +729,6 @@ WebStateOpener GetOpenerForTab(id<NSFastEnumeration> tabs, Tab* tab) {
   DCHECK_EQ(sessions.count, [restoredTabs count]);
   DCHECK_GT(_webStateList->count(), oldCount);
 
-  // Fix openers now that all Tabs have been restored. Only look for an opener
-  // Tab in the newly restored Tabs and not in the already open Tabs.
-  for (int index = oldCount; index < _webStateList->count(); ++index) {
-    DCHECK_GE(index, oldCount);
-    NSUInteger tabIndex = static_cast<NSUInteger>(index - oldCount);
-    Tab* tab = [restoredTabs objectAtIndex:tabIndex];
-    WebStateOpener opener = GetOpenerForTab(restoredTabs.get(), tab);
-    if (opener.opener)
-      _webStateList->SetOpenerOfWebStateAt(index, opener);
-  }
-
   // Update the selected tab if there was a selected Tab in the saved session.
   if (window.selectedIndex != NSNotFound) {
     NSUInteger selectedIndex = window.selectedIndex + oldCount;
@@ -821,7 +751,7 @@ WebStateOpener GetOpenerForTab(id<NSFastEnumeration> tabs, Tab* tab) {
     }
   }
   if (_tabUsageRecorder)
-    _tabUsageRecorder->InitialRestoredTabs(self.currentTab, restoredTabs);
+    _tabUsageRecorder->InitialRestoredTabs(self.currentTab, restoredTabs.get());
   return closedNTPTab;
 }