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

ios/shared/chrome/browser/tabs/web_state_list_serialisation.mm

Index: ios/shared/chrome/browser/tabs/web_state_list_serialisation.mm
diff --git a/ios/shared/chrome/browser/tabs/web_state_list_serialisation.mm b/ios/shared/chrome/browser/tabs/web_state_list_serialisation.mm
new file mode 100644
index 0000000000000000000000000000000000000000..ce68a1854d7c724e647e731a33b9cbcbb33ec349
--- /dev/null
+++ b/ios/shared/chrome/browser/tabs/web_state_list_serialisation.mm
@@ -0,0 +1,203 @@
+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import "ios/shared/chrome/browser/tabs/web_state_list_serialisation.h"
+
+#include <memory>
+#include <unordered_map>
+
+#include "base/logging.h"
+#import "base/mac/foundation_util.h"
+#import "ios/shared/chrome/browser/tabs/web_state_list.h"
+#import "ios/shared/chrome/browser/tabs/web_state_opener.h"
+#import "ios/web/public/serializable_user_data_manager.h"
+#import "ios/web/public/web_state/web_state.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+namespace {
+// Keys used to store information about the opener-opened relationship between
+// the WebStates stored in the WebStateList.
+NSString* const kOpenerIndexKey = @"OpenerIndex";
+NSString* const kOpenerNavigationIndexKey = @"OpenerNavigationIndex";
+
+// Legacy keys used to store information about the opener-opener relationship
+// before the M-60 release. Remove once M-70 has shipped.

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

I'd prefer a bug to track this rather than milestones that are pretty much
guaranteed to be wrong.

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

Done.

+NSString* const kObjectIDKey = @"TabID";
+NSString* const kOpenerIDKey = @"OpenerID";
+
+// Hash functor for using NSString* as a key of std::unordered_map<>.
+struct NSStringHash {

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

You're using ~15 lines of helpers here to enable using NSStrings as keys to an
unordered_map with webState pointers as values.

Why not invert the solution and use an NSMutableDictionary, storing the webState
pointers using [NSValue valueWithPointer:]? That seems less intrusive.

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

Done.

+  using argument_type = NSString*;
+  using result_type = size_t;
+
+  result_type operator()(NSString* value) const {
+    return std::hash<NSUInteger>()([value hash]);
+  }
+};
+
+// Comparison functor for using NSString* as a key of std::unordered_map<>.
+struct NSStringEqualTo : std::binary_function<NSString*, NSString*, bool> {
+  bool operator()(NSString* lhs, NSString* rhs) const {
+    return [lhs isEqualToString:rhs];
+  }
+};
+
+// Returns the value bound to |key| casted to type T. If the value is missing

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

casted -> cast. Yay, English!

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

Function was never used, removed.

+// or not of type T, then returns nil.
+template <typename T>
+T* GetValueForSerialisationKey(web::WebState* web_state, NSString* key) {

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

Is using the same name as a web::SerializableUserDataManager intentional? Is it
potentially confusing?

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

It was intentional. I've removed as it is not used.

+  return base::mac::ObjCCast<T>(
+      web::SerializableUserDataManager::FromWebState(web_state)
+          ->GetValueForSerializationKey(key));
+}
+
+// Restores the WebStates opener-opened relationship. This function includes
+// compatibility code to load sessions saved with M-59 or older; remove this

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

Same comment wrt milestones.

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

Done.

+// compatibility code once M-70 is released.
+void RestoreOpenerRelationShip(WebStateList* web_state_list, int old_count) {
+  // Detect if version M-59 or older created the serialised session state by
+  // looking for the presence of kOpenerIndexKey in WebState's data manager.
+  bool pre_m60_saved_session = false;

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

legacy_saved_session?

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

Ack.

+  for (int index = old_count; index < web_state_list->count(); ++index) {
+    web::WebState* web_state = web_state_list->GetWebStateAt(index);
+    web::SerializableUserDataManager* user_data_manager =
+        web::SerializableUserDataManager::FromWebState(web_state);

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

Doesn't your helper method on line 52 do this for you?

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

I've removed the method.

+
+    if (user_data_manager->GetValueForSerializationKey(kOpenerIndexKey))
+      continue;
+
+    pre_m60_saved_session = true;
+    break;
+  }
+
+  if (pre_m60_saved_session) {

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

This is a 90-line method, so maybe extract the legacy deserialization into its
own function.

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

Done.

+    std::unordered_map<NSString*, web::WebState*, NSStringHash, NSStringEqualTo>
+        id_to_web_state;
+
+    for (int index = old_count; index < web_state_list->count(); ++index) {
+      web::WebState* web_state = web_state_list->GetWebStateAt(index);
+      web::SerializableUserDataManager* user_data_manager =
+          web::SerializableUserDataManager::FromWebState(web_state);
+
+      NSString* object_id = base::mac::ObjCCast<NSString>(
+          user_data_manager->GetValueForSerializationKey(kObjectIDKey));
+
+      // The value may be missing during tests, skip object.

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

Is this conditional *only* to support tests? If so, we need better tests.

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

Removed the comment as this is also to support "corrupted" saved state.

+      if (!object_id || ![object_id length])
+        continue;
+
+      DCHECK(id_to_web_state.find(object_id) == id_to_web_state.end());
+      id_to_web_state.insert(std::make_pair(object_id, web_state));

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

Any reason why the (to me) more readable

 id_to_web_state[object_id] = web_state;

isn't preferred? (this is more of a C++ question than a review comment)

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

operator [] requires the value to be copyable, default constructible, and, if
used on a const map,  copy constructible. This mean that using
"id_to_web_state[object_id] = web_state;" would cause an object to be default
constructed (to return a reference to it) then the assignment operator would be
called. The map value were pointers, so this is okay (default construction of a
pointer set it to nullptr), but I just got used to using "insert" method instead
to avoid those unnecessary method calls.

I've changed the code to use NSMutableDictionary, so this is no longer relevant.

+    }
+
+    for (int index = old_count; index < web_state_list->count(); ++index) {
+      web::WebState* web_state = web_state_list->GetWebStateAt(index);
+      web::SerializableUserDataManager* user_data_manager =
+          web::SerializableUserDataManager::FromWebState(web_state);
+
+      NSString* opener_id = base::mac::ObjCCast<NSString>(
+          user_data_manager->GetValueForSerializationKey(kOpenerIDKey));
+
+      NSNumber* boxed_opener_navigation_index = base::mac::ObjCCast<NSNumber>(
+          user_data_manager->GetValueForSerializationKey(
+              kOpenerNavigationIndexKey));
+
+      // The opener ID or opener navigation index may be missing, assume no
+      // opener in that case.
+      if (!opener_id || !boxed_opener_navigation_index || ![opener_id length])
+        continue;

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

Also continue if the opener nav index < 0?

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

No, negative value is used during the tests.

+
+      // Assume no opener if the opener ID is not found in id_to_web_state
+      // (i.e. do not crash if saved state is partially broken).
+      auto iterator = id_to_web_state.find(opener_id);
+      if (iterator == id_to_web_state.end())
+        continue;
+
+      web_state_list->SetOpenerOfWebStateAt(
+          index, WebStateOpener(iterator->second,
+                                [boxed_opener_navigation_index intValue]));
+    }
+
+    return;
+  }
+
+  for (int index = old_count; index < web_state_list->count(); ++index) {
+    web::WebState* web_state = web_state_list->GetWebStateAt(index);
+    web::SerializableUserDataManager* user_data_manager =
+        web::SerializableUserDataManager::FromWebState(web_state);
+
+    NSNumber* boxed_opener_index = base::mac::ObjCCast<NSNumber>(
+        user_data_manager->GetValueForSerializationKey(kOpenerIndexKey));
+
+    NSNumber* boxed_opener_navigation_index = base::mac::ObjCCast<NSNumber>(
+        user_data_manager->GetValueForSerializationKey(
+            kOpenerNavigationIndexKey));
+
+    // The opener index and opener navigation index will be set to NSNull if
+    // there is no opener, thus the cast to NSNumber will fail. In that case,
+    // assumes there is no opener.
+    if (!boxed_opener_index || !boxed_opener_navigation_index)
+      continue;
+
+    // If opener index is out of bound then assume there is no opener.
+    int opener_index = [boxed_opener_index intValue] + old_count;
+    if (opener_index < old_count || opener_index >= web_state_list->count())
+      continue;
+
+    web::WebState* opener = web_state_list->GetWebStateAt(opener_index);
+    web_state_list->SetOpenerOfWebStateAt(
+        index, WebStateOpener(opener, [boxed_opener_index intValue]));
+  }
+}
+}  // namespace
+
+NSArray<CRWSessionStorage*>* SerializeWebStateList(
+    WebStateList* web_state_list) {
+  NSMutableArray<CRWSessionStorage*>* serialized_session =
+      [NSMutableArray arrayWithCapacity:web_state_list->count()];
+
+  for (int index = 0; index < web_state_list->count(); ++index) {
+    web::WebState* web_state = web_state_list->GetWebStateAt(index);
+    WebStateOpener opener = web_state_list->GetOpenerOfWebStateAt(index);
+
+    web::SerializableUserDataManager* user_data_manager =
+        web::SerializableUserDataManager::FromWebState(web_state);
+
+    int opener_index = WebStateList::kInvalidIndex;
+    if (opener.opener) {
+      opener_index = web_state_list->GetIndexOfWebState(opener.opener);
+      DCHECK_NE(opener_index, WebStateList::kInvalidIndex);
+      user_data_manager->AddSerializableData(@(opener_index), kOpenerIndexKey);
+      user_data_manager->AddSerializableData(@(opener.navigation_index),
+                                             kOpenerNavigationIndexKey);
+    } else {
+      user_data_manager->AddSerializableData([NSNull null], kOpenerIndexKey);
+      user_data_manager->AddSerializableData([NSNull null],
+                                             kOpenerNavigationIndexKey);
+    }
+
+    [serialized_session addObject:web_state->BuildSessionStorage()];
+  }
+
+  return serialized_session;

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

[serialized_session copy] since the return value is immutable.

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

Done.

+}
+
+int DeserializeWebStateListFrom(WebStateList* web_state_list,
+                                web::WebState::CreateParams params,
+                                NSArray<CRWSessionStorage*>* sessions) {
+  int old_count = web_state_list->count();
+  for (CRWSessionStorage* session in sessions) {
+    std::unique_ptr<web::WebState> web_state =
+        web::WebState::Create(params, session);
+
+    web_state_list->InsertWebState(web_state_list->count(),
+                                   std::move(web_state));
+  }
+
+  RestoreOpenerRelationShip(web_state_list, old_count);
+  return web_state_list->count() - old_count;
+}