Moved NavigationManagerImpl serialization out of CRWSessionController.

ios/chrome/browser/sessions/session_window.mm

Index: ios/chrome/browser/sessions/session_window.mm
diff --git a/ios/chrome/browser/sessions/session_window.mm b/ios/chrome/browser/sessions/session_window.mm
index cef99d54e3bb0e8e33009e30053a66057a48bd2e..3193bbb20c01dd864e0fbea099a3b86547739391 100644
--- a/ios/chrome/browser/sessions/session_window.mm
+++ b/ios/chrome/browser/sessions/session_window.mm
@@ -10,6 +10,7 @@
 #include "base/format_macros.h"
 #include "base/location.h"
 #include "base/logging.h"
+#include "base/mac/scoped_nsobject.h"

[Eugene But (OOO till 7-30), 2017/01/25 19:24:29]

nit: s/include/import

[kkhorimoto, 2017/01/26 07:19:09]

Done.

 #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
 #import "ios/chrome/browser/sessions/NSCoder+Compatibility.h"
 #import "ios/chrome/browser/sessions/session_service.h"
@@ -19,18 +20,19 @@
 
 using web::WebStateImpl;
 
+// Serialization keys.
+NSString* const kSessionsKey = @"sessions";
+NSString* const kSelectedIndexKey = @"selectedIndex";
+
 @interface SessionWindowIOS () {
- @private
-  NSUInteger _selectedIndex;  // Currently selected session.
-  // For SessionWindows created via -initWithSessions:currentIndex:, the
-  // WebStateImpls in |_sessions| are owned by the calling code. When created
-  // via -initwithCoder:, the code which accepts the SessionWindow object
-  // should take or assign ownership of the contents of |_sessions|.
-  std::list<web::WebStateImpl*> _sessions;
+  // Backing objects for properties of the same name.
+  base::scoped_nsobject<NSMutableArray> _sessions;
+  NSUInteger _selectedIndex;
 }
 
-// For testing only. Empties _sessions.
-- (void)clearSessions;
+// Returns whether |index| is valid for a session window with |sessionCount|
+// entries.
+- (BOOL)isIndex:(NSUInteger)index validForSessionCount:(NSUInteger)sessionCount;
 
 @end
 
@@ -40,115 +42,78 @@ @implementation SessionWindowIOS
 
 - (id)init {
   if ((self = [super init])) {
+    _sessions.reset([[NSMutableArray alloc] init]);
     _selectedIndex = NSNotFound;
   }
   return self;
 }
 
+#pragma mark - Accessors
+
+- (NSArray*)sessions {
+  return [NSArray arrayWithArray:_sessions];
+}
+
+- (void)setSelectedIndex:(NSUInteger)selectedIndex {
+  DCHECK([self isIndex:selectedIndex validForSessionCount:[_sessions count]]);
+  _selectedIndex = selectedIndex;
+}
+
+#pragma mark - Public
+
+- (void)addSerializedSession:(CRWNavigationManagerSerialization*)session {
+  [_sessions addObject:session];
+  // Set the selected index to 0 (this session) if this is the first session
+  // added.
+  if (_selectedIndex == NSNotFound)
+    _selectedIndex = 0;
+}
+
+- (void)clearSessions {
+  [_sessions removeAllObjects];
+  _selectedIndex = NSNotFound;
+}
+
+#pragma mark - NSCoding
+
 - (id)initWithCoder:(NSCoder*)aDecoder {
   self = [super init];
   if (self) {
     DCHECK([aDecoder isKindOfClass:[SessionWindowUnarchiver class]]);
-    ios::ChromeBrowserState* browserState =
-        static_cast<SessionWindowUnarchiver*>(aDecoder).browserState;
-    DCHECK(browserState);
-    _selectedIndex = [aDecoder cr_decodeIndexForKey:@"selectedIndex"];
-    base::scoped_nsobject<NSArray> decodedSessionControllers(
-        [[aDecoder decodeObjectForKey:@"sessions"] retain]);
-    for (CRWSessionController* sc in decodedSessionControllers.get()) {
-      WebStateImpl* webState = new WebStateImpl(browserState);
-      webState->GetNavigationManagerImpl().SetSessionController(sc);
-      _sessions.push_back(webState);
-    }
-    DCHECK((_sessions.size() && _selectedIndex < _sessions.size()) ||
-           (_sessions.empty() && _selectedIndex == NSNotFound));
+    _selectedIndex = [aDecoder cr_decodeIndexForKey:kSelectedIndexKey];
+    _sessions.reset([[aDecoder decodeObjectForKey:kSessionsKey] retain]);
+    DCHECK(
+        [self isIndex:_selectedIndex validForSessionCount:[_sessions count]]);
     // If index is somehow corrupted, reset it to zero.
     // (note that if |_selectedIndex| == |_sessions.size()|, that's
     // incorrect because the maximum index of a vector of size |n| is |n-1|).
     // Empty sessions should have |_selectedIndex| values of NSNotFound.
-    if (_sessions.empty()) {
+    if (![_sessions count]) {
       _selectedIndex = NSNotFound;
-    } else if (_selectedIndex >= _sessions.size()) {
+    } else if (_selectedIndex >= [_sessions count]) {
       _selectedIndex = 0;
     }
   }
   return self;
 }
 
-- (void)dealloc {
-  DCHECK(_sessions.empty());
-  [super dealloc];
-}
-
-- (void)clearSessions {
-  while (self.unclaimedSessions) {
-    std::unique_ptr<WebStateImpl> webState = [self nextSession];
-    webState.reset();
-  }
-}
-
 - (void)encodeWithCoder:(NSCoder*)aCoder {
-  // Destructively pull all of the WebStateImpls out of |_sessions| and hand
-  // off their sessionControllers for archiving. When encoding is complete,
-  // all of the objects in |_sessions| have been destroyed.
-  NSMutableArray* sessionControllers =
-      [NSMutableArray arrayWithCapacity:_sessions.size()];
-  while (self.unclaimedSessions) {
-    std::unique_ptr<WebStateImpl> webState = [self nextSession];
-    CRWSessionController* sessionController =
-        webState->GetNavigationManagerImpl().GetSessionController();
-    [sessionControllers addObject:sessionController];
-
-    // NOTE: WebStateImpl must be destroyed on the UI thread for safety
-    // reasons.
-    web::WebThread::DeleteSoon(web::WebThread::UI, FROM_HERE,
-                               webState.release());
-  }
-
-  [aCoder cr_encodeIndex:_selectedIndex forKey:@"selectedIndex"];
-  [aCoder encodeObject:sessionControllers forKey:@"sessions"];
-}
-
-- (void)addSession:(std::unique_ptr<web::WebStateImpl>)session {
-  DCHECK(session->GetNavigationManagerImpl().GetSessionController());
-  _sessions.push_back(session.release());
-  // Set the selected index to 0 (this session) if this is the first session
-  // added.
-  if (_selectedIndex == NSNotFound)
-    _selectedIndex = 0;
-}
-
-- (void)setSelectedIndex:(NSUInteger)selectedIndex {
-  DCHECK((_sessions.size() && selectedIndex < _sessions.size()) ||
-         (_sessions.empty() && selectedIndex == NSNotFound));
-  _selectedIndex = selectedIndex;
+  [aCoder cr_encodeIndex:_selectedIndex forKey:kSelectedIndexKey];
+  [aCoder encodeObject:_sessions forKey:kSessionsKey];
 }
 
-- (std::unique_ptr<web::WebStateImpl>)nextSession {
-  std::unique_ptr<web::WebStateImpl> session(_sessions.front());
-  _sessions.pop_front();
-  return session;
-}
+#pragma mark -
 
-- (NSUInteger)unclaimedSessions {
-  return _sessions.size();
+- (BOOL)isIndex:(NSUInteger)index
+    validForSessionCount:(NSUInteger)sessionCount {
+  return (sessionCount && index < sessionCount) ||
+         (!sessionCount && index == NSNotFound);
 }
 
-#pragma mark -
-#pragma mark Debugging conveniences.
-
 - (NSString*)description {
-  NSMutableArray* sessionControllers =
-      [NSMutableArray arrayWithCapacity:_sessions.size()];
-  for (auto it = _sessions.begin(); it != _sessions.end(); ++it) {
-    CRWSessionController* sessionController =
-        (*it)->GetNavigationManagerImpl().GetSessionController();
-    [sessionControllers addObject:sessionController];
-  }
-
   return [NSString stringWithFormat:@"selected index: %" PRIuNS
                                      "\nsessions:\n%@\n",
-                                    _selectedIndex, sessionControllers];
+                                    _selectedIndex, _sessions.get()];
 }
 
 @end