[Nav Experiment] Correct navItem title usage.

- Work around pushState changes that overwrite the navigation item
title on pushState.  The root cause of the bug is crbug.com/908173.

- Correctly set navigation item titles on restore.

Bug: 854174, 872211
Change-Id: Ife407429d7f2723fc9dac6458bf81bdb1ae64ff8
Reviewed-on: https://chromium-review.googlesource.com/c/1349776
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Danyao Wang <danyao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611180}

ios/web/navigation/wk_based_navigation_manager_impl.mm

@@ -501,6 +501,10 @@ void WKBasedNavigationManagerImpl::Restore(
   // This pending item will become the first item in the restored history.
   params.virtual_url = items[0]->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();
+
   // Ordering is important. Cache the visible item of the restored session
   // before starting the new navigation, which may trigger client lookup of
   // visible item. The visible item of the restored session is the last
@@ -512,6 +516,13 @@ void WKBasedNavigationManagerImpl::Restore(
     restored_visible_item_ = std::move(items[last_committed_item_index]);
 
   LoadURLWithParams(params);
+
+  // On restore prime the first navigation item with the title.  The remaining
+  // navItem titles will be set from the WKBackForwardListItem title value.
+  NavigationItemImpl* pendingItem = GetPendingItemImpl();
+  if (pendingItem) {
+    pendingItem->SetTitle(firstTitle);
+  }
 }
 
 void WKBasedNavigationManagerImpl::AddRestoreCompletionCallback(
@@ -742,6 +753,7 @@ WKBasedNavigationManagerImpl::WKWebViewCache::GetNavigationItemImplAtIndex(
           nullptr /* use default rewriters only */);
   new_item->SetTimestamp(
       navigation_manager_->time_smoother_.GetSmoothedTime(base::Time::Now()));
+  new_item->SetTitle(base::SysNSStringToUTF16(wk_item.title));
   const GURL& url = new_item->GetURL();
   // If this navigation item has a restore_session.html URL, then it was created
   // to restore session history and will redirect to the target URL encoded in

ios/web/navigation/wk_based_navigation_manager_impl_unittest.mm

@@ -617,9 +617,9 @@ TEST_F(WKBasedNavigationManagerTest, RestoreSessionWithHistory) {
   EXPECT_TRUE(pending_url.SchemeIsFile());
   EXPECT_EQ("restore_session.html", pending_url.ExtractFileName());
   EXPECT_EQ("http://www.0.com/", pending_item->GetVirtualURL());
+  EXPECT_EQ("Test Website 0", base::UTF16ToUTF8(pending_item->GetTitle()));
 
-  EXPECT_EQ(
-      "{\"offset\":0,\"titles\":[\"Test Website 0\",\"\"],"
+  EXPECT_EQ("{\"offset\":0,\"titles\":[\"Test Website 0\",\"\"],"
             "\"urls\":[\"http://www.0.com/\",\"http://www.1.com/\"]}",
             ExtractRestoredSession(pending_url));
 

ios/web/web_state/ui/crw_web_controller.mm

@@ -4947,7 +4947,8 @@ registerLoadRequestForURL:(const GURL&)requestURL
   // for placeholder page because the actual navigation item will not be
   // committed until the native content or WebUI is shown.
   if (context && !context->IsPlaceholderNavigation() &&
-      !context->GetUrl().SchemeIs(url::kAboutScheme)) {
+      !context->GetUrl().SchemeIs(url::kAboutScheme) &&
+      !IsRestoreSessionUrl(context->GetUrl())) {
     [self updateSSLStatusForCurrentNavigationItem];
     [self updateHTML5HistoryState];
     if (!context->IsLoadingErrorPage() && !IsRestoreSessionUrl(webViewURL)) {
@@ -5560,12 +5561,17 @@ registerLoadRequestForURL:(const GURL&)requestURL
                                               hasUserGesture:NO
                                        placeholderNavigation:NO];
 
-      // Use the current title for items created by same document navigations.
+      // With slim nav, the web page title is stored in WKBackForwardListItem
+      // and synced to Navigationitem when the web view title changes.
+      // Otherwise, sync the current title for items created by same document
+      // navigations.
+      if (!web::GetWebClient()->IsSlimNavigationManagerEnabled()) {
         auto* pendingItem = self.navigationManagerImpl->GetPendingItem();
         if (pendingItem)
           pendingItem->SetTitle(_webStateImpl->GetTitle());
       }
     }
+  }
 
   [self setDocumentURL:newURL context:newNavigationContext.get()];
 

ios/web/web_state/web_state_unittest.mm

@@ -418,6 +418,42 @@ TEST_P(WebStateTest, RestoredFromHistory) {
                                                  kTestSessionStoragePageText));
 }
 
+// Verifies that each page title is restored.
+TEST_P(WebStateTest, RestorePageTitles) {
+  // Create session storage.
+  const int kItemCount = 3;
+  NSMutableArray<CRWNavigationItemStorage*>* item_storages =
+      [NSMutableArray arrayWithCapacity:kItemCount];
+  for (unsigned int i = 0; i < kItemCount; i++) {
+    CRWNavigationItemStorage* item = [[CRWNavigationItemStorage alloc] init];
+    item.virtualURL = GURL(base::StringPrintf("http://www.%u.com", i));
+    item.title = base::ASCIIToUTF16(base::StringPrintf("Test%u", i));
+    [item_storages addObject:item];
+  }
+
+  // Restore the session.
+  WebState::CreateParams params(GetBrowserState());
+  CRWSessionStorage* session_storage = [[CRWSessionStorage alloc] init];
+  session_storage.itemStorages = item_storages;
+  auto web_state = WebState::CreateWithStorageSession(params, session_storage);
+  NavigationManager* navigation_manager = web_state->GetNavigationManager();
+  // TODO(crbug.com/873729): The session will not be restored until
+  // LoadIfNecessary call. Fix the bug and remove extra call.
+  navigation_manager->LoadIfNecessary();
+
+  EXPECT_TRUE(WaitUntilConditionOrTimeout(kWaitForPageLoadTimeout, ^{
+    return navigation_manager->GetItemCount() == kItemCount;
+  }));
+
+  for (unsigned int i = 0; i < kItemCount; i++) {
+    NavigationItem* item = navigation_manager->GetItemAtIndex(i);
+    EXPECT_EQ(GURL(base::StringPrintf("http://www.%u.com", i)),
+              item->GetVirtualURL());
+    EXPECT_EQ(base::ASCIIToUTF16(base::StringPrintf("Test%u", i)),
+              item->GetTitle());
+  }
+}
+
 // Tests that NavigationManager::LoadIfNecessary() restores the page after
 // disabling and re-enabling web usage.
 TEST_P(WebStateTest, DisableAndReenableWebUsage) {