[ios] Moves cached bookmark path reconstruction out of viewDidLoad.

UINavigationBar renders incorrectly when pushViewController:animated: is called
from within viewDidLoad.  Sidestep this by pushing every
BookmarkHomeViewController onto the navigation stack before making the Bookmarks
UI visible.

In cases where the BookmarkModel has not yet been loaded and a spinner is shown,
we fall back to the original logic of recursively pushing new view controllers.
That does not trigger the UIKit bug because viewDidLoad has already completed.

BUG=None

Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Id9535f6cb9e93fb85d27d4627adeb404d8564dc4
Reviewed-on: https://chromium-review.googlesource.com/1070793
Commit-Queue: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Sergio Collazos <sczs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561622}

ios/chrome/browser/ui/bookmarks/bookmark_home_view_controller.h

@@ -46,17 +46,12 @@ class BookmarkNode;
 // Class to navigate the bookmark hierarchy.
 @interface BookmarkHomeViewController : ChromeTableViewController
 
-// Set to YES, only when this view controller instance is being created
-// from cached path. Once the view controller is shown, this is set to NO.
-// This is so that the cache code is called only once in viewWillAppear, and
-// not every time the view appears.
-@property(nonatomic, assign) BOOL isReconstructingFromCache;
-
 // Delegate for presenters. Note that this delegate is currently being set only
 // in case of handset, and not tablet. In the future it will be used by both
 // cases.
 @property(nonatomic, weak) id<BookmarkHomeViewControllerDelegate> homeDelegate;
 
+// Initializers.
 - (instancetype)initWithLoader:(id<UrlLoader>)loader
                   browserState:(ios::ChromeBrowserState*)browserState
                     dispatcher:(id<ApplicationCommands>)dispatcher
@@ -69,6 +64,13 @@ class BookmarkNode;
 // Setter to set _rootNode value.
 - (void)setRootNode:(const bookmarks::BookmarkNode*)rootNode;
 
+// Returns an array of BookmarkHomeViewControllers, one per BookmarkNode in the
+// path from this view controller's node to the latest cached node (as
+// determined by BookmarkPathCache).  Includes |self| as the first element of
+// the returned array.  Sets the cached scroll position for the last element of
+// the returned array, if appropriate.
+- (NSArray<BookmarkHomeViewController*>*)cachedViewControllerStack;
+
 @end
 
 #endif  // IOS_CHROME_BROWSER_UI_BOOKMARKS_HOME_VIEW_CONTROLLER_H_

ios/chrome/browser/ui/bookmarks/bookmark_home_view_controller.mm

@@ -196,6 +196,11 @@ const CGFloat kShadowRadius = 12.0f;
 // property is set to nil after it is used.
 @property(nonatomic, strong) NSNumber* cachedContentPosition;
 
+// Set to YES, only when this view controller instance is being created
+// from cached path. Once the view controller is shown, this is set to NO.
+// This is so that the cache code is called only once in loadBookmarkViews.
+@property(nonatomic, assign) BOOL isReconstructingFromCache;
+
 // Dispatcher for sending commands.
 @property(nonatomic, readonly, weak) id<ApplicationCommands> dispatcher;
 
@@ -275,6 +280,59 @@ const CGFloat kShadowRadius = 12.0f;
   _rootNode = rootNode;
 }
 
+- (NSArray<BookmarkHomeViewController*>*)cachedViewControllerStack {
+  // This method is only designed to be called for the view controller
+  // associated with the root node.
+  DCHECK(self.bookmarks->loaded());
+  DCHECK_EQ(_rootNode, self.bookmarks->root_node());
+
+  NSMutableArray<BookmarkHomeViewController*>* stack = [NSMutableArray array];
+  [stack addObject:self];
+
+  int64_t cachedFolderID;
+  double cachedScrollPosition;
+  // If cache is present then reconstruct the last visited bookmark from
+  // cache.
+  if (![BookmarkPathCache
+          getBookmarkUIPositionCacheWithPrefService:self.browserState
+                                                        ->GetPrefs()
+                                              model:self.bookmarks
+                                           folderId:&cachedFolderID
+                                     scrollPosition:&cachedScrollPosition] ||
+      cachedFolderID == self.bookmarks->root_node()->id()) {
+    return stack;
+  }
+
+  NSArray* path =
+      bookmark_utils_ios::CreateBookmarkPath(self.bookmarks, cachedFolderID);
+  if (!path) {
+    return stack;
+  }
+
+  DCHECK_EQ(self.bookmarks->root_node()->id(),
+            [[path firstObject] longLongValue]);
+  for (NSUInteger ii = 1; ii < [path count]; ii++) {
+    int64_t nodeID = [[path objectAtIndex:ii] longLongValue];
+    const BookmarkNode* node =
+        bookmark_utils_ios::FindFolderById(self.bookmarks, nodeID);
+    DCHECK(node);
+    // if node is an empty permanent node, stop.
+    if (node->empty() && IsPrimaryPermanentNode(node, self.bookmarks)) {
+      break;
+    }
+
+    BookmarkHomeViewController* controller =
+        [self createControllerWithRootFolder:node];
+    if (nodeID == cachedFolderID) {
+      [controller
+          setCachedContentPosition:[NSNumber
+                                       numberWithDouble:cachedScrollPosition]];
+    }
+    [stack addObject:controller];
+  }
+  return stack;
+}
+
 #pragma mark - UIViewController
 
 - (void)viewDidLoad {
@@ -870,51 +928,10 @@ const CGFloat kShadowRadius = 12.0f;
 - (void)setupUIStackCacheIfApplicable {
   self.isReconstructingFromCache = NO;
 
-  int64_t folderId;
-  double scrollPosition;
-  // If folderId is invalid or rootNode reached the cached folderId, stop
-  // stacking and return.
-  if (![BookmarkPathCache
-          getBookmarkUIPositionCacheWithPrefService:self.browserState
-                                                        ->GetPrefs()
-                                              model:self.bookmarks
-                                           folderId:&folderId
-                                     scrollPosition:&scrollPosition] ||
-      folderId == _rootNode->id()) {
-    return;
-  }
-
-  // Otherwise drill down until we recreate the UI stack for the cached bookmark
-  // path.
-  NSMutableArray* mutablePath = [bookmark_utils_ios::CreateBookmarkPath(
-      self.bookmarks, folderId) mutableCopy];
-  if (!mutablePath) {
-    return;
-  }
-  NSArray* thisBookmarkPath =
-      bookmark_utils_ios::CreateBookmarkPath(self.bookmarks, _rootNode->id());
-  if (!thisBookmarkPath) {
-    return;
-  }
-
-  [mutablePath removeObjectsInArray:thisBookmarkPath];
-  const BookmarkNode* node = bookmark_utils_ios::FindFolderById(
-      self.bookmarks, [[mutablePath firstObject] longLongValue]);
-  DCHECK(node);
-  // if node is an empty permanent node, return.
-  if (node->empty() && IsPrimaryPermanentNode(node, self.bookmarks)) {
-    return;
-  }
-
-  BookmarkHomeViewController* controller =
-      [self createControllerWithRootFolder:node];
-  // Only scroll to the last viewing position for the leaf node.
-  if (mutablePath.count == 1 && scrollPosition) {
-    [controller
-        setCachedContentPosition:[NSNumber numberWithDouble:scrollPosition]];
-  }
-  controller.isReconstructingFromCache = YES;
-  [self.navigationController pushViewController:controller animated:NO];
+  NSArray<BookmarkHomeViewController*>* replacementViewControllers =
+      [self cachedViewControllerStack];
+  DCHECK(replacementViewControllers);
+  [self.navigationController setViewControllers:replacementViewControllers];
 }
 
 // Set up context bar for the new UI.

ios/chrome/browser/ui/bookmarks/bookmark_interaction_controller.mm

@@ -182,32 +182,24 @@ using bookmarks::BookmarkNode;
                                               dispatcher:self.dispatcher];
   self.bookmarkBrowser.homeDelegate = self;
 
-  // Set the root node if the model has been loaded. If the model has not been
-  // loaded yet, the root node will be set in BookmarkHomeViewController after
-  // the model is finished loading.
+  NSArray<BookmarkHomeViewController*>* replacementViewControllers = nil;
   if (self.bookmarkModel->loaded()) {
+    // Set the root node if the model has been loaded. If the model has not been
+    // loaded yet, the root node will be set in BookmarkHomeViewController after
+    // the model is finished loading.
     [self.bookmarkBrowser setRootNode:self.bookmarkModel->root_node()];
-  }
-
-  int64_t unusedFolderId;
-  double unusedScrollPosition;
-  // If cache is present then reconstruct the last visited bookmark from
-  // cache.  If bookmarkModel is not loaded yet, the following checking will
-  // be done again at bookmarkModelLoaded in BookmarkHomeViewController to
-  // prevent http://crbug.com/765503.
-  if ([BookmarkPathCache
-          getBookmarkUIPositionCacheWithPrefService:_currentBrowserState
-                                                        ->GetPrefs()
-                                              model:self.bookmarkModel
-                                           folderId:&unusedFolderId
-                                     scrollPosition:&unusedScrollPosition]) {
-    self.bookmarkBrowser.isReconstructingFromCache = YES;
+    replacementViewControllers =
+        [self.bookmarkBrowser cachedViewControllerStack];
   }
 
   if (experimental_flags::IsBookmarksUIRebootEnabled()) {
     TableViewNavigationController* navController =
         [[TableViewNavigationController alloc]
             initWithTable:self.bookmarkBrowser];
+    if (replacementViewControllers) {
+      [navController setViewControllers:replacementViewControllers];
+    }
+
     navController.toolbarHidden = YES;
     self.bookmarkTransitioningDelegate =
         [[BookmarkTransitioningDelegate alloc] init];
@@ -220,6 +212,9 @@ using bookmarks::BookmarkNode;
     FormSheetNavigationController* navController =
         [[FormSheetNavigationController alloc]
             initWithRootViewController:self.bookmarkBrowser];
+    if (replacementViewControllers) {
+      [navController setViewControllers:replacementViewControllers];
+    }
     [navController setModalPresentationStyle:UIModalPresentationFormSheet];
     [_parentController presentViewController:navController
                                     animated:YES