[iOS] Remove extra calls to replaceState in injected {push, replace}State

The injected versions of pushState and replaceState synchronously call
the built-in version of these functions, and then send a message to
Chrome's process. After receiving this message, Chrome injects script
to perform a replaceState with the same state and URL. Since this happens
asynchronously, it will incorrectly clobber changes to history state that
have happened since the original call to pushState or replaceState.

These extra calls to replaceState are also unnecessary, since the injected
functions already call the built-in equivalents.

A previous attempt (https://crrev.com/c/1514060) at removing this logic only
did so for pushState, leading to a bug (crbug.com/949305) when a page does:
replaceState(someState, someTitle, "#replace");
pushState(someOtherState, someOtherTitle, "#push");

Because replaceState still had the extra asynchronous call, the actual
sequence that was excecuted was:
replaceState(someState, someTitle, "#replace");
pushState(someOtherState, someOtherTitle, "#push");
// A bit later:
replaceState(someState, someTitle, "#replace");

This effectively erased the pushState.

This CL removes the extra calls from both pushState and replaceState, and
adds test coverage for back-to-back calls to replaceState followed by
pushState and vice-versa.

This also fixes subtests in the following two Web Platform Tests that currently
fail in Chrome but pass in Safari:
html/browsers/history/the-history-interface/history_pushstate_url.html
html/browsers/browsing-the-web/history-traversal/persisted-user-state-restoration/scroll-restoration-fragment-scrolling-samedoc.html

Bug: 769945
Change-Id: I34b51a0a91def67cbe85ef16761497a293f1b497
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1867308
Commit-Queue: Ali Juma <ajuma@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707796}

ios/chrome/browser/web/push_and_replace_state_navigation_egtest.mm

@@ -193,6 +193,85 @@ const char* kReplaceStateRootPathSpaceURL = "http://ios/rep lace";
               pageLoaded:NO];
 }
 
+// Tests calling history.replaceState(), then history.pushState() and then
+// navigating back/forward.
+- (void)testHtml5HistoryReplaceStatePushStateThenGoBackAndForward {
+  const GURL firstReplaceStateURL = web::test::HttpServer::MakeUrl(
+      "http://ios/testing/data/http_server_files/history.html"
+      "#firstReplaceState");
+  const std::string firstReplaceStateOmniboxText =
+      net::GetContentAndFragmentForUrl(firstReplaceStateURL);
+  const GURL replaceStateThenPushStateURL = web::test::HttpServer::MakeUrl(
+      "http://ios/testing/data/http_server_files/history.html"
+      "#replaceStateThenPushState");
+  const std::string replaceStateThenPushStateOmniboxText =
+      net::GetContentAndFragmentForUrl(replaceStateThenPushStateURL);
+
+  web::test::SetUpFileBasedHttpServer();
+  [ChromeEarlGrey loadURL:web::test::HttpServer::MakeUrl(kHistoryTestUrl)];
+
+  // Replace state and then push state. Verify that at the end, the URL changed
+  // to the pushed URL and the status was updated.
+  [ChromeEarlGrey tapWebStateElementWithID:@"replaceStateThenPushState"];
+  [self assertStatusText:@"replaceStateThenPushState"
+         withOmniboxText:replaceStateThenPushStateOmniboxText
+              pageLoaded:NO];
+
+  // Go back and check URL.
+  [[EarlGrey selectElementWithMatcher:BackButton()] performAction:grey_tap()];
+  [self assertStatusText:@"firstReplaceState"
+         withOmniboxText:firstReplaceStateOmniboxText
+              pageLoaded:NO];
+
+  // Go forward and check URL.
+  [[EarlGrey selectElementWithMatcher:ForwardButton()]
+      performAction:grey_tap()];
+  [self assertStatusText:@"replaceStateThenPushState"
+         withOmniboxText:replaceStateThenPushStateOmniboxText
+              pageLoaded:NO];
+}
+
+// Tests calling history.pushState(), then history.replaceState() and then
+// navigating back/forward.
+- (void)testHtml5HistoryPushStateReplaceStateThenGoBackAndForward {
+  const GURL firstPushStateURL = web::test::HttpServer::MakeUrl(
+      "http://ios/testing/data/http_server_files/history.html#firstPushState");
+  const std::string firstPushStateOmniboxText =
+      net::GetContentAndFragmentForUrl(firstPushStateURL);
+  const GURL pushStateThenReplaceStateURL = web::test::HttpServer::MakeUrl(
+      "http://ios/testing/data/http_server_files/history.html"
+      "#pushStateThenReplaceState");
+  const std::string pushStateThenReplaceStateOmniboxText =
+      net::GetContentAndFragmentForUrl(pushStateThenReplaceStateURL);
+
+  web::test::SetUpFileBasedHttpServer();
+
+  const GURL historyTestURL = web::test::HttpServer::MakeUrl(kHistoryTestUrl);
+  [ChromeEarlGrey loadURL:historyTestURL];
+  const std::string historyTestOmniboxText =
+      net::GetContentAndFragmentForUrl(historyTestURL);
+
+  // Push state and then replace state. Verify that at the end, the URL changed
+  // to the replaceState URL and the status was updated.
+  [ChromeEarlGrey tapWebStateElementWithID:@"pushStateThenReplaceState"];
+  [self assertStatusText:@"pushStateThenReplaceState"
+         withOmniboxText:pushStateThenReplaceStateOmniboxText
+              pageLoaded:NO];
+
+  // Go back and check URL.
+  [[EarlGrey selectElementWithMatcher:BackButton()] performAction:grey_tap()];
+  [self assertStatusText:nil
+         withOmniboxText:historyTestOmniboxText
+              pageLoaded:NO];
+
+  // Go forward and check URL.
+  [[EarlGrey selectElementWithMatcher:ForwardButton()]
+      performAction:grey_tap()];
+  [self assertStatusText:@"pushStateThenReplaceState"
+         withOmniboxText:pushStateThenReplaceStateOmniboxText
+              pageLoaded:NO];
+}
+
 // Tests that page loads occur when navigating to or past a non-pushed URL.
 - (void)testHtml5HistoryNavigatingPastNonPushedURL {
   GURL nonPushedURL = web::test::HttpServer::MakeUrl(kNonPushedUrl);

ios/testing/data/http_server_files/history.html

@@ -40,6 +40,10 @@ found in the LICENSE file. -->
     id="replaceStateRootPathSpace" onclick="replaceStateRootPathSpace()" /><br>
 <input type="button" value="replaceStatePath"
     id="replaceStatePath" onclick="replaceStatePath()" /><br>
+<input type="button" value="replaceStateThenPushState"
+    id="replaceStateThenPushState" onclick="replaceStateThenPushState()" /><br>
+<input type="button" value="pushStateThenReplaceState"
+    id="pushStateThenReplaceState" onclick="pushStateThenReplaceState()" /><br>
 
 <br>
 <br>

ios/testing/data/http_server_files/history.js

@@ -118,6 +118,24 @@ function replaceStatePath() {
   updateStatusText('replaceStatePath');
 };
 
+function replaceStateThenPushState() {
+  clearOnloadDivText();
+  window.history.replaceState('firstReplaceState', 'First replaceState',
+      '#firstReplaceState');
+  window.history.pushState('replaceStateThenPushState',
+      'Replace state then push state', '#replaceStateThenPushState');
+  updateStatusText('replaceStateThenPushState');
+}
+
+function pushStateThenReplaceState() {
+  clearOnloadDivText();
+  window.history.pushState('firstPushState', 'First pushState',
+      '#firstPushState');
+  window.history.replaceState('pushStateThenReplaceState',
+      'Push state then replace state', '#pushStateThenReplaceState');
+  updateStatusText('pushStateThenReplaceState');
+}
+
 function goBack() {
   clearOnloadDivText();
   window.history.back();

ios/web/navigation/crw_js_navigation_handler.h

@@ -14,7 +14,6 @@ class WebStateImpl;
 class UserInteractionState;
 class NavigationContextImpl;
 }
-@class CRWJSInjector;
 @class CRWJSNavigationHandler;
 
 @protocol CRWJSNavigationHandlerDelegate
@@ -35,10 +34,6 @@ class NavigationContextImpl;
 - (WKWebView*)webViewForJSNavigationHandler:
     (CRWJSNavigationHandler*)navigationHandler;
 
-// Returns the associated js injector.
-- (CRWJSInjector*)JSInjectorForJSNavigationHandler:
-    (CRWJSNavigationHandler*)navigationHandler;
-
 // Instructs the delegate to update SSL status.
 - (void)JSNavigationHandlerUpdateSSLStatusForCurrentNavigationItem:
     (CRWJSNavigationHandler*)navigationHandler;

ios/web/navigation/crw_js_navigation_handler.mm

@@ -56,8 +56,6 @@ GURL URLEscapedForHistory(const GURL& url) {
     web::UserInteractionState* userInteractionState;
 // Returns WKWebView from self.delegate.
 @property(nonatomic, readonly, weak) WKWebView* webView;
-// Returns CRWJSInjector from self.delegate.
-@property(nonatomic, readonly, weak) CRWJSInjector* JSInjector;
 // Returns current URL from self.delegate.
 @property(nonatomic, readonly, assign) GURL currentURL;
 
@@ -132,10 +130,6 @@ GURL URLEscapedForHistory(const GURL& url) {
   return [self.delegate webViewForJSNavigationHandler:self];
 }
 
-- (CRWJSInjector*)JSInjector {
-  return [self.delegate JSInjectorForJSNavigationHandler:self];
-}
-
 - (GURL)currentURL {
   return [self.delegate currentURLForJSNavigationHandler:self];
 }
@@ -223,22 +217,6 @@ GURL URLEscapedForHistory(const GURL& url) {
                                  self.webView)];
   [self.delegate
       JSNavigationHandlerUpdateSSLStatusForCurrentNavigationItem:self];
-
-  // This is needed for some special pushState. See http://crbug.com/949305 .
-  NSString* replaceWebViewJS = [self javaScriptToReplaceWebViewURL:pushURL
-                                                   stateObjectJSON:stateObject];
-  __weak CRWJSNavigationHandler* weakSelf = self;
-  [self.JSInjector
-      executeJavaScript:replaceWebViewJS
-      completionHandler:^(id, NSError*) {
-        CRWJSNavigationHandler* strongSelf = weakSelf;
-        if (strongSelf && !strongSelf.beingDestroyed) {
-          [strongSelf.delegate
-              JSNavigationHandlerOptOutScrollsToTopForSubviews:self];
-          [strongSelf.delegate JSNavigationHandler:self
-                               didFinishNavigation:nullptr];
-        }
-      }];
 }
 
 // Handles the navigation.didReplaceState message sent from |senderFrame|.
@@ -286,17 +264,6 @@ GURL URLEscapedForHistory(const GURL& url) {
                     stateObject:stateObject
                  hasUserGesture:self.userInteractionState->IsUserInteracting(
                                     self.webView)];
-  NSString* replaceStateJS = [self javaScriptToReplaceWebViewURL:replaceURL
-                                                 stateObjectJSON:stateObject];
-  __weak CRWJSNavigationHandler* weakSelf = self;
-  [self.JSInjector executeJavaScript:replaceStateJS
-                   completionHandler:^(id, NSError*) {
-                     CRWJSNavigationHandler* strongSelf = weakSelf;
-                     if (!strongSelf || strongSelf.beingDestroyed)
-                       return;
-                     [strongSelf.delegate JSNavigationHandler:self
-                                          didFinishNavigation:nullptr];
-                   }];
   return;
 }
 

ios/web/web_state/ui/crw_web_controller.mm

@@ -2229,11 +2229,6 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*);
   return self.webView;
 }
 
-- (CRWJSInjector*)JSInjectorForJSNavigationHandler:
-    (CRWJSNavigationHandler*)navigationHandler {
-  return self.jsInjector;
-}
-
 - (void)JSNavigationHandlerUpdateSSLStatusForCurrentNavigationItem:
     (CRWJSNavigationHandler*)navigationHandler {
   [self updateSSLStatusForCurrentNavigationItem];