Created SerializedUserDataManager.

Created SerializedUserDataManager.

This class allows for clients of web/ to associate arbitrary
data with a browsing session that will be persisted with the
NavigationManager.

BUG=454984

[kkhorimoto, 2015-09-24 00:55:48]

kkhorimoto@chromium.org changed reviewers:
+ eugenebut@chromium.org, stuartmorgan@chromium.org

[kkhorimoto, 2015-09-24 00:55:48]

https://codereview.chromium.org/1361173005/diff/1/ios/web/public/browser_url_...
File ios/web/public/browser_url_rewriter.h (right):

https://codereview.chromium.org/1361173005/diff/1/ios/web/public/browser_url_...
ios/web/public/browser_url_rewriter.h:8: #include <vector>
Not sure why, but my changes in this CL made it so that this header couldn't
find vector, so I've included it here.

[Eugene But (OOO till 7-30), 2015-09-24 17:05:07]

https://codereview.chromium.org/1361173005/diff/1/ios/web/navigation/crw_seri...
File ios/web/navigation/crw_serialized_navigation_manager.h (right):

https://codereview.chromium.org/1361173005/diff/1/ios/web/navigation/crw_seri...
ios/web/navigation/crw_serialized_navigation_manager.h:37: // Dictionary of
arbitrary serialized data added using
NIT: maybe "A dictionary"?

https://codereview.chromium.org/1361173005/diff/1/ios/web/navigation/serializ...
File ios/web/navigation/serialized_user_data_manager_impl.h (right):

https://codereview.chromium.org/1361173005/diff/1/ios/web/navigation/serializ...
ios/web/navigation/serialized_user_data_manager_impl.h:30:
base::scoped_nsobject<NSMutableDictionary> dictionary_;
s/dictionary_/user_data_ ?

https://codereview.chromium.org/1361173005/diff/1/ios/web/navigation/serializ...
ios/web/navigation/serialized_user_data_manager_impl.h:32: }
 // namespace web

https://codereview.chromium.org/1361173005/diff/1/ios/web/navigation/serializ...
File ios/web/navigation/serialized_user_data_manager_impl.mm (right):

https://codereview.chromium.org/1361173005/diff/1/ios/web/navigation/serializ...
ios/web/navigation/serialized_user_data_manager_impl.mm:81: return [dictionary_
objectForKey:serialization_key];
DCHECK(serialization_key.length)

https://codereview.chromium.org/1361173005/diff/1/ios/web/navigation/serializ...
ios/web/navigation/serialized_user_data_manager_impl.mm:86: return dictionary_;
I would return an autoreleased copy of this dict to avoid issues in the future.
However It's up to you.

https://codereview.chromium.org/1361173005/diff/1/ios/web/navigation/serializ...
ios/web/navigation/serialized_user_data_manager_impl.mm:91: [dictionary_
addEntriesFromDictionary:dictionary];
DCHECK(dictionary)

https://codereview.chromium.org/1361173005/diff/1/ios/web/public/navigation_m...
File ios/web/public/navigation_manager.h (right):

https://codereview.chromium.org/1361173005/diff/1/ios/web/public/navigation_m...
ios/web/public/navigation_manager.h:22: class NavigationManager : public
base::SupportsUserData {
Is there any specific reason why NavigationManager is a base::SupportsUserData
subclass? Can you make NavigationManagerImpl support user data?

https://codereview.chromium.org/1361173005/diff/1/ios/web/public/serialized_u...
File ios/web/public/serialized_user_data_manager.h (right):

https://codereview.chromium.org/1361173005/diff/1/ios/web/public/serialized_u...
ios/web/public/serialized_user_data_manager.h:37: // |data| is retained by the
manager, and |serialization_key| is copied.
|serialization_key| can not be empty?

https://codereview.chromium.org/1361173005/diff/1/ios/web/public/serialized_u...
ios/web/public/serialized_user_data_manager.h:43: NSString* serialization_key)
const = 0;
ditto

https://codereview.chromium.org/1361173005/diff/1/ios/web/public/serialized_u...
ios/web/public/serialized_user_data_manager.h:48: // Adds values and
serialization keys from |dictionary| to the manager.
|dictionary| can not be null?

[kkhorimoto, 2015-09-24 21:24:29]

https://codereview.chromium.org/1361173005/diff/1/ios/web/navigation/crw_seri...
File ios/web/navigation/crw_serialized_navigation_manager.h (right):

https://codereview.chromium.org/1361173005/diff/1/ios/web/navigation/crw_seri...
ios/web/navigation/crw_serialized_navigation_manager.h:37: // Dictionary of
arbitrary serialized data added using
On 2015/09/24 17:05:06, eugenebut wrote:
> NIT: maybe "A dictionary"?

Done.

https://codereview.chromium.org/1361173005/diff/1/ios/web/navigation/serializ...
File ios/web/navigation/serialized_user_data_manager_impl.h (right):

https://codereview.chromium.org/1361173005/diff/1/ios/web/navigation/serializ...
ios/web/navigation/serialized_user_data_manager_impl.h:30:
base::scoped_nsobject<NSMutableDictionary> dictionary_;
On 2015/09/24 17:05:07, eugenebut wrote:
> s/dictionary_/user_data_ ?

Done.

https://codereview.chromium.org/1361173005/diff/1/ios/web/navigation/serializ...
ios/web/navigation/serialized_user_data_manager_impl.h:32: }
On 2015/09/24 17:05:07, eugenebut wrote:
>  // namespace web

Done.

https://codereview.chromium.org/1361173005/diff/1/ios/web/navigation/serializ...
File ios/web/navigation/serialized_user_data_manager_impl.mm (right):

https://codereview.chromium.org/1361173005/diff/1/ios/web/navigation/serializ...
ios/web/navigation/serialized_user_data_manager_impl.mm:81: return [dictionary_
objectForKey:serialization_key];
On 2015/09/24 17:05:07, eugenebut wrote:
> DCHECK(serialization_key.length)

Done.

https://codereview.chromium.org/1361173005/diff/1/ios/web/navigation/serializ...
ios/web/navigation/serialized_user_data_manager_impl.mm:86: return dictionary_;
On 2015/09/24 17:05:07, eugenebut wrote:
> I would return an autoreleased copy of this dict to avoid issues in the
future.
> However It's up to you.

Done.

https://codereview.chromium.org/1361173005/diff/1/ios/web/navigation/serializ...
ios/web/navigation/serialized_user_data_manager_impl.mm:91: [dictionary_
addEntriesFromDictionary:dictionary];
On 2015/09/24 17:05:07, eugenebut wrote:
> DCHECK(dictionary)

Done.

https://codereview.chromium.org/1361173005/diff/1/ios/web/public/navigation_m...
File ios/web/public/navigation_manager.h (right):

https://codereview.chromium.org/1361173005/diff/1/ios/web/public/navigation_m...
ios/web/public/navigation_manager.h:22: class NavigationManager : public
base::SupportsUserData {
On 2015/09/24 17:05:07, eugenebut wrote:
> Is there any specific reason why NavigationManager is a base::SupportsUserData
> subclass? Can you make NavigationManagerImpl support user data?

I initially did this because of the style guide's rules on multiple inheritance
in C++ (SupportsUserData is not a pure interface).  However, I've updated the
implementation to simply use the web state's user data.

https://codereview.chromium.org/1361173005/diff/1/ios/web/public/serialized_u...
File ios/web/public/serialized_user_data_manager.h (right):

https://codereview.chromium.org/1361173005/diff/1/ios/web/public/serialized_u...
ios/web/public/serialized_user_data_manager.h:37: // |data| is retained by the
manager, and |serialization_key| is copied.
On 2015/09/24 17:05:07, eugenebut wrote:
> |serialization_key| can not be empty?

Done.

https://codereview.chromium.org/1361173005/diff/1/ios/web/public/serialized_u...
ios/web/public/serialized_user_data_manager.h:43: NSString* serialization_key)
const = 0;
On 2015/09/24 17:05:07, eugenebut wrote:
> ditto

Done.

https://codereview.chromium.org/1361173005/diff/1/ios/web/public/serialized_u...
ios/web/public/serialized_user_data_manager.h:48: // Adds values and
serialization keys from |dictionary| to the manager.
On 2015/09/24 17:05:07, eugenebut wrote:
> |dictionary| can not be null?

Done.

[Eugene But (OOO till 7-30), 2015-09-24 21:39:25]

https://codereview.chromium.org/1361173005/diff/20001/ios/web/navigation/crw_...
File ios/web/navigation/crw_serialized_navigation_manager.mm (right):

https://codereview.chromium.org/1361173005/diff/20001/ios/web/navigation/crw_...
ios/web/navigation/crw_serialized_navigation_manager.mm:121: return
_serializedUserData;
NIT: Maybe return an autoreleased copy here (same as in
GetSerializationDictionary)?

https://codereview.chromium.org/1361173005/diff/20001/ios/web/public/serializ...
File ios/web/public/serialized_user_data_manager.h (right):

https://codereview.chromium.org/1361173005/diff/20001/ios/web/public/serializ...
ios/web/public/serialized_user_data_manager.h:55: }
NIT: // namespace web

https://codereview.chromium.org/1361173005/diff/20001/ios/web/web_state/web_s...
File ios/web/web_state/web_state_impl.mm (right):

https://codereview.chromium.org/1361173005/diff/20001/ios/web/web_state/web_s...
ios/web/web_state/web_state_impl.mm:115: WebStateImpl* non_const_this =
const_cast<WebStateImpl*>(this);
This is a violation of API contract. This method promises to be const, but it
changes WebState.

[kkhorimoto, 2017-03-15 17:33:25]

Description was changed from

==========
Created SerializedUserDataManager.

This class allows for clients of web/ to associate arbitrary
data with a browsing session that will be persisted with the
NavigationManager.

BUG=454984
==========

to

==========
Created SerializedUserDataManager.

This class allows for clients of web/ to associate arbitrary
data with a browsing session that will be persisted with the
NavigationManager.

BUG=454984
==========