Reland: [ios] Refactored back-forward navigation in CRWSessionController.

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 05f8f476bf33e2502a217d674c12dc2da33b6624..430ff519e3430404bca69eb035a1ce4f253c4084 100644
--- a/ios/web/navigation/crw_session_controller.mm
+++ b/ios/web/navigation/crw_session_controller.mm
@@ -132,6 +132,9 @@ NSString* const kXCallbackParametersKey = @"xCallbackParameters";
 // Return the PageTransition for the underlying navigationItem at |index| in
 // |entries_|
 - (ui::PageTransition)transitionForIndex:(NSUInteger)index;
+// Returns YES if the PageTransition for the underlying navigationItem at
+// |index| in |entries_| has ui::PAGE_TRANSITION_IS_REDIRECT_MASK.
+- (BOOL)isRedirectTransitionForEntryAtIndex:(NSInteger)index;
 @end
 
 @implementation CRWSessionController
@@ -612,101 +615,47 @@ NSString* const kXCallbackParametersKey = @"xCallbackParameters";
 }
 
 - (BOOL)canGoBack {
-  if ([_entries count] == 0)
-    return NO;
-
-  // A transient entry behaves from a user perspective in most ways like a
-  // committed entry, so allow going back from a transient entry since this
-  // object already has at least one committed entry.
-  if (_transientEntry)
-    return YES;
-
-  NSInteger lastNonRedirectedIndex = _currentNavigationIndex;
-  while (lastNonRedirectedIndex >= 0 &&
-         ui::PageTransitionIsRedirect(
-            [self transitionForIndex:lastNonRedirectedIndex])) {
-    --lastNonRedirectedIndex;
-  }
-
-  return lastNonRedirectedIndex > 0;
+  return [self canGoDelta:-1];
 }
 
 - (BOOL)canGoForward {
-  // In case there are pending entries return no since when the entry will be
-  // committed the history will be cleared from that point forward.
-  if (_pendingEntry)
-    return NO;
-  // If the current index is less than the last element, there are entries to
-  // go forward to.
-  const NSInteger count = [_entries count];
-  return count && _currentNavigationIndex < (count - 1);
+  return [self canGoDelta:1];
 }
 
-- (void)goBack {
-  if (![self canGoBack])
-    return;
-
-  BOOL hadTransientEntry = _transientEntry != nil;
-
-  [self discardNonCommittedEntries];
-
-  // Going back from a transient entry doesn't require anything beyond
-  // discarding the pending entry.
-  if (hadTransientEntry)
-    return;
-
-  base::RecordAction(UserMetricsAction("Back"));
-  _previousNavigationIndex = _currentNavigationIndex;
-  // To stop the user getting 'stuck' on redirecting pages they weren't even
-  // aware existed, it is necessary to pass over pages that would immediately
-  // result in a redirect (the entry *before* the redirected page).
-  while (_currentNavigationIndex &&
-         [self transitionForIndex:_currentNavigationIndex] &
-             ui::PAGE_TRANSITION_IS_REDIRECT_MASK) {
-    --_currentNavigationIndex;
-  }
+- (BOOL)canGoDelta:(int)delta {
+  NSInteger index = [self indexOfEntryForDelta:delta];
+  return 0 <= index && static_cast<NSUInteger>(index) < _entries.count;
+}
 
-  if (_currentNavigationIndex)
-    --_currentNavigationIndex;
+- (void)goBack {
+  [self goDelta:-1];
 }
 
 - (void)goForward {
-  [self discardTransientEntry];
-
-  base::RecordAction(UserMetricsAction("Forward"));
-  if (_currentNavigationIndex + 1 < static_cast<NSInteger>([_entries count])) {
-    _previousNavigationIndex = _currentNavigationIndex;
-    ++_currentNavigationIndex;
-  }
-  // To reduce the chance of a redirect kicking in (truncating the history
-  // stack) we skip over any pages that might do this; we detect this by
-  // looking for when the *next* page had rediection transition type (was
-  // auto redirected to).
-  while (_currentNavigationIndex + 1 <
-         (static_cast<NSInteger>([_entries count])) &&
-         ([self transitionForIndex:_currentNavigationIndex + 1] &
-          ui::PAGE_TRANSITION_IS_REDIRECT_MASK)) {
-    ++_currentNavigationIndex;
-  }
+  [self goDelta:1];
 }
 
 - (void)goDelta:(int)delta {
-  // Store the navigation index at the start of this function, as |-goForward|
-  // and |-goBack| will incrementally reset |_previousNavigationIndex| each time
-  // they are called.
-  NSInteger previousNavigationIndex = self.currentNavigationIndex;
+  if (delta == 0 || ![self canGoDelta:delta])
+    return;
+
+  NSInteger oldNavigationIndex = self.currentNavigationIndex;
+  NSInteger newNavigationIndex = [self indexOfEntryForDelta:delta];
+
   if (delta < 0) {
-    while ([self canGoBack] && delta < 0) {
-      [self goBack];
-      ++delta;
+    [self discardNonCommittedEntries];
+    for (int i = delta; i < 0; i++) {
+      base::RecordAction(UserMetricsAction("Back"));
     }
-  } else {
-    while ([self canGoForward] && delta > 0) {
-      [self goForward];
-      --delta;
+  } else if (delta > 0) {
+    [self discardTransientEntry];
+    for (int i = 0; i < delta; i++) {
+      base::RecordAction(UserMetricsAction("Forward"));
     }
   }
-  _previousNavigationIndex = previousNavigationIndex;
+
+  _currentNavigationIndex = newNavigationIndex;
+  _previousNavigationIndex = oldNavigationIndex;
 }
 
 - (void)goToEntry:(CRWSessionEntry*)entry {
@@ -844,6 +793,53 @@ NSString* const kXCallbackParametersKey = @"xCallbackParameters";
     _useDesktopUserAgentForNextPendingEntry = YES;
 }
 
+- (NSInteger)indexOfEntryForDelta:(int)delta {
+  NSInteger result = _currentNavigationIndex;
+  if (delta < 0) {
+    if (_transientEntry) {
+      // Going back from transient entry is a matter of discarding it and there
+      // is no need to move navigation index back.
+      delta++;
+    }
+
+    while (delta < 0) {
+      // To stop the user getting 'stuck' on redirecting pages they weren't
+      // even aware existed, it is necessary to pass over pages that would
+      // immediately result in a redirect (the entry *before* the redirected
+      // page).
+      while (result > 0 && [self isRedirectTransitionForEntryAtIndex:result]) {
+        --result;
+      }
+      --result;
+      ++delta;
+    }
+  } else if (delta > 0) {
+    NSInteger count = static_cast<NSInteger>([_entries count]);
+    if (_pendingEntry) {
+      // Chrome for iOS does not allow forward navigation if there is another
+      // pending navigation in progress. Returning invalid index indicates that
+      // forward navigation will not be allowed (and |NSNotFound| works for
+      // that). This is different from other platforms which allow forward
+      // navigation if pending entry exist.
+      // TODO(crbug.com/661858): Remove this once back-forward navigation uses
+      // pending index.
+      return NSNotFound;
+    }
+
+    while (delta > 0) {
+      ++result;
+      --delta;
+      // As with going back, skip over redirects.
+      while (result + 1 < count &&
+             [self isRedirectTransitionForEntryAtIndex:result + 1]) {
+        ++result;
+      }
+    }
+  }
+
+  return result;
+}
+
 #pragma mark -
 #pragma mark Private methods
 
@@ -886,4 +882,9 @@ NSString* const kXCallbackParametersKey = @"xCallbackParameters";
   return [[CRWSessionEntry alloc] initWithNavigationItem:std::move(item)];
 }
 
+- (BOOL)isRedirectTransitionForEntryAtIndex:(NSInteger)index {
+  ui::PageTransition transition = [self transitionForIndex:index];
+  return (transition & ui::PAGE_TRANSITION_IS_REDIRECT_MASK) ? YES : NO;

[Eugene But (OOO till 7-30), 2016/11/08 04:03:01]

https://google.github.io/styleguide/objcguide.xml#BOOL_Pitfalls

+}
+
 @end