[ios/web] Fix more algebra errors in CreateRestoreSessionUrl.

Bug: 945865
Change-Id: Ief57de24729bd99e7dff1612677e594ac109dc63
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1540059
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Auto-Submit: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#644904}

ios/web/navigation/wk_based_navigation_manager_impl.mm

@@ -597,8 +597,13 @@ void WKBasedNavigationManagerImpl::UnsafeRestore(
   // TODO(crbug.com/771200): Retain these original NavigationItems restored from
   // storage and associate them with new WKBackForwardListItems created after
   // history restore so information such as scroll position is restored.
-  GURL url = wk_navigation_util::CreateRestoreSessionUrl(
-      last_committed_item_index, items);
+  int first_index = -1;
+  GURL url;
+  wk_navigation_util::CreateRestoreSessionUrl(last_committed_item_index, items,
+                                              &url, &first_index);
+  DCHECK_GE(first_index, 0);
+  DCHECK_LT(base::checked_cast<NSUInteger>(first_index), items.size());
+  DCHECK(url.is_valid());
 
   WebLoadParams params(url);
   // It's not clear how this transition type will be used and what's the impact.
@@ -607,11 +612,11 @@ void WKBasedNavigationManagerImpl::UnsafeRestore(
   params.transition_type = ui::PAGE_TRANSITION_RELOAD;
 
   // This pending item will become the first item in the restored history.
-  params.virtual_url = items[0]->GetVirtualURL();
+  params.virtual_url = items[first_index]->GetVirtualURL();
 
   // Grab the title of the first item before |restored_visible_item_| (which may
   // or may not be the first index) is moved out of |items| below.
-  const base::string16& firstTitle = items[0]->GetTitle();
+  const base::string16& firstTitle = items[first_index]->GetTitle();
 
   // Ordering is important. Cache the visible item of the restored session
   // before starting the new navigation, which may trigger client lookup of

ios/web/navigation/wk_navigation_util.h

@@ -50,13 +50,16 @@ bool URLNeedsUserAgentType(const GURL& url);
 // This is used in unit tests.
 GURL GetRestoreSessionBaseUrl();
 
-// Creates a restore_session.html URL with the provided session history encoded
-// in the URL fragment, such that when this URL is loaded in the web view,
-// recreates all the history entries in |items| and the current loaded item is
-// the entry at |last_committed_item_index|.
-GURL CreateRestoreSessionUrl(
+// Creates a restore_session.html |url| with the provided session
+// history encoded in the URL fragment, such that when this URL is loaded in the
+// web view, recreates all the history entries in |items| and the current loaded
+// item is the entry at |last_committed_item_index|.  Sets |first_index| to the
+// new beginning of items.
+void CreateRestoreSessionUrl(
     int last_committed_item_index,
-    const std::vector<std::unique_ptr<NavigationItem>>& items);
+    const std::vector<std::unique_ptr<NavigationItem>>& items,
+    GURL* url,
+    int* first_index);
 
 // Returns true if the base URL of |url| is restore_session.html.
 bool IsRestoreSessionUrl(const GURL& url);

ios/web/navigation/wk_navigation_util.mm

@@ -63,13 +63,15 @@ int GetSafeItemIterators(
     // on the left side of the vector. Trim those.
     *begin = items.end() - kMaxSessionSize;
     *end = items.end();
-    return last_committed_item_index - kMaxSessionSize;
+  } else {
+    // Trim items from both sides of the vector. Keep the same number of items
+    // on the left and right side of |last_committed_item_index|.
+    *begin = items.begin() + last_committed_item_index - kMaxSessionSize / 2;
+    *end = items.begin() + last_committed_item_index + kMaxSessionSize / 2 + 1;
   }
 
-  // Trim items from both sides of the vector. Keep the same number of items
-  // on the left and right side of |last_committed_item_index|.
-  *begin = items.begin() + last_committed_item_index - kMaxSessionSize / 2;
-  *end = items.begin() + last_committed_item_index + kMaxSessionSize / 2 + 1;
+  // The beginning of the vector has been trimmed, so move up the last committed
+  // item index by whatever was trimmed from the left.
   return last_committed_item_index - (*begin - items.begin());
 }
 }
@@ -109,9 +111,11 @@ GURL GetRestoreSessionBaseUrl() {
   return GURL(url::kAboutBlankURL).ReplaceComponents(replacements);
 }
 
-GURL CreateRestoreSessionUrl(
+void CreateRestoreSessionUrl(
     int last_committed_item_index,
-    const std::vector<std::unique_ptr<NavigationItem>>& items) {
+    const std::vector<std::unique_ptr<NavigationItem>>& items,
+    GURL* url,
+    int* first_index) {
   DCHECK(last_committed_item_index >= 0 &&
          last_committed_item_index < static_cast<int>(items.size()));
 
@@ -151,7 +155,8 @@ GURL CreateRestoreSessionUrl(
       net::EscapeQueryParamValue(session_json, false /* use_plus */);
   GURL::Replacements replacements;
   replacements.SetRefStr(ref);
-  return GetRestoreSessionBaseUrl().ReplaceComponents(replacements);
+  *first_index = begin - items.begin();
+  *url = GetRestoreSessionBaseUrl().ReplaceComponents(replacements);
 }
 
 bool IsRestoreSessionUrl(const GURL& url) {

ios/web/navigation/wk_navigation_util_unittest.mm

@@ -76,8 +76,11 @@ TEST_F(WKNavigationUtilTest, CreateRestoreSessionUrl) {
   items.push_back(std::move(item1));
   items.push_back(std::move(item2));
 
-  GURL restore_session_url =
-      CreateRestoreSessionUrl(0 /* last_committed_item_index */, items);
+  int first_index = 0;
+  GURL restore_session_url;
+  CreateRestoreSessionUrl(0 /* last_committed_item_index */, items,
+                          &restore_session_url, &first_index);
+  ASSERT_EQ(0, first_index);
   ASSERT_TRUE(IsRestoreSessionUrl(restore_session_url));
 
   net::UnescapeRule::Type unescape_rules =
@@ -99,8 +102,11 @@ TEST_F(WKNavigationUtilTest, CreateRestoreSessionUrlForLargeSession) {
   const size_t kItemCount = kMaxSessionSize;
   std::vector<std::unique_ptr<NavigationItem>> items;
   CreateTestNavigationItems(kItemCount, items);
-  GURL restore_session_url =
-      CreateRestoreSessionUrl(/*last_committed_item_index=*/0, items);
+  int first_index = 0;
+  GURL restore_session_url;
+  CreateRestoreSessionUrl(
+      /*last_committed_item_index=*/0, items, &restore_session_url,
+      &first_index);
   ASSERT_TRUE(IsRestoreSessionUrl(restore_session_url));
 
   // Extract session JSON from restoration URL.
@@ -126,11 +132,15 @@ TEST_F(WKNavigationUtilTest, CreateRestoreSessionUrlForLargeSession) {
 TEST_F(WKNavigationUtilTest, CreateRestoreSessionUrlForExtraLargeForwardList) {
   // Create restore session URL with large number of items that exceeds
   // kMaxSessionSize.
-  const size_t kItemCount = kMaxSessionSize * 2;
+  const size_t kItemCount = kMaxSessionSize * 3;
   std::vector<std::unique_ptr<NavigationItem>> items;
   CreateTestNavigationItems(kItemCount, items);
-  GURL restore_session_url =
-      CreateRestoreSessionUrl(/*last_committed_item_index=*/0, items);
+  int first_index = 0;
+  GURL restore_session_url;
+  CreateRestoreSessionUrl(
+      /*last_committed_item_index=*/0, items, &restore_session_url,
+      &first_index);
+  ASSERT_EQ(0, first_index);
   ASSERT_TRUE(IsRestoreSessionUrl(restore_session_url));
 
   // Extract session JSON from restoration URL.
@@ -166,11 +176,15 @@ TEST_F(WKNavigationUtilTest, CreateRestoreSessionUrlForExtraLargeForwardList) {
 TEST_F(WKNavigationUtilTest, CreateRestoreSessionUrlForExtraLargeBackList) {
   // Create restore session URL with large number of items that exceeds
   // kMaxSessionSize.
-  const size_t kItemCount = kMaxSessionSize * 2;
+  const size_t kItemCount = kMaxSessionSize * 3;
   std::vector<std::unique_ptr<NavigationItem>> items;
   CreateTestNavigationItems(kItemCount, items);
-  GURL restore_session_url = CreateRestoreSessionUrl(
-      /*last_committed_item_index=*/kItemCount - 1, items);
+  int first_index = 0;
+  GURL restore_session_url;
+  CreateRestoreSessionUrl(
+      /*last_committed_item_index=*/kItemCount - 1, items, &restore_session_url,
+      &first_index);
+  ASSERT_EQ(150, first_index);
   ASSERT_TRUE(IsRestoreSessionUrl(restore_session_url));
 
   // Extract session JSON from restoration URL.
@@ -185,16 +199,16 @@ TEST_F(WKNavigationUtilTest, CreateRestoreSessionUrlForExtraLargeBackList) {
   ASSERT_TRUE(titles_value->is_list());
   ASSERT_EQ(static_cast<size_t>(kMaxSessionSize),
             titles_value->GetList().size());
-  ASSERT_EQ("Test75", titles_value->GetList()[0].GetString());
-  ASSERT_EQ("Test149",
+  ASSERT_EQ("Test150", titles_value->GetList()[0].GetString());
+  ASSERT_EQ("Test224",
             titles_value->GetList()[kMaxSessionSize - 1].GetString());
 
   base::Value* urls_value = value_with_error.value->FindKey("urls");
   ASSERT_TRUE(urls_value);
   ASSERT_TRUE(urls_value->is_list());
   ASSERT_EQ(static_cast<size_t>(kMaxSessionSize), urls_value->GetList().size());
-  ASSERT_EQ("http:%2F%2Fwww.75.com%2F", urls_value->GetList()[0].GetString());
-  ASSERT_EQ("http:%2F%2Fwww.149.com%2F",
+  ASSERT_EQ("http:%2F%2Fwww.150.com%2F", urls_value->GetList()[0].GetString());
+  ASSERT_EQ("http:%2F%2Fwww.224.com%2F",
             urls_value->GetList()[kMaxSessionSize - 1].GetString());
 
   // Verify the offset is correct.
@@ -210,8 +224,12 @@ TEST_F(WKNavigationUtilTest,
   const size_t kItemCount = kMaxSessionSize * 2;
   std::vector<std::unique_ptr<NavigationItem>> items;
   CreateTestNavigationItems(kItemCount, items);
-  GURL restore_session_url = CreateRestoreSessionUrl(
-      /*last_committed_item_index=*/kMaxSessionSize, items);
+  int first_index = 0;
+  GURL restore_session_url;
+  CreateRestoreSessionUrl(
+      /*last_committed_item_index=*/kMaxSessionSize, items,
+      &restore_session_url, &first_index);
+  ASSERT_EQ(38, first_index);
   ASSERT_TRUE(IsRestoreSessionUrl(restore_session_url));
 
   // Extract session JSON from restoration URL.