Cleaned up old navigation code that did not use pending navigation item.

ios/web/web_state/ui/crw_web_controller.mm

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import "ios/web/web_state/ui/crw_web_controller.h"
 
 #import <WebKit/WebKit.h>
 
 #import <objc/runtime.h>
 #include <stddef.h>
@@ skipping 458 matching lines @@
 // Returns the navigation item for the current page.
 @property(nonatomic, readonly) web::NavigationItemImpl* currentNavItem;
 // Returns the current transition type.
 @property(nonatomic, readonly) ui::PageTransition currentTransition;
 // Returns the referrer for current navigation item. May be empty.
 @property(nonatomic, readonly) web::Referrer currentNavItemReferrer;
 // The HTTP headers associated with the current navigation item. These are nil
 // unless the request was a POST.
 @property(nonatomic, readonly) NSDictionary* currentHTTPHeaders;
 
-// TODO(crbug.com/684098): Remove these methods and inline their content.
-// Called before finishing a history navigation from a page with the given
-// UserAgentType.
-- (void)webWillFinishHistoryNavigationWithPreviousUserAgentType:
-    (web::UserAgentType)userAgentType;
 // Requires page reconstruction if |item| has a non-NONE UserAgentType and it
 // differs from that of |fromItem|.
 - (void)updateDesktopUserAgentForItem:(web::NavigationItem*)item
                 previousUserAgentType:(web::UserAgentType)userAgentType;
 
 // Removes the container view from the hierarchy and resets the ivar.
 - (void)resetContainerView;
 // Called when the web page has changed document and/or URL, and so the page
 // navigation should be reported to the delegate, and internal state updated to
 // reflect the fact that the navigation has occurred.
@@ skipping 102 matching lines @@
 - (void)didFinishNavigation;
 // Update the appropriate parts of the model and broadcast to the embedder. This
 // may be called multiple times and thus must be idempotent.
 - (void)loadCompleteWithSuccess:(BOOL)loadSuccess;
 // Called after URL is finished loading and _loadPhase is set to PAGE_LOADED.
 - (void)didFinishWithURL:(const GURL&)currentURL loadSuccess:(BOOL)loadSuccess;
 // Navigates forwards or backwards by |delta| pages. No-op if delta is out of
 // bounds. Reloads if delta is 0.
 // TODO(crbug.com/661316): Move this method to NavigationManager.
 - (void)goDelta:(int)delta;
-// Loads a new URL if the current entry is not from a pushState() navigation.
-// |fromURL| is the URL of the previous NavigationItem, |fromUserAgentType| is
-// that item's UserAgentType, and |sameDocument| is YES if the navigation is
-// between two pages with the same document.
-- (void)finishHistoryNavigationFromURL:(const GURL&)fromURL
-                         userAgentType:(web::UserAgentType)fromUserAgentType
-                          sameDocument:(BOOL)sameDocument;
 // Informs the native controller if web usage is allowed or not.
 - (void)setNativeControllerWebUsageEnabled:(BOOL)webUsageEnabled;
 // Called when web controller receives a new message from the web page.
 - (void)didReceiveScriptMessage:(WKScriptMessage*)message;
 // Returns a new script which wraps |script| with windowID check so |script| is
 // not evaluated on windowID mismatch.
 - (NSString*)scriptByAddingWindowIDCheckForScript:(NSString*)script;
 // Attempts to handle a script message. Returns YES on success, NO otherwise.
 - (BOOL)respondToWKScriptMessage:(WKScriptMessage*)scriptMessage;
 // Registers load request with empty referrer and link or client redirect
@@ skipping 88 matching lines @@
 - (BOOL)isCurrentNavigationBackForward;
 // Returns whether the given navigation is triggered by a user link click.
 - (BOOL)isLinkNavigation:(WKNavigationType)navigationType;
 
 // Inject windowID if not yet injected.
 - (void)injectWindowID;
 
 // Returns YES if the given WKBackForwardListItem is valid to use for
 // navigation.
 - (BOOL)isBackForwardListItemValid:(WKBackForwardListItem*)item;
-// Compares the two URLs being navigated between during a history navigation to
-// determine if a # needs to be appended to the URL of |toItem| to trigger a
-// hashchange event. If so, also saves the modified URL into |toItem|.
-- (GURL)URLForHistoryNavigationToItem:(web::NavigationItem*)toItem
-                          previousURL:(const GURL&)previousURL;
 // Finds all the scrollviews in the view hierarchy and makes sure they do not
 // interfere with scroll to top when tapping the statusbar.
 - (void)optOutScrollsToTopForSubviews;
 // Tears down the old native controller, and then replaces it with the new one.
 - (void)setNativeController:(id<CRWNativeContent>)nativeController;
 // Returns whether |url| should be opened.
 - (BOOL)shouldOpenURL:(const GURL&)url
       mainDocumentURL:(const GURL&)mainDocumentURL
           linkClicked:(BOOL)linkClicked;
 // Called when |URL| needs to be opened in a matching native app.
@@ skipping 664 matching lines @@
 
 - (BOOL)isBackForwardListItemValid:(WKBackForwardListItem*)item {
   // The current back-forward list item MUST be in the WKWebView's back-forward
   // list to be valid.
   WKBackForwardList* list = [_webView backForwardList];
   return list.currentItem == item ||
          [list.forwardList indexOfObject:item] != NSNotFound ||
          [list.backList indexOfObject:item] != NSNotFound;
 }
 
-- (GURL)URLForHistoryNavigationToItem:(web::NavigationItem*)toItem
-                          previousURL:(const GURL&)previousURL {
-  // If navigating with native API, i.e. using a back forward list item,
-  // hashchange events will be triggered automatically, so no URL tampering is
-  // required.
-  web::WKBackForwardListItemHolder* holder =
-      web::WKBackForwardListItemHolder::FromNavigationItem(toItem);
-  if (holder->back_forward_list_item()) {
-    return toItem->GetURL();
-  }
-
-  const GURL& URL = toItem->GetURL();
-
-  // Check the state of the fragments on both URLs (aka, is there a '#' in the
-  // url or not).
-  if (!previousURL.has_ref() || URL.has_ref()) {
-    return URL;
-  }
-
-  // startURL contains a fragment and endURL doesn't. Remove the fragment from
-  // startURL and compare the resulting string to endURL. If they are equal, add
-  // # to endURL to cause a hashchange event.
-  GURL hashless = web::GURLByRemovingRefFromGURL(previousURL);
-
-  if (hashless != URL)
-    return URL;
-
-  url::StringPieceReplacements<std::string> emptyRef;
-  emptyRef.SetRefStr("");
-  GURL newEndURL = URL.ReplaceComponents(emptyRef);
-  toItem->SetURL(newEndURL);
-  return newEndURL;
-}
-
 - (void)injectWindowID {
   // Default value for shouldSuppressDialogs is NO, so updating them only
   // when necessary is a good optimization.
   if (_shouldSuppressDialogsOnWindowIDInjection) {
     self.shouldSuppressDialogs = YES;
     _shouldSuppressDialogsOnWindowIDInjection = NO;
   }
 
   [_windowIDJSManager inject];
 }
@@ skipping 385 matching lines @@
                    referrer:referrer
                  transition:self.currentTransition];
   [self loadNativeViewWithSuccess:YES];
 }
 
 - (void)loadWithParams:(const NavigationManager::WebLoadParams&)originalParams {
   // Make a copy of |params|, as some of the delegate methods may modify it.
   NavigationManager::WebLoadParams params(originalParams);
 
   // Initiating a navigation from the UI, record the current page state before
-  // the new page loads. Don't record for back/forward, as the current entry
-  // has already been moved to the next entry in the history. Do, however,
-  // record it for general reload.
-  // TODO(jimblackler): consider a single unified call to record state whenever
-  // the page is about to be changed. This cannot currently be done after
-  // addPendingItem is called.
+  // the new page loads.
 
   [_delegate webWillInitiateLoadWithParams:params];
 
   GURL navUrl = params.url;
   ui::PageTransition transition = params.transition_type;
+  DCHECK(!(transition & ui::PAGE_TRANSITION_FORWARD_BACK));
+  DCHECK(!(transition & ui::PAGE_TRANSITION_RELOAD));

[rohitrao (ping after 24h), 2017/03/16 19:36:54]

Looks like this DCHECK is firing during an internal test:

[0316/104559.588473:FATAL:crw_web_controller.mm(1805)] Check failed:
!(transition & ui::PAGE_TRANSITION_RELOAD).
0   ios_internal_chrome_external_url_egtests 0x0157cec6
base::debug::StackTrace::StackTrace(unsigned long) + 134
1   ios_internal_chrome_external_url_egtests 0x0157cf44
base::debug::StackTrace::StackTrace(unsigned long) + 36
2   ios_internal_chrome_external_url_egtests 0x0157bbb7
base::debug::StackTrace::StackTrace() + 39
3   ios_internal_chrome_external_url_egtests 0x015e299c
logging::LogMessage::~LogMessage() + 92
4   ios_internal_chrome_external_url_egtests 0x015e06d7
logging::LogMessage::~LogMessage() + 23
5   ios_internal_chrome_external_url_egtests 0x003758d4 -[CRWWebController
loadWithParams:] + 660
6   ios_internal_chrome_external_url_egtests 0x0037728d -[CRWWebController
reloadInternal] + 493
7   ios_internal_chrome_external_url_egtests 0x003773bb -[CRWWebController
reload] + 91
8   ios_internal_chrome_external_url_egtests 0x00e3ebb5 -[Tab reload] + 69
9   ios_internal_chrome_external_url_egtests 0x00cb61f3 -[BrowserViewController
chromeExecuteCommand:] + 3011
10  ios_internal_chrome_external_url_egtests 0x00629eb5
-[UIResponder(ChromeExecuteCommand) chromeExecuteCommand:] + 133
11  ios_internal_chrome_external_url_egtests 0x00629eb5
-[UIResponder(ChromeExecuteCommand) chromeExecuteCommand:] + 133
12  ios_internal_chrome_external_url_egtests 0x00629eb5
-[UIResponder(ChromeExecuteCommand) chromeExecuteCommand:] + 133
13  ios_internal_chrome_external_url_egtests 0x00629eb5
-[UIResponder(ChromeExecuteCommand) chromeExecuteCommand:] + 133
14  ios_internal_chrome_external_url_egtests 0x00c26216
-[ToolsMenuViewController buttonPressed:] + 758

 
   BOOL initialNavigation = NO;
-  BOOL forwardBack =
-      PageTransitionCoreTypeIs(transition, ui::PAGE_TRANSITION_RELOAD) &&
-      (transition & ui::PAGE_TRANSITION_FORWARD_BACK);
-  if (forwardBack) {
-    // Setting these for back/forward is not supported.
-    DCHECK(!params.extra_headers);
-    DCHECK(!params.post_data);
-  } else {
-    // Clear transient view before making any changes to history and navigation
-    // manager. TODO(stuartmorgan): Drive Transient Item clearing from
-    // navigation system, rather than from WebController.
-    [self clearTransientContentView];
+  // Clear transient view before making any changes to history and navigation
+  // manager. TODO(stuartmorgan): Drive Transient Item clearing from
+  // navigation system, rather than from WebController.
+  [self clearTransientContentView];
 
-    // TODO(stuartmorgan): Why doesn't recordStateInHistory get called for
-    // forward/back transitions?
-    [self recordStateInHistory];
+  [self recordStateInHistory];
 
-    if (!self.currentNavItem)
-      initialNavigation = YES;
+  if (!self.currentNavItem)
+    initialNavigation = YES;
 
-    web::NavigationInitiationType navigationInitiationType =
-        params.is_renderer_initiated
-            ? web::NavigationInitiationType::RENDERER_INITIATED
-            : web::NavigationInitiationType::USER_INITIATED;
-    self.navigationManagerImpl->AddPendingItem(
-        navUrl, params.referrer, transition, navigationInitiationType);
+  web::NavigationInitiationType navigationInitiationType =
+      params.is_renderer_initiated
+          ? web::NavigationInitiationType::RENDERER_INITIATED
+          : web::NavigationInitiationType::USER_INITIATED;
+  self.navigationManagerImpl->AddPendingItem(
+      navUrl, params.referrer, transition, navigationInitiationType);
 
-    web::NavigationItemImpl* addedItem = self.currentNavItem;
-    DCHECK(addedItem);
-    if (params.extra_headers)
-      addedItem->AddHttpRequestHeaders(params.extra_headers);
-    if (params.post_data) {
-      DCHECK([addedItem->GetHttpRequestHeaders() objectForKey:@"Content-Type"])
-          << "Post data should have an associated content type";
-      addedItem->SetPostData(params.post_data);
-      addedItem->SetShouldSkipRepostFormConfirmation(true);
-    }
+  web::NavigationItemImpl* addedItem = self.currentNavItem;
+  DCHECK(addedItem);
+  if (params.extra_headers)
+    addedItem->AddHttpRequestHeaders(params.extra_headers);
+  if (params.post_data) {
+    DCHECK([addedItem->GetHttpRequestHeaders() objectForKey:@"Content-Type"])
+        << "Post data should have an associated content type";
+    addedItem->SetPostData(params.post_data);
+    addedItem->SetShouldSkipRepostFormConfirmation(true);
   }
 
   [_delegate webDidUpdateSessionForLoadWithParams:params
                              wasInitialNavigation:initialNavigation];
 
   [self loadCurrentURL];
 }
 
 - (void)loadCurrentURL {
   // If the content view doesn't exist, the tab has either been evicted, or
@@ skipping 183 matching lines @@
   CRWSessionController* sessionController = self.sessionController;
   const web::ScopedNavigationItemImplList& items = sessionController.items;
   if (index < 0 || index >= static_cast<int>(items.size())) {
     NOTREACHED();
     return;
   }
 
   if (!_webStateImpl->IsShowingWebInterstitial())
     [self recordStateInHistory];
 
+  [self clearTransientContentView];
+
+  // Update the user agent before attempting the navigation.
+  web::NavigationItem* toItem = items[index].get();
   web::NavigationItem* previousItem = sessionController.currentItem;
-  GURL previousURL = previousItem ? previousItem->GetURL() : GURL::EmptyGURL();
   web::UserAgentType previousUserAgentType =
       previousItem ? previousItem->GetUserAgentType()
                    : web::UserAgentType::NONE;
-  web::NavigationItem* toItem = items[index].get();
+  [self updateDesktopUserAgentForItem:toItem
+                previousUserAgentType:previousUserAgentType];
+
   BOOL sameDocumentNavigation =
       [sessionController isSameDocumentNavigationBetweenItem:previousItem
                                                      andItem:toItem];
+  if (sameDocumentNavigation) {
+    [sessionController goToItemAtIndex:index];
+    [self updateHTML5HistoryState];
+  } else {
+    [sessionController discardNonCommittedItems];
+    [sessionController setPendingItemIndex:index];
 
-  NSUserDefaults* userDefaults = [NSUserDefaults standardUserDefaults];
-  if (![userDefaults boolForKey:@"PendingIndexNavigationDisabled"]) {
-    [self clearTransientContentView];
+    web::NavigationItemImpl* pendingItem = sessionController.pendingItem;
+    pendingItem->SetTransitionType(ui::PageTransitionFromInt(
+        pendingItem->GetTransitionType() | ui::PAGE_TRANSITION_FORWARD_BACK));
 
-    // Update the user agent before attempting the navigation.
-    [self updateDesktopUserAgentForItem:toItem
-                  previousUserAgentType:previousUserAgentType];
-
-    if (sameDocumentNavigation) {
-      [sessionController goToItemAtIndex:index];
-      [self updateHTML5HistoryState];
-    } else {
-      [sessionController discardNonCommittedItems];
-      [sessionController setPendingItemIndex:index];
-
-      web::NavigationItemImpl* pendingItem = sessionController.pendingItem;
-      pendingItem->SetTransitionType(ui::PageTransitionFromInt(
-          pendingItem->GetTransitionType() | ui::PAGE_TRANSITION_FORWARD_BACK));
-
-      [self loadCurrentURL];
-    }
-  } else {
-    [sessionController goToItemAtIndex:index];
-    if (previousURL.is_valid()) {
-      [self finishHistoryNavigationFromURL:previousURL
-                             userAgentType:previousUserAgentType
-                              sameDocument:sameDocumentNavigation];
-    }
+    [self loadCurrentURL];
   }
 }
 
 - (BOOL)isLoaded {
   return _loadPhase == web::PAGE_LOADED;
 }
 
 - (void)didFinishNavigation {
   // This can be called at multiple times after the document has loaded. Do
   // nothing if the document has already loaded.
@@ skipping 73 matching lines @@
     [self reload];
     return;
   }
 
   if (self.navigationManagerImpl->CanGoToOffset(delta)) {
     NSInteger index = self.navigationManagerImpl->GetIndexForOffset(delta);
     [self goToItemAtIndex:index];
   }
 }
 
-- (void)finishHistoryNavigationFromURL:(const GURL&)fromURL
-                         userAgentType:(web::UserAgentType)fromUserAgentType
-                          sameDocument:(BOOL)sameDocument {
-  [self webWillFinishHistoryNavigationWithPreviousUserAgentType:
-            fromUserAgentType];
-
-  // Only load the new URL if it has a different document than |fromEntry| to
-  // prevent extra page loads from NavigationItems created by hash changes or
-  // calls to window.history.pushState().
-  web::NavigationItem* currentItem = self.currentNavItem;
-  GURL endURL =
-      [self URLForHistoryNavigationToItem:currentItem previousURL:fromURL];
-  if (!sameDocument) {
-    ui::PageTransition transition = ui::PageTransitionFromInt(
-        ui::PAGE_TRANSITION_RELOAD | ui::PAGE_TRANSITION_FORWARD_BACK);
-
-    NavigationManager::WebLoadParams params(endURL);
-    if (currentItem) {
-      params.referrer = currentItem->GetReferrer();
-    }
-    params.transition_type = transition;
-    [self loadWithParams:params];
-  }
-  // If this is a same-document navigation, update the HTML5 history state
-  // immediately.  For cross-document navigations, the history state will be
-  // updated after the navigation is committed, as attempting to replace the URL
-  // here will result in a JavaScript SecurityError due to the URLs having
-  // different origins.
-  if (sameDocument)
-    [self updateHTML5HistoryState];
-}
-
 - (void)addGestureRecognizerToWebView:(UIGestureRecognizer*)recognizer {
   if ([_gestureRecognizers containsObject:recognizer])
     return;
 
   [_webView addGestureRecognizer:recognizer];
   [_gestureRecognizers addObject:recognizer];
 }
 
 - (void)removeGestureRecognizerFromWebView:(UIGestureRecognizer*)recognizer {
   if (![_gestureRecognizers containsObject:recognizer])
@@ skipping 69 matching lines @@
       [delegate webController:strongSelf didLoadPassKitObject:data];
     }
   };
   web::BrowserState* browserState = self.webStateImpl->GetBrowserState();
   _passKitDownloader.reset([[CRWPassKitDownloader alloc]
       initWithContextGetter:browserState->GetRequestContext()
           completionHandler:passKitCompletion]);
   return _passKitDownloader.get();
 }
 
-- (void)webWillFinishHistoryNavigationWithPreviousUserAgentType:
-    (web::UserAgentType)userAgentType {
-  [self updateDesktopUserAgentForItem:self.currentNavItem
-                previousUserAgentType:userAgentType];
-  [_delegate webWillFinishHistoryNavigation];
-}
-
 - (void)updateDesktopUserAgentForItem:(web::NavigationItem*)item
                 previousUserAgentType:(web::UserAgentType)userAgentType {
   if (!item)
     return;
   web::UserAgentType itemUserAgentType = item->GetUserAgentType();
   if (itemUserAgentType == web::UserAgentType::NONE)
     return;
   if (itemUserAgentType != userAgentType)
     [self requirePageReconstruction];
 }
@@ skipping 2819 matching lines @@
 - (void)simulateLoadRequestWithURL:(const GURL&)URL {
   _lastRegisteredRequestURL = URL;
   _loadPhase = web::LOAD_REQUESTED;
 }
 
 - (NSString*)referrerFromNavigationAction:(WKNavigationAction*)action {
   return [action.request valueForHTTPHeaderField:kReferrerHeaderName];
 }
 
 @end