Revert of Cleaned up old navigation code that did not use pending navigation...

Revert of Cleaned up old navigation code that did not use pending navigation item. (patchset #4 id:60001 of https://codereview.chromium.org/2751793002/ )

Reason for revert:
testReloadInterstitial is failing on an internal bot.  For an example, see build 4404 on the internal iphone10-simulator bot.

Original issue's description:
> Cleaned up old navigation code that did not use pending navigation item.
>
> Also removed PendingIndexNavigationDisabled experimental setting.
>
> BUG=692331,701717
> TEST=Back forward navigation works correctly.
>
> Review-Url: https://codereview.chromium.org/2751793002
> Cr-Commit-Position: refs/heads/master@{#457459}
> Committed: https://chromium.googlesource.com/chromium/src/+/2feaa058530953a2232e6128e16deb31192ad9d2

TBR=kkhorimoto@chromium.org,eugenebut@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=692331,701717

Review-Url: https://codereview.chromium.org/2759483002
Cr-Commit-Position: refs/heads/master@{#457519}

ios/chrome/browser/experimental_flags.h

@@ -76,6 +76,9 @@ bool IsPasswordGenerationEnabled();
 // Whether the Payment Request API is enabled or not.
 bool IsPaymentRequestEnabled();
 
+// Whether the back-forward navigation uses pending index.
+bool IsPendingIndexNavigationEnabled();
+
 // Whether the Physical Web feature is enabled.
 bool IsPhysicalWebEnabled();
 

ios/chrome/browser/experimental_flags.mm

@@ -41,6 +41,8 @@ NSString* const kHeuristicsForPasswordGeneration =
     @"HeuristicsForPasswordGeneration";
 NSString* const kMDMIntegrationDisabled = @"MDMIntegrationDisabled";
 NSString* const kOriginServerHost = @"AlternateOriginServerHost";
+NSString* const kPendingIndexNavigationDisabled =
+    @"PendingIndexNavigationDisabled";
 NSString* const kSafariVCSignInDisabled = @"SafariVCSignInDisabled";
 NSString* const kWhatsNewPromoStatus = @"WhatsNewPromoStatus";
 
@@ -199,6 +201,11 @@ bool IsPaymentRequestEnabled() {
                           base::CompareCase::INSENSITIVE_ASCII);
 }
 
+bool IsPendingIndexNavigationEnabled() {
+  return ![[NSUserDefaults standardUserDefaults]
+      boolForKey:kPendingIndexNavigationDisabled];
+}
+
 bool IsPhysicalWebEnabled() {
   base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
   if (command_line->HasSwitch(switches::kEnableIOSPhysicalWeb)) {

ios/chrome/browser/resources/Settings.bundle/Experimental.plist

@@ -4,6 +4,16 @@
 <dict>
 	<key>PreferenceSpecifiers</key>
 	<array>
+		<dict>
+			<key>Type</key>
+			<string>PSToggleSwitchSpecifier</string>
+			<key>Title</key>
+			<string>Disable pending index navigation</string>
+			<key>Key</key>
+			<string>PendingIndexNavigationDisabled</string>
+			<key>DefaultValue</key>
+			<false/>
+		</dict>
 		<dict>
 			<key>Type</key>
 			<string>PSMultiValueSpecifier</string>

ios/chrome/browser/tabs/tab.mm

@@ -1319,6 +1319,10 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
   return NO;
 }
 
+- (void)webWillFinishHistoryNavigation {
+  [parentTabModel_ notifyTabChanged:self];
+}
+
 - (void)webState:(web::WebState*)webState
     didFinishNavigation:(web::NavigationContext*)navigation {
   if (navigation->IsSameDocument()) {

ios/chrome/browser/web/visible_url_egtest.mm

@@ -7,12 +7,14 @@
 #include "base/memory/ptr_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/sys_string_conversions.h"
+#include "ios/chrome/browser/experimental_flags.h"
 #include "ios/chrome/browser/ui/ui_util.h"
 #import "ios/chrome/test/app/chrome_test_util.h"
 #import "ios/chrome/test/earl_grey/chrome_earl_grey.h"
 #import "ios/chrome/test/earl_grey/chrome_earl_grey_ui.h"
 #import "ios/chrome/test/earl_grey/chrome_matchers.h"
 #import "ios/chrome/test/earl_grey/chrome_test_case.h"
+#import "ios/testing/earl_grey/disabled_test_macros.h"
 #import "ios/web/public/test/http_server.h"
 #include "ios/web/public/test/http_server_util.h"
 #include "ios/web/public/test/response_providers/html_response_provider.h"
@@ -173,6 +175,10 @@ class PausableResponseProvider : public HtmlResponseProvider {
 // Tests that visible URL is always the same as last committed URL during
 // pending back and forward navigations.
 - (void)testBackForwardNavigation {
+  if (!experimental_flags::IsPendingIndexNavigationEnabled()) {
+    EARL_GREY_TEST_SKIPPED(@"Pending Index Navigation experiment is disabled");
+  }
+
   // Purge web view caches and pause the server to make sure that tests can
   // verify omnibox state before server starts responding.
   PurgeCachedWebViewPages();
@@ -219,6 +225,10 @@ class PausableResponseProvider : public HtmlResponseProvider {
 // Tests that visible URL is always the same as last committed URL during
 // pending navigations initialted from back history popover.
 - (void)testHistoryNavigation {
+  if (!experimental_flags::IsPendingIndexNavigationEnabled()) {
+    EARL_GREY_TEST_SKIPPED(@"Pending Index Navigation experiment is disabled");
+  }
+
   // Purge web view caches and pause the server to make sure that tests can
   // verify omnibox state before server starts responding.
   PurgeCachedWebViewPages();
@@ -248,6 +258,10 @@ class PausableResponseProvider : public HtmlResponseProvider {
 // Tests that stopping a pending Back navigation and reloading reloads committed
 // URL, not pending URL.
 - (void)testStoppingPendingBackNavigationAndReload {
+  if (!experimental_flags::IsPendingIndexNavigationEnabled()) {
+    EARL_GREY_TEST_SKIPPED(@"Pending Index Navigation experiment is disabled");
+  }
+
   // Purge web view caches and pause the server to make sure that tests can
   // verify omnibox state before server starts responding.
   PurgeCachedWebViewPages();
@@ -281,6 +295,10 @@ class PausableResponseProvider : public HtmlResponseProvider {
 // Tests that visible URL is always the same as last committed URL during
 // back forward navigations initiated with JS.
 - (void)testJSBackForwardNavigation {
+  if (!experimental_flags::IsPendingIndexNavigationEnabled()) {
+    EARL_GREY_TEST_SKIPPED(@"Pending Index Navigation experiment is disabled");
+  }
+
   // Purge web view caches and pause the server to make sure that tests can
   // verify omnibox state before server starts responding.
   PurgeCachedWebViewPages();
@@ -326,6 +344,10 @@ class PausableResponseProvider : public HtmlResponseProvider {
 // Tests that visible URL is always the same as last committed URL during go
 // navigations initiated with JS.
 - (void)testJSGoNavigation {
+  if (!experimental_flags::IsPendingIndexNavigationEnabled()) {
+    EARL_GREY_TEST_SKIPPED(@"Pending Index Navigation experiment is disabled");
+  }
+
   // Purge web view caches and pause the server to make sure that tests can
   // verify omnibox state before server starts responding.
   PurgeCachedWebViewPages();
@@ -372,6 +394,10 @@ class PausableResponseProvider : public HtmlResponseProvider {
 // Tests that visible URL is always the same as last committed URL during go
 // back navigation started with pending reload in progress.
 - (void)testBackNavigationWithPendingReload {
+  if (!experimental_flags::IsPendingIndexNavigationEnabled()) {
+    EARL_GREY_TEST_SKIPPED(@"Pending Index Navigation experiment is disabled");
+  }
+
   // Purge web view caches and pause the server to make sure that tests can
   // verify omnibox state before server starts responding.
   PurgeCachedWebViewPages();
@@ -410,6 +436,10 @@ class PausableResponseProvider : public HtmlResponseProvider {
 // back navigation initiated with pending renderer-initiated navigation in
 // progress.
 - (void)testBackNavigationWithPendingRendererInitiatedNavigation {
+  if (!experimental_flags::IsPendingIndexNavigationEnabled()) {
+    EARL_GREY_TEST_SKIPPED(@"Pending Index Navigation experiment is disabled");
+  }
+
   // Purge web view caches and pause the server to make sure that tests can
   // verify omnibox state before server starts responding.
   PurgeCachedWebViewPages();
@@ -440,6 +470,10 @@ class PausableResponseProvider : public HtmlResponseProvider {
 // renderer-initiated navigation started with pending back navigation in
 // progress.
 - (void)testRendererInitiatedNavigationWithPendingBackNavigation {
+  if (!experimental_flags::IsPendingIndexNavigationEnabled()) {
+    EARL_GREY_TEST_SKIPPED(@"Pending Index Navigation experiment is disabled");
+  }
+
   // Purge web view caches and pause the server to make sure that tests can
   // verify omnibox state before server starts responding.
   PurgeCachedWebViewPages();
@@ -470,6 +504,10 @@ class PausableResponseProvider : public HtmlResponseProvider {
 // Tests that visible URL is always the same as last committed URL if user
 // issues 2 go back commands.
 - (void)testDoubleBackNavigation {
+  if (!experimental_flags::IsPendingIndexNavigationEnabled()) {
+    EARL_GREY_TEST_SKIPPED(@"Pending Index Navigation experiment is disabled");
+  }
+
   // Create 3rd entry in the history, to be able to go back twice.
   [ChromeEarlGrey loadURL:_testURL3];
 
@@ -501,6 +539,10 @@ class PausableResponseProvider : public HtmlResponseProvider {
 // Tests that visible URL is always the same as last committed URL if page calls
 // window.history.back() twice.
 - (void)testDoubleBackJSNavigation {
+  if (!experimental_flags::IsPendingIndexNavigationEnabled()) {
+    EARL_GREY_TEST_SKIPPED(@"Pending Index Navigation experiment is disabled");
+  }
+
   // Create 3rd entry in the history, to be able to go back twice.
   [ChromeEarlGrey loadURL:_testURL3];
 

ios/web/interstitials/web_interstitial_impl.mm

@@ -84,12 +84,19 @@ void WebInterstitialImpl::DontProceed() {
 
   // Clear the pending entry, since that's the page that's not being
   // proceeded to.
-  GetWebStateImpl()->GetNavigationManager()->DiscardNonCommittedItems();
+  NavigationManager* nav_manager = GetWebStateImpl()->GetNavigationManager();
+  nav_manager->DiscardNonCommittedItems();
 
   Hide();
 
   GetDelegate()->OnDontProceed();
 
+  NSUserDefaults* user_defaults = [NSUserDefaults standardUserDefaults];
+  if (![user_defaults boolForKey:@"PendingIndexNavigationDisabled"]) {
+    // Reload last committed entry.
+    nav_manager->Reload(ReloadType::NORMAL, true /* check_for_repost */);
+  }
+
   delete this;
 }
 

ios/web/public/web_state/ui/crw_web_delegate.h

@@ -58,6 +58,9 @@ class GURL;
 - (void)webDidUpdateSessionForLoadWithParams:
             (const web::NavigationManager::WebLoadParams&)params
                         wasInitialNavigation:(BOOL)initialNavigation;
+// Called from finishHistoryNavigationFromEntry.
+// TODO(crbug.com/692331): Remove this method and use |DidFinishNavigation|.
+- (void)webWillFinishHistoryNavigation;
 
 @optional
 

ios/web/web_state/ui/crw_web_controller.mm

@@ -476,6 +476,11 @@ NSError* WKWebViewErrorWithSource(NSError* error, WKWebViewErrorSource source) {
 // 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
@@ -598,6 +603,13 @@ NSError* WKWebViewErrorWithSource(NSError* error, WKWebViewErrorSource source) {
 // 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.
@@ -706,6 +718,11 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*);
 // 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;
@@ -1390,6 +1407,40 @@ const NSTimeInterval kSnapshotOverlayTransition = 0.5;
          [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.
@@ -1795,42 +1846,56 @@ const NSTimeInterval kSnapshotOverlayTransition = 0.5;
   NavigationManager::WebLoadParams params(originalParams);
 
   // Initiating a navigation from the UI, record the current page state before
-  // the new page loads.
+  // 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.
 
   [_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));
 
   BOOL initialNavigation = NO;
-  // 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];
+  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];
 
-  [self recordStateInHistory];
+    // TODO(stuartmorgan): Why doesn't recordStateInHistory get called for
+    // forward/back transitions?
+    [self recordStateInHistory];
 
-  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::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);
+    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::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
@@ -2034,32 +2099,44 @@ const NSTimeInterval kSnapshotOverlayTransition = 0.5;
   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;
-  [self updateDesktopUserAgentForItem:toItem
-                previousUserAgentType:previousUserAgentType];
-
+  web::NavigationItem* toItem = items[index].get();
   BOOL sameDocumentNavigation =
       [sessionController isSameDocumentNavigationBetweenItem:previousItem
                                                      andItem:toItem];
-  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));
+  NSUserDefaults* userDefaults = [NSUserDefaults standardUserDefaults];
+  if (![userDefaults boolForKey:@"PendingIndexNavigationDisabled"]) {
+    [self clearTransientContentView];
 
-    [self loadCurrentURL];
+    // 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];
+    }
   }
 }
 
@@ -2153,6 +2230,38 @@ const NSTimeInterval kSnapshotOverlayTransition = 0.5;
   }
 }
 
+- (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;
@@ -2242,6 +2351,13 @@ const NSTimeInterval kSnapshotOverlayTransition = 0.5;
   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)

ios/web/web_state/ui/crw_web_controller_unittest.mm

@@ -50,6 +50,8 @@ using web::NavigationManagerImpl;
 
 @interface CRWWebController (PrivateAPI)
 @property(nonatomic, readwrite) web::PageDisplayState pageDisplayState;
+- (GURL)URLForHistoryNavigationToItem:(web::NavigationItem*)toItem
+                          previousURL:(const GURL&)previousURL;
 @end
 
 @interface CountingObserver : NSObject<CRWWebControllerObserver>
@@ -168,6 +170,72 @@ class CRWWebControllerTest : public web::WebTestWithWebController {
   base::scoped_nsobject<id> mock_web_view_;
 };
 
+#define MAKE_URL(url_string) GURL([url_string UTF8String])
+
+TEST_F(CRWWebControllerTest, UrlForHistoryNavigation) {
+  NSArray* urls_without_fragments = @[
+    @"http://one.com", @"http://two.com/", @"http://three.com/bar",
+    @"http://four.com/bar/", @"five", @"/six", @"/seven/", @""
+  ];
+
+  NSArray* fragments = @[ @"#", @"#bar" ];
+  NSMutableArray* urls_with_fragments = [NSMutableArray array];
+  for (NSString* url in urls_without_fragments) {
+    for (NSString* fragment in fragments) {
+      [urls_with_fragments addObject:[url stringByAppendingString:fragment]];
+    }
+  }
+
+  GURL previous_url;
+  web::NavigationItemImpl to_item;
+
+  // No start fragment: the end url is never changed.
+  for (NSString* start in urls_without_fragments) {
+    for (NSString* end in urls_with_fragments) {
+      previous_url = MAKE_URL(start);
+      to_item.SetURL(MAKE_URL(end));
+      EXPECT_EQ(MAKE_URL(end),
+                [web_controller() URLForHistoryNavigationToItem:&to_item
+                                                    previousURL:previous_url]);
+    }
+  }
+  // Both contain fragments: the end url is never changed.
+  for (NSString* start in urls_with_fragments) {
+    for (NSString* end in urls_with_fragments) {
+      previous_url = MAKE_URL(start);
+      to_item.SetURL(MAKE_URL(end));
+      EXPECT_EQ(MAKE_URL(end),
+                [web_controller() URLForHistoryNavigationToItem:&to_item
+                                                    previousURL:previous_url]);
+    }
+  }
+  for (unsigned start_index = 0; start_index < urls_with_fragments.count;
+       ++start_index) {
+    NSString* start = urls_with_fragments[start_index];
+    for (unsigned end_index = 0; end_index < urls_without_fragments.count;
+         ++end_index) {
+      NSString* end = urls_without_fragments[end_index];
+      previous_url = MAKE_URL(start);
+      if (start_index / 2 != end_index) {
+        // The URLs have nothing in common, they are left untouched.
+        to_item.SetURL(MAKE_URL(end));
+        EXPECT_EQ(
+            MAKE_URL(end),
+            [web_controller() URLForHistoryNavigationToItem:&to_item
+                                                previousURL:previous_url]);
+      } else {
+        // Start contains a fragment and matches end: An empty fragment is
+        // added.
+        to_item.SetURL(MAKE_URL(end));
+        EXPECT_EQ(
+            MAKE_URL([end stringByAppendingString:@"#"]),
+            [web_controller() URLForHistoryNavigationToItem:&to_item
+                                                previousURL:previous_url]);
+      }
+    }
+  }
+}
+
 // Tests that AllowCertificateError is called with correct arguments if
 // WKWebView fails to load a page with bad SSL cert.
 TEST_F(CRWWebControllerTest, SslCertError) {