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

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

Index: ios/shared/chrome/browser/tabs/web_state_list_serialisation.h
diff --git a/ios/shared/chrome/browser/tabs/web_state_list_serialisation.h b/ios/shared/chrome/browser/tabs/web_state_list_serialisation.h
new file mode 100644
index 0000000000000000000000000000000000000000..d03c99f5cb82a66324f11dad2c01ae048c4a32a9
--- /dev/null
+++ b/ios/shared/chrome/browser/tabs/web_state_list_serialisation.h
@@ -0,0 +1,27 @@
+// 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.
+
+#ifndef IOS_SHARED_CHROME_BROWSER_TABS_WEB_STATE_LIST_SERIALISATION_H_
+#define IOS_SHARED_CHROME_BROWSER_TABS_WEB_STATE_LIST_SERIALISATION_H_
+
+#import <Foundation/Foundation.h>
+
+#import "ios/web/public/web_state/web_state.h"

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

I'm sure you've explained this already, but assume I have no useful attention
span. Why can't these be WebStateList methods? I don't see any problematic
dependencies.

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

Two reasons:
1. on desktop, those are not methods of TabStripModel,
2. they can be implemented as separate functions.

Creating this as separate function also allow me to test them with test
implementation of WebStateList.

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

I'm not sure I find the first two reasons compelling, although the testing is
nice. It's OK for now, but I'll probably want to talk about general patterns for
free functions vs methods at some point.

+
+@class CRWSessionStorage;
+class WebStateList;
+
+// Returns an array of serialised sessions.
+// TODO(sdefresne): add unit tests.
+NSArray<CRWSessionStorage*>* SerializeWebStateList(
+    WebStateList* web_state_list);
+
+// Restores serialised sessions into the specified WebStateList. Returns the
+// number of WebStates restored.

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

Prefer comments that explicitly name parameters:

// Restores |sessions| into |web_state_list| ...

Please add a comment explaining what |param| does.

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

Done.

+// TODO(sdefresne): add unit tests.
+int DeserializeWebStateListFrom(WebStateList* web_state_list,

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

Is the return value just a convenience, or is it actually needed? That is, can a
caller assume that the return value should always equal the different between
|web_state_list|'s count() before and after calling this function?

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

Naming nit: If you're going to use a preposition at the end of the function name
(such as "From"), then the referent of that preposition should be the first
parameter. Right now this reads to me as "Deserialize a WebStateList from
|web_state_list_, also using |param| and |sessions|". 

So either drop the "From", or move |sessions| to be the first parameter.

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

Dropped "From".

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

For convenience. Removed.

+                                web::WebState::CreateParams params,
+                                NSArray<CRWSessionStorage*>* sessions);
+
+#endif  // IOS_SHARED_CHROME_BROWSER_TABS_WEB_STATE_LIST_SERIALISATION_H_