Moved TabID implementation to SerializableUserData.

Moved TabID implementation to SerializableUserData.

CRWSessionController's |tabId| wasn't used within web/, so it is now
generated by Tab and stored using SerializableUserData.

BUG=454984
Review-Url: https://codereview.chromium.org/2693523003
Cr-Commit-Position: refs/heads/master@{#450614}
Committed: https://chromium.googlesource.com/chromium/src/+/cca8d4385fa16a0e7f94d86266f2b1440cc50b15

[kkhorimoto, 2017-02-11 03:35:24]

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

[Eugene But (OOO till 7-30), 2017-02-13 17:37:57]

This is great! lgtm

https://codereview.chromium.org/2693523003/diff/1/ios/chrome/browser/tabs/tab.mm
File ios/chrome/browser/tabs/tab.mm (right):

https://codereview.chromium.org/2693523003/diff/1/ios/chrome/browser/tabs/tab...
ios/chrome/browser/tabs/tab.mm:800: id<NSCoding> tabId =
userDataManager->GetValueForSerializationKey(kTabIDKey);
s/tabId/tabID

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

https://codereview.chromium.org/2693523003/diff/1/ios/web/navigation/serializ...
ios/web/navigation/serializable_user_data_manager_impl.h:36:
base::scoped_nsobject<NSDictionary> legacy_key_conversions_;
Do we need to set a milestone when we drop support for legacy keys?

https://codereview.chromium.org/2693523003/diff/1/ios/web/navigation/serializ...
ios/web/navigation/serializable_user_data_manager_impl.h:41: NSDictionary*
GetDecodedLegacyValues(NSCoder* coder);
Do you want to move method higher (before ivars)?

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

https://codereview.chromium.org/2693523003/diff/1/ios/web/navigation/serializ...
ios/web/navigation/serializable_user_data_manager_impl.mm:63:
legacy_key_conversions_(@{@"tabId" : @"TabID"}) {}
Should we keep the old tabId name to avoid extra complexity?

[kkhorimoto, 2017-02-13 23:41:18]

https://codereview.chromium.org/2693523003/diff/1/ios/chrome/browser/tabs/tab.mm
File ios/chrome/browser/tabs/tab.mm (right):

https://codereview.chromium.org/2693523003/diff/1/ios/chrome/browser/tabs/tab...
ios/chrome/browser/tabs/tab.mm:800: id<NSCoding> tabId =
userDataManager->GetValueForSerializationKey(kTabIDKey);
On 2017/02/13 17:37:57, Eugene But wrote:
> s/tabId/tabID

Done.

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

https://codereview.chromium.org/2693523003/diff/1/ios/web/navigation/serializ...
ios/web/navigation/serializable_user_data_manager_impl.h:36:
base::scoped_nsobject<NSDictionary> legacy_key_conversions_;
On 2017/02/13 17:37:57, Eugene But wrote:
> Do we need to set a milestone when we drop support for legacy keys?

Done.

https://codereview.chromium.org/2693523003/diff/1/ios/web/navigation/serializ...
ios/web/navigation/serializable_user_data_manager_impl.h:41: NSDictionary*
GetDecodedLegacyValues(NSCoder* coder);
On 2017/02/13 17:37:57, Eugene But wrote:
> Do you want to move method higher (before ivars)?

Done.

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

https://codereview.chromium.org/2693523003/diff/1/ios/web/navigation/serializ...
ios/web/navigation/serializable_user_data_manager_impl.mm:63:
legacy_key_conversions_(@{@"tabId" : @"TabID"}) {}
On 2017/02/13 17:37:57, Eugene But wrote:
> Should we keep the old tabId name to avoid extra complexity?

Since we've set a milestone to remove the legacy conversion, I think this is
okay for the short term.  I'd prefer to use serialization keys that more closely
match the style of the style of the property name.

[kkhorimoto, 2017-02-13 23:41:24]

The CQ bit was checked by kkhorimoto@chromium.org

[kkhorimoto, 2017-02-13 23:41:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
eugenebut@chromium.org
Link to the patchset: https://codereview.chromium.org/2693523003/#ps20001
(title: "Eugene's comments")

[commit-bot: I haz the power, 2017-02-13 23:42:06]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-13 23:58:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-13 23:58:09]

Failed to apply patch for ios/web/navigation/crw_session_controller.mm:
While running git apply --index -p1;
  error: patch failed: ios/web/navigation/crw_session_controller.mm:86
  error: ios/web/navigation/crw_session_controller.mm: patch does not apply

Patch:       ios/web/navigation/crw_session_controller.mm
Index: ios/web/navigation/crw_session_controller.mm
diff --git a/ios/web/navigation/crw_session_controller.mm
b/ios/web/navigation/crw_session_controller.mm
index
2374036eba26630f47937ccdeed18464b25307f1..83b11a1d38d4e18a4d012e7b4f32d877cb4acb1c
100644
--- a/ios/web/navigation/crw_session_controller.mm
+++ b/ios/web/navigation/crw_session_controller.mm
@@ -37,7 +37,6 @@ @interface CRWSessionController () {
   // the incremental merging of the two classes.
   web::NavigationManagerImpl* _navigationManager;
 
-  NSString* _tabId;  // Unique id of the tab.
   NSString* _openerId;  // Id of tab who opened this tab, empty/nil if none.
   // Navigation index of the tab which opened this tab. Do not rely on the
   // value of this member variable to indicate whether or not this tab has
@@ -86,7 +85,6 @@ @interface CRWSessionController () {
 
 // TODO(rohitrao): These properties must be redefined readwrite to work around
a
 // clang bug. crbug.com/228650
-@property(nonatomic, readwrite, copy) NSString* tabId;
 @property(nonatomic, readwrite, strong)
     CRWSessionCertificatePolicyManager* sessionCertificatePolicyManager;
 
@@ -98,7 +96,6 @@ @interface CRWSessionController () {
 @property(nonatomic, readwrite, assign) NSInteger openerNavigationIndex;
 @property(nonatomic, readwrite, assign) NSInteger previousNavigationIndex;
 
-- (NSString*)uniqueID;
 // Removes all items after currentNavigationIndex_.
 - (void)clearForwardItems;
 // Discards the transient item, if any.
@@ -123,7 +120,6 @@ - (NSArray*)entryListForItemList:(const
web::NavigationItemList&)itemList;
 
 @implementation CRWSessionController
 
-@synthesize tabId = _tabId;
 @synthesize currentNavigationIndex = _currentNavigationIndex;
 @synthesize previousNavigationIndex = _previousNavigationIndex;
 @synthesize pendingItemIndex = _pendingItemIndex;
@@ -142,7 +138,6 @@ - (id)initWithWindowName:(NSString*)windowName
   self = [super init];
   if (self) {
     self.windowName = windowName;
-    _tabId = [[self uniqueID] copy];
     _openerId = [openerId copy];
     _openedByDOM = openedByDOM;
     _openerNavigationIndex = openerIndex;
@@ -162,7 +157,6 @@ -
(id)initWithNavigationItems:(web::ScopedNavigationItemList)items
                  browserState:(web::BrowserState*)browserState {
   self = [super init];
   if (self) {
-    _tabId = [[self uniqueID] copy];
     _openerId = nil;
     _browserState = browserState;
     _items = web::CreateScopedNavigationItemImplList(std::move(items));
@@ -331,20 +325,20 @@ - (NSArray*)forwardEntries {
 
 - (NSString*)description {
   return [NSString
-      stringWithFormat:
-          @"id: %@\nname: %@\nlast visit: %f\ncurrent index: %" PRIdNS
-          @"\nprevious index: %" PRIdNS @"\npending index: %" PRIdNS
-                                        @"\n%@\npending: %@\ntransient: %@\n",
-          _tabId, self.windowName, _lastVisitedTimestamp,
-          _currentNavigationIndex, _previousNavigationIndex, _pendingItemIndex,
-          self.entries, self.pendingEntry, self.transientEntry];
+      stringWithFormat:@"name: %@\nlast visit: %f\ncurrent index: %" PRIdNS
+                       @"\nprevious index: %" PRIdNS
+                       @"\npending index: %" PRIdNS
+                       @"\n%@\npending: %@\ntransient: %@\n",
+                       self.windowName, _lastVisitedTimestamp,
+                       _currentNavigationIndex, _previousNavigationIndex,
+                       _pendingItemIndex, self.entries, self.pendingEntry,
+                       self.transientEntry];
 }
 
 #pragma mark - NSCopying
 
 - (id)copyWithZone:(NSZone*)zone {
   CRWSessionController* copy = [[[self class] alloc] init];
-  copy->_tabId = [_tabId copy];
   copy->_openerId = [_openerId copy];
   copy->_openedByDOM = _openedByDOM;
   copy->_openerNavigationIndex = _openerNavigationIndex;
@@ -678,17 +672,6 @@ - (NSInteger)indexOfItem:(web::NavigationItem*)item {
 #pragma mark -
 #pragma mark Private methods
 
-- (NSString*)uniqueID {
-  CFUUIDRef uuidRef = CFUUIDCreate(NULL);
-  CFStringRef uuidStringRef = CFUUIDCreateString(NULL, uuidRef);
-  CFRelease(uuidRef);
-
-  NSString* uuid =
-      [NSString stringWithString:base::mac::ObjCCastStrict<NSString>(
-                                     CFBridgingRelease(uuidStringRef))];
-  return uuid;
-}
-
 - (std::unique_ptr<web::NavigationItemImpl>)
         itemWithURL:(const GURL&)url
            referrer:(const web::Referrer&)referrer

[kkhorimoto, 2017-02-14 00:15:26]

The CQ bit was checked by kkhorimoto@chromium.org

[kkhorimoto, 2017-02-14 00:15:26]

The patchset sent to the CQ was uploaded after l-g-t-m from
eugenebut@chromium.org
Link to the patchset: https://codereview.chromium.org/2693523003/#ps40001
(title: "rebase")

[commit-bot: I haz the power, 2017-02-14 00:16:03]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-14 00:25:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-14 00:25:53]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[kkhorimoto, 2017-02-15 00:18:34]

The CQ bit was checked by kkhorimoto@chromium.org

[kkhorimoto, 2017-02-15 00:18:35]

The patchset sent to the CQ was uploaded after l-g-t-m from
eugenebut@chromium.org
Link to the patchset: https://codereview.chromium.org/2693523003/#ps60001
(title: "fix compile")

[commit-bot: I haz the power, 2017-02-15 00:19:32]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-15 06:58:05]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1487117914646080,
"parent_rev": "7cb5f46bd60ed4d426efce2d20403f727f66636f", "commit_rev":
"cca8d4385fa16a0e7f94d86266f2b1440cc50b15"}

[commit-bot: I haz the power, 2017-02-15 06:58:38]

Description was changed from

==========
Moved TabID implementation to SerializableUserData.

CRWSessionController's |tabId| wasn't used within web/, so it is now
generated by Tab and stored using SerializableUserData.

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

to

==========
Moved TabID implementation to SerializableUserData.

CRWSessionController's |tabId| wasn't used within web/, so it is now
generated by Tab and stored using SerializableUserData.

BUG=454984

Review-Url: https://codereview.chromium.org/2693523003
Cr-Commit-Position: refs/heads/master@{#450614}
Committed:
https://chromium.googlesource.com/chromium/src/+/cca8d4385fa16a0e7f94d86266f2...
==========

[commit-bot: I haz the power, 2017-02-15 06:58:39]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/cca8d4385fa16a0e7f94d86266f2...