[Nav Experiment] Associate NavigationItem with WKBackForwardList.

Added WKBasedNavigationManagerTest for implementation-specific test
cases. The majority of WKBasedNavigationManagerImpl can be tested
using NavigationManagerTest. This will be enabled in a subsequent CL.

Bug: 734150
Change-Id: Ief59f397a72fd5bb1404e94a58486fa3daa87780
Reviewed-on: https://chromium-review.googlesource.com/580628Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: Danyao Wang <danyao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490117}

ios/web/BUILD.gn

@@ -61,6 +61,8 @@ source_set("web_arc") {
     "interstitials/web_interstitial_impl.h",
     "interstitials/web_interstitial_impl.mm",
     "load_committed_details.cc",
+    "navigation/crw_navigation_item_holder.h",
+    "navigation/crw_navigation_item_holder.mm",
     "navigation/crw_session_controller+private_constructors.h",
     "navigation/crw_session_controller.h",
     "navigation/crw_session_controller.mm",
@@ -523,6 +525,7 @@ source_set("ios_web_navigation_unittests") {
   ]
 
   sources = [
+    "navigation/crw_navigation_item_holder_unittest.mm",
     "navigation/crw_navigation_item_storage_unittest.mm",
     "navigation/crw_session_controller_unittest.mm",
     "navigation/crw_session_storage_unittest.mm",
@@ -533,6 +536,7 @@ source_set("ios_web_navigation_unittests") {
     "navigation/navigation_manager_util_unittest.mm",
     "navigation/nscoder_util_unittest.mm",
     "navigation/serializable_user_data_manager_impl_unittest.mm",
+    "navigation/wk_based_navigation_manager_impl_unittest.mm",
   ]
 }
 

ios/web/navigation/crw_navigation_item_holder.h

+// Copyright 2017 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.
+
+#ifndef IOS_WEB_NAVIGATION_CRW_NAVIGATION_ITEM_HOLDER_H_
+#define IOS_WEB_NAVIGATION_CRW_NAVIGATION_ITEM_HOLDER_H_
+
+#include <WebKit/WebKit.h>
+
+#include <memory>
+
+NS_ASSUME_NONNULL_BEGIN
+
+namespace web {
+class NavigationItemImpl;
+}  // namespace web
+
+// An NSObject wrapper for an std::unique_ptr<NavigationItemImpl>. This object
+// is used to associate a NavigationItemImpl to a WKBackForwardListItem to store
+// additional navigation states needed by the embedder.
+@interface CRWNavigationItemHolder : NSObject
+
+// Returns the CRWNavigationItemHolder object associated with |item|. Creates an
+// empty holder if none currently exists for |item|.
++ (instancetype)holderForBackForwardListItem:(WKBackForwardListItem*)item;
+
+// Returns the NavigationItemImpl stored in this instance.
+@property(nullable, nonatomic, readonly)
+    web::NavigationItemImpl* navigationItem;
+
+- (instancetype)init NS_UNAVAILABLE;
+
+// Designated init method that associates the new instance with |item|.
+- (instancetype)initWithBackForwardListItem:(WKBackForwardListItem*)item;
+
+// Stores a NavigationItemImpl object in this instance.
+- (void)setNavigationItem:
+    (std::unique_ptr<web::NavigationItemImpl>)navigationItem;
+
+@end
+
+NS_ASSUME_NONNULL_END
+
+#endif  // IOS_WEB_NAVIGATION_CRW_NAVIGATION_ITEM_HOLDER_H_

ios/web/navigation/crw_navigation_item_holder.mm

+// Copyright 2017 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/navigation/crw_navigation_item_holder.h"
+
+#import <objc/runtime.h>
+
+#import "ios/web/navigation/navigation_item_impl.h"
+
+// The address of this static variable is used to set and get the associated
+// NavigationItemImpl object from a WKBackForwardListItem.
+const void* kNavigationItemKey = &kNavigationItemKey;
+
+@implementation CRWNavigationItemHolder {
+  std::unique_ptr<web::NavigationItemImpl> _navigationItem;
+}
+
++ (instancetype)holderForBackForwardListItem:(WKBackForwardListItem*)item {
+  DCHECK(item);
+  CRWNavigationItemHolder* holder =
+      objc_getAssociatedObject(item, &kNavigationItemKey);
+  if (!holder) {
+    holder = [[CRWNavigationItemHolder alloc] initWithBackForwardListItem:item];
+  }
+  return holder;
+}
+
+- (instancetype)initWithBackForwardListItem:(WKBackForwardListItem*)item {
+  self = [super init];
+  if (self) {
+    objc_setAssociatedObject(item, &kNavigationItemKey, self,
+                             OBJC_ASSOCIATION_RETAIN);
+  }
+  return self;
+}
+
+- (web::NavigationItemImpl*)navigationItem {
+  return _navigationItem.get();
+}
+
+- (void)setNavigationItem:
+    (std::unique_ptr<web::NavigationItemImpl>)navigationItem {
+  _navigationItem = std::move(navigationItem);
+}
+
+@end

ios/web/navigation/crw_navigation_item_holder_unittest.mm

+// Copyright 2017 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/navigation/crw_navigation_item_holder.h"
+
+#include "base/memory/ptr_util.h"
+#import "ios/web/navigation/navigation_item_impl.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#import "testing/gtest_mac.h"
+#include "testing/platform_test.h"
+#include "third_party/ocmock/OCMock/OCMock.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+// Test fixture for CRWNavigationItemHolder.
+typedef PlatformTest CRWNavigationItemHolderTest;
+
+TEST_F(CRWNavigationItemHolderTest, NewHolderCreatedAutomatically) {
+  WKBackForwardListItem* item = OCMClassMock([WKBackForwardListItem class]);
+  CRWNavigationItemHolder* holder =
+      [CRWNavigationItemHolder holderForBackForwardListItem:item];
+  ASSERT_NSNE(holder, nil);
+  EXPECT_TRUE(holder.navigationItem == nullptr);
+}
+
+// Tests that stored NavigationItemImpl can be retrieved.
+TEST_F(CRWNavigationItemHolderTest, SetNavigationItem) {
+  GURL url("http://www.0.com");
+  auto navigation_item = base::MakeUnique<web::NavigationItemImpl>();
+  navigation_item->SetURL(url);
+
+  WKBackForwardListItem* item = OCMClassMock([WKBackForwardListItem class]);
+  [[CRWNavigationItemHolder holderForBackForwardListItem:item]
+      setNavigationItem:std::move(navigation_item)];
+
+  CRWNavigationItemHolder* holder =
+      [CRWNavigationItemHolder holderForBackForwardListItem:item];
+
+  ASSERT_NSNE(holder, nil);
+  ASSERT_TRUE(holder.navigationItem != nullptr);
+  EXPECT_EQ(url, holder.navigationItem->GetURL());
+}
+
+// Tests that each WKBackForwardListItem has its unique CRWNavigationItemHolder.
+TEST_F(CRWNavigationItemHolderTest, OneHolderPerWKItem) {
+  GURL url1("http://www.1.com");
+  auto navigation_item1 = base::MakeUnique<web::NavigationItemImpl>();
+  navigation_item1->SetURL(url1);
+  WKBackForwardListItem* item1 = OCMClassMock([WKBackForwardListItem class]);
+  [[CRWNavigationItemHolder holderForBackForwardListItem:item1]
+      setNavigationItem:std::move(navigation_item1)];
+
+  GURL url2("http://www.2.com");
+  auto navigation_item2 = base::MakeUnique<web::NavigationItemImpl>();
+  navigation_item2->SetURL(url2);
+  WKBackForwardListItem* item2 = OCMClassMock([WKBackForwardListItem class]);
+  [[CRWNavigationItemHolder holderForBackForwardListItem:item2]
+      setNavigationItem:std::move(navigation_item2)];
+
+  ASSERT_NSNE(item1, item2);
+
+  CRWNavigationItemHolder* holder1 =
+      [CRWNavigationItemHolder holderForBackForwardListItem:item1];
+  CRWNavigationItemHolder* holder2 =
+      [CRWNavigationItemHolder holderForBackForwardListItem:item2];
+
+  EXPECT_NSNE(holder1, holder2);
+  ASSERT_TRUE(holder1.navigationItem);
+  EXPECT_EQ(url1, holder1.navigationItem->GetURL());
+  ASSERT_TRUE(holder2.navigationItem);
+  EXPECT_EQ(url2, holder2.navigationItem->GetURL());
+}

ios/web/navigation/legacy_navigation_manager_impl.mm

@@ -75,7 +75,7 @@ void LegacyNavigationManagerImpl::OnNavigationItemCommitted() {
   if (details.previous_item_index >= 0) {
     DCHECK([session_controller_ previousItem]);
     details.previous_url = [session_controller_ previousItem]->GetURL();
-    details.is_in_page = AreUrlsFragmentChangeNavigation(
+    details.is_in_page = IsFragmentChangeNavigationBetweenUrls(
         details.previous_url, details.item->GetURL());
   } else {
     details.previous_url = GURL();
@@ -115,46 +115,13 @@ void LegacyNavigationManagerImpl::AddPendingItem(
                        initiationType:initiation_type
               userAgentOverrideOption:user_agent_override_option];
 
-  // Set the user agent type for web URLs.
-  NavigationItem* pending_item = GetPendingItem();
-  if (!pending_item)
+  if (!GetPendingItem()) {
     return;
-
-  // |user_agent_override_option| must be INHERIT if |pending_item|'s
-  // UserAgentType is NONE, as requesting a desktop or mobile user agent should
-  // be disabled for app-specific URLs.
-  DCHECK(pending_item->GetUserAgentType() != UserAgentType::NONE ||
-         user_agent_override_option == UserAgentOverrideOption::INHERIT);
-
-  // Newly created pending items are created with UserAgentType::NONE for native
-  // pages or UserAgentType::MOBILE for non-native pages.  If the pending item's
-  // URL is non-native, check which user agent type it should be created with
-  // based on |user_agent_override_option|.
-  DCHECK_NE(UserAgentType::DESKTOP, pending_item->GetUserAgentType());
-  if (pending_item->GetUserAgentType() == UserAgentType::NONE)
-    return;
-
-  switch (user_agent_override_option) {
-    case UserAgentOverrideOption::DESKTOP:
-      pending_item->SetUserAgentType(UserAgentType::DESKTOP);
-      break;
-    case UserAgentOverrideOption::MOBILE:
-      pending_item->SetUserAgentType(UserAgentType::MOBILE);
-      break;
-    case UserAgentOverrideOption::INHERIT: {
-      // Propagate the last committed non-native item's UserAgentType if there
-      // is one, otherwise keep the default value, which is mobile.
-      NavigationItem* last_non_native_item =
-          GetLastCommittedNonAppSpecificItem();
-      DCHECK(!last_non_native_item ||
-             last_non_native_item->GetUserAgentType() != UserAgentType::NONE);
-      if (last_non_native_item) {
-        pending_item->SetUserAgentType(
-            last_non_native_item->GetUserAgentType());
-      }
-      break;
-    }
   }
+
+  UpdatePendingItemUserAgentType(user_agent_override_option,
+                                 GetLastCommittedNonAppSpecificItem(),
+                                 GetPendingItem());
 }
 
 void LegacyNavigationManagerImpl::CommitPendingItem() {

ios/web/navigation/navigation_manager_impl.h

@@ -143,17 +143,23 @@ class NavigationManagerImpl : public NavigationManager {
   // TODO(crbug.com/738020): Remove legacy code and merge
   // WKBasedNavigationManager into this class after the navigation experiment.
 
+  // Checks whether or not two URLs differ only in the fragment.
+  static bool IsFragmentChangeNavigationBetweenUrls(const GURL& existing_url,
+                                                    const GURL& new_url);
+
+  // Applies the user agent override to |pending_item|, or inherits the user
+  // agent of |inherit_from| if |user_agent_override_option| is INHERIT.
+  static void UpdatePendingItemUserAgentType(
+      UserAgentOverrideOption override_option,
+      const NavigationItem* inherit_from,
+      NavigationItem* pending_item);
+
   // Identical to GetItemAtIndex() but returns the underlying NavigationItemImpl
   // instead of the public NavigationItem interface. This is used by
   // SessionStorageBuilder to persist session state.
   virtual NavigationItemImpl* GetNavigationItemImplAtIndex(
       size_t index) const = 0;
 
-  // Checks whether or not two URL are an in-page navigation (differing only
-  // in the fragment).
-  static bool AreUrlsFragmentChangeNavigation(const GURL& existing_url,
-                                              const GURL& new_url);
-
   // The primary delegate for this manager.
   NavigationManagerDelegate* delegate_;
 

ios/web/navigation/navigation_manager_impl.mm

@@ -47,15 +47,58 @@ NavigationManager::WebLoadParams& NavigationManager::WebLoadParams::operator=(
 }
 
 /* static */
-bool NavigationManagerImpl::AreUrlsFragmentChangeNavigation(
+bool NavigationManagerImpl::IsFragmentChangeNavigationBetweenUrls(
     const GURL& existing_url,
     const GURL& new_url) {
+  // TODO(crbug.com/749542): Current implementation incorrectly returns false
+  // if URL changes from http://google.com#foo to http://google.com.
   if (existing_url == new_url || !new_url.has_ref())
     return false;
 
   return existing_url.EqualsIgnoringRef(new_url);
 }
 
+/* static */
+void NavigationManagerImpl::UpdatePendingItemUserAgentType(
+    UserAgentOverrideOption user_agent_override_option,
+    const NavigationItem* inherit_from_item,
+    NavigationItem* pending_item) {
+  DCHECK(pending_item);
+
+  // |user_agent_override_option| must be INHERIT if |pending_item|'s
+  // UserAgentType is NONE, as requesting a desktop or mobile user agent should
+  // be disabled for app-specific URLs.
+  DCHECK(pending_item->GetUserAgentType() != UserAgentType::NONE ||
+         user_agent_override_option == UserAgentOverrideOption::INHERIT);
+
+  // Newly created pending items are created with UserAgentType::NONE for native
+  // pages or UserAgentType::MOBILE for non-native pages.  If the pending item's
+  // URL is non-native, check which user agent type it should be created with
+  // based on |user_agent_override_option|.
+  DCHECK_NE(UserAgentType::DESKTOP, pending_item->GetUserAgentType());
+  if (pending_item->GetUserAgentType() == UserAgentType::NONE)
+    return;
+
+  switch (user_agent_override_option) {
+    case UserAgentOverrideOption::DESKTOP:
+      pending_item->SetUserAgentType(UserAgentType::DESKTOP);
+      break;
+    case UserAgentOverrideOption::MOBILE:
+      pending_item->SetUserAgentType(UserAgentType::MOBILE);
+      break;
+    case UserAgentOverrideOption::INHERIT: {
+      // Propagate the last committed non-native item's UserAgentType if there
+      // is one, otherwise keep the default value, which is mobile.
+      DCHECK(!inherit_from_item ||
+             inherit_from_item->GetUserAgentType() != UserAgentType::NONE);
+      if (inherit_from_item) {
+        pending_item->SetUserAgentType(inherit_from_item->GetUserAgentType());
+      }
+      break;
+    }
+  }
+}
+
 NavigationManagerImpl::NavigationManagerImpl()
     : delegate_(nullptr), browser_state_(nullptr) {}
 

ios/web/navigation/wk_based_navigation_manager_impl.h

@@ -14,10 +14,13 @@
 #include "base/macros.h"
 #import "ios/web/navigation/navigation_item_impl.h"
 #import "ios/web/navigation/navigation_manager_impl.h"
+#include "ios/web/navigation/time_smoother.h"
 #include "ios/web/public/reload_type.h"
 #include "ui/base/page_transition_types.h"
 #include "url/gurl.h"
 
+@class WKBackForwardListItem;
+
 namespace web {
 class BrowserState;
 class NavigationItem;
@@ -27,12 +30,28 @@ class SessionStorageBuilder;
 // WKBackForwardList based implementation of NavigationManagerImpl.
 // This class relies on the following WKWebView APIs, defined by the
 // CRWWebViewNavigationProxy protocol:
+//   @property URL
 //   @property backForwardList
 //   @property canGoBack
 //   @property canGoForward
 //   - goBack
 //   - goForward
 //   - goToBackForwardListItem:
+//
+// This navigation manager uses WKBackForwardList as the ground truth for back-
+// forward navigation history. It uses the Associated Objects runtime feature
+// to link a NavigationItemImpl object to each WKBackForwardListItem to store
+// additional states needed by the embedder.
+//
+// For all main frame navigations (both UI-initiated and renderer-initiated),
+// the NavigationItemImpl objects are created proactively via AddPendingItem and
+// CommitPendingItem.
+//
+// Non-main-frame navigations can only be initiated from the renderer. The
+// NavigationItemImpl objects in this case are created lazily in GetItemAtIndex
+// because the provisional load and commit events for iframe navigation are not
+// visible via the WKNavigationDelegate interface. Consequently, pending item
+// and previous item are only tracked for the main frame.
 class WKBasedNavigationManagerImpl : public NavigationManagerImpl {
  public:
   WKBasedNavigationManagerImpl();
@@ -56,6 +75,7 @@ class WKBasedNavigationManagerImpl : public NavigationManagerImpl {
       UserAgentOverrideOption user_agent_override_option) override;
   void CommitPendingItem() override;
   int GetIndexForOffset(int offset) const override;
+  // Returns the previous navigation item in the main frame.
   int GetPreviousItemIndex() const override;
 
   // NavigationManager:
@@ -63,6 +83,7 @@ class WKBasedNavigationManagerImpl : public NavigationManagerImpl {
   WebState* GetWebState() const override;
   NavigationItem* GetVisibleItem() const override;
   NavigationItem* GetLastCommittedItem() const override;
+  // Returns the pending navigation item in the main frame.
   NavigationItem* GetPendingItem() const override;
   NavigationItem* GetTransientItem() const override;
   void DiscardNonCommittedItems() override;
@@ -92,9 +113,40 @@ class WKBasedNavigationManagerImpl : public NavigationManagerImpl {
   // NavigationManagerImpl methods used by SessionStorageBuilder.
   NavigationItemImpl* GetNavigationItemImplAtIndex(size_t index) const override;
 
+  // Returns the absolute index of WKBackForwardList's |currentItem|. Returns -1
+  // if |currentItem| is nil.
+  int GetWKCurrentItemIndex() const;
+
+  // Returns the WKNavigationItem object at the absolute index, where index = 0
+  // corresponds to the oldest entry in the back-forward history, and
+  // index = GetItemCount() - 1 corresponds to the newest entry in the back-
+  // forward history. Returns nil if |index| is outside [0, GetItemCount()).
+  WKBackForwardListItem* GetWKItemAtIndex(int index) const;
+
+  // The pending main frame navigation item. This is nullptr if there is no
+  // pending item or if the pending item is a back-forward navigation, in which
+  // case the NavigationItemImpl is stored on the WKBackForwardListItem.
+  std::unique_ptr<NavigationItemImpl> pending_item_;
+
+  // -1 if pending_item_ represents a new navigation or there is no pending
+  // navigation. Otherwise, this is the index of the pending_item in the
+  // back-forward list.
+  int pending_item_index_;
+
+  // Index of the previous navigation item in the main frame. If there is none,
+  // this field will have value -1.
+  int previous_item_index_;
+
+  // Index of the last committed item in the main frame. If there is none, this
+  // field will equal to -1.
+  int last_committed_item_index_;
 
+  // The transient item in main frame.
+  std::unique_ptr<NavigationItemImpl> transient_item_;
 
-  // List of transient url rewriters added by |AddTransientURLRewriter()|.
+  // Time smoother for navigation item timestamps. See comment in
+  // navigation_controller_impl.h.
+  TimeSmoother time_smoother_;
 
   DISALLOW_COPY_AND_ASSIGN(WKBasedNavigationManagerImpl);
 };

ios/web/navigation/wk_based_navigation_manager_impl.mm

@@ -4,10 +4,12 @@
 
 #import "ios/web/navigation/wk_based_navigation_manager_impl.h"
 
+#import <Foundation/Foundation.h>
 #include <memory>
 
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
+#import "ios/web/navigation/crw_navigation_item_holder.h"
 #import "ios/web/navigation/navigation_item_impl.h"
 #import "ios/web/navigation/navigation_manager_delegate.h"
 #include "ios/web/public/load_committed_details.h"
@@ -22,9 +24,31 @@
 
 @class CRWSessionController;
 
+namespace {
+
+void SetNavigationItemInWKItem(WKBackForwardListItem* wk_item,
+                               std::unique_ptr<web::NavigationItemImpl> item) {
+  DCHECK(wk_item);
+  [[CRWNavigationItemHolder holderForBackForwardListItem:wk_item]
+      setNavigationItem:std::move(item)];
+}
+
+web::NavigationItemImpl* GetNavigationItemFromWKItem(
+    WKBackForwardListItem* wk_item) {
+  return wk_item
+             ? [CRWNavigationItemHolder holderForBackForwardListItem:wk_item]
+                   .navigationItem
+             : nullptr;
+}
+
+}  // namespace
+
 namespace web {
 
-WKBasedNavigationManagerImpl::WKBasedNavigationManagerImpl() = default;
+WKBasedNavigationManagerImpl::WKBasedNavigationManagerImpl()
+    : pending_item_index_(-1),
+      previous_item_index_(-1),
+      last_committed_item_index_(-1) {}
 
 WKBasedNavigationManagerImpl::~WKBasedNavigationManagerImpl() = default;
 
@@ -57,7 +81,7 @@ void WKBasedNavigationManagerImpl::OnNavigationItemCommitted() {
     NavigationItem* previous_item = GetItemAtIndex(details.previous_item_index);
     DCHECK(previous_item);
     details.previous_url = previous_item->GetURL();
-    details.is_in_page = AreUrlsFragmentChangeNavigation(
+    details.is_in_page = IsFragmentChangeNavigationBetweenUrls(
         details.previous_url, details.item->GetURL());
   } else {
     details.previous_url = GURL();
@@ -73,7 +97,16 @@ CRWSessionController* WKBasedNavigationManagerImpl::GetSessionController()
 }
 
 void WKBasedNavigationManagerImpl::AddTransientItem(const GURL& url) {
-  DLOG(WARNING) << "Not yet implemented.";
+  transient_item_ =
+      CreateNavigationItem(url, Referrer(), ui::PAGE_TRANSITION_CLIENT_REDIRECT,
+                           NavigationInitiationType::USER_INITIATED);
+  transient_item_->SetTimestamp(
+      time_smoother_.GetSmoothedTime(base::Time::Now()));
+
+  // Transient item is only supposed to be added for pending non-app-specific
+  // navigations.
+  DCHECK(pending_item_->GetUserAgentType() != UserAgentType::NONE);
+  transient_item_->SetUserAgentType(pending_item_->GetUserAgentType());
 }
 
 void WKBasedNavigationManagerImpl::AddPendingItem(
@@ -82,16 +115,88 @@ void WKBasedNavigationManagerImpl::AddPendingItem(
     ui::PageTransition navigation_type,
     NavigationInitiationType initiation_type,
     UserAgentOverrideOption user_agent_override_option) {
-  DLOG(WARNING) << "Not yet implemented.";
+  DiscardNonCommittedItems();
+
+  pending_item_ =
+      CreateNavigationItem(url, referrer, navigation_type, initiation_type);
+
+  // WKBasedNavigationManagerImpl does not track
+  // native URLs yet so just inherit from the
+  // last committed item.
+  // TODO(crbug.com/734150): Change GetLastCommittedItem() to
+  // GetLastCommittedNonAppSpecificItem() after
+  // integrating with native URLs.
+  UpdatePendingItemUserAgentType(user_agent_override_option,
+                                 GetLastCommittedItem(), pending_item_.get());
+
+  // AddPendingItem is called no later than |didCommitNavigation|. The only time
+  // when all three of WKWebView's URL, the pending URL and WKBackForwardList's
+  // current item URL are identical before |didCommitNavigation| is when the
+  // in-progress navigation is a back-forward navigation. In this case, current
+  // item has already been updated to point to the new location in back-forward
+  // history, so pending item index should be set to the current item index.
+  id<CRWWebViewNavigationProxy> proxy = delegate_->GetWebViewNavigationProxy();
+  WKBackForwardListItem* current_wk_item = proxy.backForwardList.currentItem;
+  GURL current_item_url = net::GURLWithNSURL(current_wk_item.URL);
+  if (proxy.backForwardList.currentItem &&
+      current_item_url == net::GURLWithNSURL(proxy.URL) &&
+      current_item_url == pending_item_->GetURL()) {
+    pending_item_index_ = GetWKCurrentItemIndex();
+
+    // If |currentItem| is not already associated with a NavigationItemImpl,
+    // associate the newly created item with it. Otherwise, discard the new item
+    // since it will be a duplicate.
+    NavigationItemImpl* current_item =
+        GetNavigationItemFromWKItem(current_wk_item);
+    if (!current_item) {
+      SetNavigationItemInWKItem(current_wk_item, std::move(pending_item_));
+    }
+    pending_item_.reset();
+  } else {
+    pending_item_index_ = -1;
+  }
 }
 
 void WKBasedNavigationManagerImpl::CommitPendingItem() {
-  DLOG(WARNING) << "Not yet implemented.";
+  if (pending_item_index_ == -1) {
+    pending_item_->ResetForCommit();
+    pending_item_->SetTimestamp(
+        time_smoother_.GetSmoothedTime(base::Time::Now()));
+
+    // WKBackForwardList's |currentItem| would have already been updated when
+    // this method is called and it is the last committed item.
+    id<CRWWebViewNavigationProxy> proxy =
+        delegate_->GetWebViewNavigationProxy();
+    SetNavigationItemInWKItem(proxy.backForwardList.currentItem,
+                              std::move(pending_item_));
+  }
+
+  pending_item_index_ = -1;
+  previous_item_index_ = last_committed_item_index_;
+  last_committed_item_index_ = GetWKCurrentItemIndex();
+
+  OnNavigationItemCommitted();
 }
 
 int WKBasedNavigationManagerImpl::GetIndexForOffset(int offset) const {
-  DLOG(WARNING) << "Not yet implemented.";
-  return -1;
+  int result = (pending_item_index_ == -1) ? GetLastCommittedItemIndex()
+                                           : pending_item_index_;
+
+  if (offset < 0 && GetTransientItem() && pending_item_index_ == -1) {
+    // Going back from transient item that added to the end navigation stack
+    // is a matter of discarding it as there is no need to move navigation
+    // index back.
+    offset++;
+  }
+  result += offset;
+  if (result > GetItemCount() /* overflow */) {
+    result = INT_MIN;
+  }
+  return result;
+}
+
+int WKBasedNavigationManagerImpl::GetPreviousItemIndex() const {
+  return previous_item_index_;
 }
 
 BrowserState* WKBasedNavigationManagerImpl::GetBrowserState() const {
@@ -108,22 +213,22 @@ NavigationItem* WKBasedNavigationManagerImpl::GetVisibleItem() const {
 }
 
 NavigationItem* WKBasedNavigationManagerImpl::GetLastCommittedItem() const {
-  DLOG(WARNING) << "Not yet implemented.";
-  return nullptr;
+  int index = GetLastCommittedItemIndex();
+  return index == -1 ? nullptr : GetItemAtIndex(static_cast<size_t>(index));
 }
 
 NavigationItem* WKBasedNavigationManagerImpl::GetPendingItem() const {
-  DLOG(WARNING) << "Not yet implemented.";
-  return nullptr;
+  return (pending_item_index_ == -1) ? pending_item_.get()
+                                     : GetItemAtIndex(pending_item_index_);
 }
 
 NavigationItem* WKBasedNavigationManagerImpl::GetTransientItem() const {
-  DLOG(WARNING) << "Not yet implemented.";
-  return nullptr;
+  return transient_item_.get();
 }
 
 void WKBasedNavigationManagerImpl::DiscardNonCommittedItems() {
-  DLOG(WARNING) << "Not yet implemented.";
+  pending_item_.reset();
+  transient_item_.reset();
 }
 
 void WKBasedNavigationManagerImpl::LoadURLWithParams(
@@ -147,8 +252,7 @@ int WKBasedNavigationManagerImpl::GetItemCount() const {
 
 NavigationItem* WKBasedNavigationManagerImpl::GetItemAtIndex(
     size_t index) const {
-  DLOG(WARNING) << "Not yet implemented.";
-  return nullptr;
+  return GetNavigationItemImplAtIndex(index);
 }
 
 int WKBasedNavigationManagerImpl::GetIndexOfItem(
@@ -158,16 +262,19 @@ int WKBasedNavigationManagerImpl::GetIndexOfItem(
 }
 
 int WKBasedNavigationManagerImpl::GetPendingItemIndex() const {
-  DLOG(WARNING) << "Not yet implemented.";
+  if (GetPendingItem()) {
+    if (pending_item_index_ != -1) {
+      return pending_item_index_;
+    }
+    // TODO(crbug.com/665189): understand why last committed item index is
+    // returned here.
+    return GetLastCommittedItemIndex();
+  }
   return -1;
 }
 
 int WKBasedNavigationManagerImpl::GetLastCommittedItemIndex() const {
-  id<CRWWebViewNavigationProxy> proxy = delegate_->GetWebViewNavigationProxy();
-  if (proxy.backForwardList.currentItem) {
-    return static_cast<int>(proxy.backForwardList.backList.count);
-  }
-  return -1;
+  return GetWKCurrentItemIndex();
 }
 
 bool WKBasedNavigationManagerImpl::RemoveItemAtIndex(int index) {
@@ -176,11 +283,11 @@ bool WKBasedNavigationManagerImpl::RemoveItemAtIndex(int index) {
 }
 
 bool WKBasedNavigationManagerImpl::CanGoBack() const {
-  return [delegate_->GetWebViewNavigationProxy() canGoBack];
+  return CanGoToOffset(-1);
 }
 
 bool WKBasedNavigationManagerImpl::CanGoForward() const {
-  return [delegate_->GetWebViewNavigationProxy() canGoForward];
+  return CanGoToOffset(1);
 }
 
 bool WKBasedNavigationManagerImpl::CanGoToOffset(int offset) const {
@@ -189,10 +296,15 @@ bool WKBasedNavigationManagerImpl::CanGoToOffset(int offset) const {
 }
 
 void WKBasedNavigationManagerImpl::GoBack() {
+  if (transient_item_) {
+    transient_item_.reset();
+    return;
+  }
   [delegate_->GetWebViewNavigationProxy() goBack];
 }
 
 void WKBasedNavigationManagerImpl::GoForward() {
+  DiscardNonCommittedItems();
   [delegate_->GetWebViewNavigationProxy() goForward];
 }
 
@@ -220,16 +332,54 @@ bool WKBasedNavigationManagerImpl::CanPruneAllButLastCommittedItem() const {
   return true;
 }
 
-
 NavigationItemImpl* WKBasedNavigationManagerImpl::GetNavigationItemImplAtIndex(
     size_t index) const {
-  DLOG(WARNING) << "Not yet implemented.";
-  return nullptr;
-}
+  WKBackForwardListItem* wk_item = GetWKItemAtIndex(index);
+  NavigationItemImpl* item = GetNavigationItemFromWKItem(wk_item);
 
-int WKBasedNavigationManagerImpl::GetPreviousItemIndex() const {
-  DLOG(WARNING) << "Not yet implemented.";
+  if (!wk_item || item) {
+    return item;
+  }
+
+  // TODO(crbug.com/734150): Add a stat counter to track rebuilding frequency.
+  WKBackForwardListItem* prev_wk_item =
+      index == 0 ? nil : GetWKItemAtIndex(index - 1);
+  // TODO(crbug.com/734150): CreateNavigationItem is not const because it clears
+  // transient rewriters. Perhaps transient rewriters should not be used for
+  // lazily created items. Investigate and make a decision before this code goes
+  // live.
+  std::unique_ptr<web::NavigationItemImpl> new_item =
+      const_cast<WKBasedNavigationManagerImpl*>(this)->CreateNavigationItem(
+          net::GURLWithNSURL(wk_item.URL),
+          (prev_wk_item ? web::Referrer(net::GURLWithNSURL(prev_wk_item.URL),
+                                        web::ReferrerPolicyAlways)
+                        : web::Referrer()),
+          ui::PageTransition::PAGE_TRANSITION_LINK,
+          NavigationInitiationType::RENDERER_INITIATED);
+  SetNavigationItemInWKItem(wk_item, std::move(new_item));
+  return GetNavigationItemFromWKItem(wk_item);
+}
+
+int WKBasedNavigationManagerImpl::GetWKCurrentItemIndex() const {
+  id<CRWWebViewNavigationProxy> proxy = delegate_->GetWebViewNavigationProxy();
+  if (proxy.backForwardList.currentItem) {
+    return static_cast<int>(proxy.backForwardList.backList.count);
+  }
   return -1;
 }
 
+WKBackForwardListItem* WKBasedNavigationManagerImpl::GetWKItemAtIndex(
+    int index) const {
+  if (index < 0 || index >= GetItemCount()) {
+    return nil;
+  }
+
+  // Convert the index to an offset relative to backForwardList.currentItem (
+  // which is also the last committed item), then use WKBackForwardList API to
+  // retrieve the item.
+  int offset = index - GetWKCurrentItemIndex();
+  id<CRWWebViewNavigationProxy> proxy = delegate_->GetWebViewNavigationProxy();
+  return [proxy.backForwardList itemAtIndex:offset];
+}
+
 }  // namespace web

ios/web/navigation/wk_based_navigation_manager_impl_unittest.mm

+// Copyright 2017 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/navigation/wk_based_navigation_manager_impl.h"
+
+#include <WebKit/WebKit.h>
+#include <memory>
+
+#import "ios/web/navigation/navigation_manager_impl.h"
+#include "ios/web/public/navigation_item.h"
+#include "ios/web/public/test/fakes/test_browser_state.h"
+#import "ios/web/test/fakes/crw_test_back_forward_list.h"
+#include "ios/web/test/fakes/test_navigation_manager_delegate.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "testing/platform_test.h"
+#include "third_party/ocmock/OCMock/OCMock.h"
+#include "ui/base/page_transition_types.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+namespace web {
+
+// Test fixture for WKBasedNavigationManagerImpl.
+class WKBasedNavigationManagerTest : public PlatformTest {
+ protected:
+  WKBasedNavigationManagerTest() : manager_(new WKBasedNavigationManagerImpl) {
+    mock_web_view_ = OCMClassMock([WKWebView class]);
+    mock_wk_list_ = [[CRWTestBackForwardList alloc] init];
+    OCMStub([mock_web_view_ backForwardList]).andReturn(mock_wk_list_);
+    delegate_.SetWebViewNavigationProxy(mock_web_view_);
+
+    manager_->SetDelegate(&delegate_);
+    manager_->SetBrowserState(&browser_state_);
+  }
+
+ protected:
+  std::unique_ptr<NavigationManagerImpl> manager_;
+  CRWTestBackForwardList* mock_wk_list_;
+  id mock_web_view_;
+
+ private:
+  TestBrowserState browser_state_;
+  TestNavigationManagerDelegate delegate_;
+};
+
+// Tests that GetItemAtIndex() on an empty manager will sync navigation items to
+// WKBackForwardList using default properties.
+TEST_F(WKBasedNavigationManagerTest, SyncAfterItemAtIndex) {
+  EXPECT_EQ(0, manager_->GetItemCount());
+  EXPECT_EQ(nullptr, manager_->GetItemAtIndex(0));
+
+  [mock_wk_list_ setCurrentURL:@"http://www.0.com"];
+  EXPECT_EQ(1, manager_->GetItemCount());
+  EXPECT_EQ(0, manager_->GetLastCommittedItemIndex());
+
+  NavigationItem* item = manager_->GetItemAtIndex(0);
+  ASSERT_NE(item, nullptr);
+  EXPECT_EQ(GURL("http://www.0.com"), item->GetURL());
+  EXPECT_TRUE(ui::PageTransitionCoreTypeIs(ui::PAGE_TRANSITION_LINK,
+                                           item->GetTransitionType()));
+  EXPECT_EQ(UserAgentType::MOBILE, item->GetUserAgentType());
+}
+
+// Tests that Referrer is inferred from the previous WKBackForwardListItem.
+TEST_F(WKBasedNavigationManagerTest, SyncAfterItemAtIndexWithPreviousItem) {
+  [mock_wk_list_
+        setCurrentURL:@"http://www.1.com"
+         backListURLs:[NSArray arrayWithObjects:@"http://www.0.com", nil]
+      forwardListURLs:[NSArray arrayWithObjects:@"http://www.2.com", nil]];
+  EXPECT_EQ(3, manager_->GetItemCount());
+  EXPECT_EQ(1, manager_->GetLastCommittedItemIndex());
+
+  // The out-of-order access is intentionall to test that syncing doesn't rely
+  // on the previous WKBackForwardListItem having an associated NavigationItem.
+  NavigationItem* item2 = manager_->GetItemAtIndex(2);
+  ASSERT_NE(item2, nullptr);
+  EXPECT_EQ(GURL("http://www.2.com"), item2->GetURL());
+  EXPECT_TRUE(ui::PageTransitionCoreTypeIs(ui::PAGE_TRANSITION_LINK,
+                                           item2->GetTransitionType()));
+  EXPECT_EQ(UserAgentType::MOBILE, item2->GetUserAgentType());
+  EXPECT_EQ(GURL("http://www.1.com"), item2->GetReferrer().url);
+
+  NavigationItem* item1 = manager_->GetItemAtIndex(1);
+  ASSERT_NE(item1, nullptr);
+  EXPECT_EQ(GURL("http://www.1.com"), item1->GetURL());
+  EXPECT_TRUE(ui::PageTransitionCoreTypeIs(ui::PAGE_TRANSITION_LINK,
+                                           item1->GetTransitionType()));
+  EXPECT_EQ(UserAgentType::MOBILE, item1->GetUserAgentType());
+  EXPECT_EQ(GURL("http://www.0.com"), item1->GetReferrer().url);
+
+  NavigationItem* item0 = manager_->GetItemAtIndex(0);
+  ASSERT_NE(item0, nullptr);
+  EXPECT_EQ(GURL("http://www.0.com"), item0->GetURL());
+  EXPECT_TRUE(ui::PageTransitionCoreTypeIs(ui::PAGE_TRANSITION_LINK,
+                                           item0->GetTransitionType()));
+  EXPECT_EQ(UserAgentType::MOBILE, item0->GetUserAgentType());
+}
+
+// Tests that CommitPendingItem() will sync navigation items to
+// WKBackForwardList and the pending item NavigationItemImpl will be used.
+TEST_F(WKBasedNavigationManagerTest, GetItemAtIndexAfterCommitPending) {
+  // Simulate a main frame navigation.
+  manager_->AddPendingItem(
+      GURL("http://www.0.com"), Referrer(), ui::PAGE_TRANSITION_TYPED,
+      web::NavigationInitiationType::USER_INITIATED,
+      web::NavigationManager::UserAgentOverrideOption::DESKTOP);
+  NavigationItem* pending_item0 = manager_->GetPendingItem();
+
+  [mock_wk_list_ setCurrentURL:@"http://www.0.com"];
+  manager_->CommitPendingItem();
+
+  EXPECT_EQ(1, manager_->GetItemCount());
+  NavigationItem* item = manager_->GetLastCommittedItem();
+  EXPECT_EQ(pending_item0, item);
+  EXPECT_EQ(GURL("http://www.0.com"), item->GetURL());
+  EXPECT_TRUE(ui::PageTransitionCoreTypeIs(ui::PAGE_TRANSITION_TYPED,
+                                           item->GetTransitionType()));
+  EXPECT_EQ(UserAgentType::DESKTOP, item->GetUserAgentType());
+
+  // Simulate a second main frame navigation.
+  manager_->AddPendingItem(
+      GURL("http://www.2.com"), Referrer(), ui::PAGE_TRANSITION_TYPED,
+      web::NavigationInitiationType::USER_INITIATED,
+      web::NavigationManager::UserAgentOverrideOption::DESKTOP);
+  NavigationItem* pending_item2 = manager_->GetPendingItem();
+
+  // Simulate an iframe navigation between the two main frame navigations.
+  [mock_wk_list_
+        setCurrentURL:@"http://www.2.com"
+         backListURLs:[NSArray arrayWithObjects:@"http://www.0.com",
+                                                @"http://www.1.com", nil]
+      forwardListURLs:nil];
+  manager_->CommitPendingItem();
+
+  EXPECT_EQ(3, manager_->GetItemCount());
+  EXPECT_EQ(2, manager_->GetLastCommittedItemIndex());
+
+  // This item is created by syncing.
+  NavigationItem* item1 = manager_->GetItemAtIndex(1);
+  EXPECT_EQ(GURL("http://www.1.com"), item1->GetURL());
+  EXPECT_TRUE(ui::PageTransitionCoreTypeIs(ui::PAGE_TRANSITION_LINK,
+                                           item1->GetTransitionType()));
+  EXPECT_EQ(UserAgentType::MOBILE, item1->GetUserAgentType());
+  EXPECT_EQ(GURL("http://www.0.com"), item1->GetReferrer().url);
+
+  // This item is created by CommitPendingItem.
+  NavigationItem* item2 = manager_->GetItemAtIndex(2);
+  EXPECT_EQ(pending_item2, item2);
+  EXPECT_EQ(GURL("http://www.2.com"), item2->GetURL());
+  EXPECT_TRUE(ui::PageTransitionCoreTypeIs(ui::PAGE_TRANSITION_TYPED,
+                                           item2->GetTransitionType()));
+  EXPECT_EQ(UserAgentType::DESKTOP, item2->GetUserAgentType());
+  EXPECT_EQ(GURL(""), item2->GetReferrer().url);
+}
+
+// Tests that AddPendingItem does not create a new NavigationItem if the new
+// pending item is a back forward navigation.
+TEST_F(WKBasedNavigationManagerTest, ReusePendingItemForHistoryNavigation) {
+  // Simulate two regular navigations.
+  [mock_wk_list_
+        setCurrentURL:@"http://www.1.com"
+         backListURLs:[NSArray arrayWithObjects:@"http://www.0.com", nil]
+      forwardListURLs:nil];
+
+  // Force sync NavigationItems.
+  NavigationItem* original_item0 = manager_->GetItemAtIndex(0);
+  manager_->GetItemAtIndex(1);
+
+  // Simulate a back-forward navigation. Manually shuffle the objects in
+  // mock_wk_list_ to avoid creating new WKBackForwardListItem mocks and
+  // preserve existing NavigationItem associations.
+  WKBackForwardListItem* wk_item0 = [mock_wk_list_ itemAtIndex:-1];
+  WKBackForwardListItem* wk_item1 = [mock_wk_list_ itemAtIndex:0];
+  mock_wk_list_.currentItem = wk_item0;
+  mock_wk_list_.backList = nil;
+  mock_wk_list_.forwardList = [NSArray arrayWithObjects:wk_item1, nil];
+  OCMExpect([mock_web_view_ URL])
+      .andReturn([[NSURL alloc] initWithString:@"http://www.0.com"]);
+
+  manager_->AddPendingItem(
+      GURL("http://www.0.com"), Referrer(), ui::PAGE_TRANSITION_TYPED,
+      web::NavigationInitiationType::USER_INITIATED,
+      web::NavigationManager::UserAgentOverrideOption::MOBILE);
+
+  EXPECT_EQ(original_item0, manager_->GetPendingItem());
+}
+
+// Tests DiscardNonCommittedItems discards both pending and transient items.
+TEST_F(WKBasedNavigationManagerTest, DiscardNonCommittedItems) {
+  manager_->AddPendingItem(
+      GURL("http://www.0.com"), Referrer(), ui::PAGE_TRANSITION_TYPED,
+      web::NavigationInitiationType::USER_INITIATED,
+      web::NavigationManager::UserAgentOverrideOption::DESKTOP);
+  manager_->AddTransientItem(GURL("http://www.1.com"));
+
+  EXPECT_NE(nullptr, manager_->GetPendingItem());
+  EXPECT_NE(nullptr, manager_->GetTransientItem());
+
+  manager_->DiscardNonCommittedItems();
+  EXPECT_EQ(nullptr, manager_->GetPendingItem());
+  EXPECT_EQ(nullptr, manager_->GetTransientItem());
+}
+
+// Tests that in the absence of a transient item, going back is delegated to the
+// underlying WKWebView without any sanity checks such as whether any back
+// history exists.
+TEST_F(WKBasedNavigationManagerTest, GoBackWithoutTransientItem) {
+  // The cast is necessary because without it, compiler cannot disambiguate
+  // between UIWebView's goBack and WKWebView's goBack. Not using C++ style cast
+  // because it doesn't work on id type.
+  [(WKWebView*)[mock_web_view_ expect] goBack];
+  manager_->GoBack();
+  [mock_web_view_ verify];
+}
+
+// Tests that going back from a transient item will discard the transient item
+// without any navigations on the underlying WKBackForwardList.
+TEST_F(WKBasedNavigationManagerTest, GoBackFromTransientItem) {
+  manager_->AddPendingItem(
+      GURL("http://www.0.com"), Referrer(), ui::PAGE_TRANSITION_TYPED,
+      web::NavigationInitiationType::USER_INITIATED,
+      web::NavigationManager::UserAgentOverrideOption::DESKTOP);
+  manager_->AddTransientItem(GURL("http://www.1.com"));
+
+  [(WKWebView*)[mock_web_view_ reject] goBack];
+  manager_->GoBack();
+  [mock_web_view_ verify];
+
+  EXPECT_NE(nullptr, manager_->GetPendingItem());
+  EXPECT_EQ(nullptr, manager_->GetTransientItem());
+}
+
+// Tests that going forward is always delegated to the underlying WKWebView
+// without any sanity checks such as whether any forward history exists.
+TEST_F(WKBasedNavigationManagerTest, GoForward) {
+  [(WKWebView*)[mock_web_view_ expect] goForward];
+  manager_->GoForward();
+  [mock_web_view_ verify];
+}
+
+// Tests that going forward clears uncommitted items.
+TEST_F(WKBasedNavigationManagerTest, GoForwardShouldDiscardsUncommittedItems) {
+  manager_->AddPendingItem(
+      GURL("http://www.0.com"), Referrer(), ui::PAGE_TRANSITION_TYPED,
+      web::NavigationInitiationType::USER_INITIATED,
+      web::NavigationManager::UserAgentOverrideOption::DESKTOP);
+  manager_->AddTransientItem(GURL("http://www.1.com"));
+
+  EXPECT_NE(nullptr, manager_->GetPendingItem());
+  EXPECT_NE(nullptr, manager_->GetTransientItem());
+
+  manager_->GoForward();
+
+  EXPECT_EQ(nullptr, manager_->GetPendingItem());
+  EXPECT_EQ(nullptr, manager_->GetTransientItem());
+}
+
+}  // namespace web

ios/web/web_state/ui/crw_web_view_navigation_proxy.h

@@ -12,6 +12,7 @@ NS_ASSUME_NONNULL_BEGIN
 // A protocol to expose a subset of the WKWebView API to NavigationManager.
 @protocol CRWWebViewNavigationProxy
 
+@property(nullable, nonatomic, readonly, copy) NSURL* URL;
 @property(nonatomic, readonly, strong) WKBackForwardList* backForwardList;
 @property(nonatomic, readonly) BOOL canGoBack;
 @property(nonatomic, readonly) BOOL canGoForward;