Reland Add web::NavigationContext::IsPlaceholderNavigation

This is a step in multistep refactoring. The next steps will be:

1.) web::NavigationContext::URL is never set to placeholder
    URL and always represents navigation URL
2.) Same web::NavigationContext will be reused for placeholder
    navigation to extend it's lifetime.
3.) WebStateObserver::DidFinishNavigation will be caller after
    placeholder navigation is finished and will use original
    web::NavigationContext passed to WebStateObserver::DidStartNavigation

This will partially fix crbug.com/903497 and will call
WebStateObserver::DidFinishNavigation after committed URL actually
changed.

Originally Reviewed-on: https://chromium-review.googlesource.com/c/1331044

Bug: 903497
Change-Id: I0ee2ecdfb4d20d2f14fd62e99028e8de1b81ff3b
Reviewed-on: https://chromium-review.googlesource.com/c/1347212Reviewed-by: Danyao Wang <danyao@chromium.org>
Commit-Queue: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610238}

ios/chrome/browser/tabs/BUILD.gn

@@ -197,6 +197,7 @@ source_set("unit_tests") {
     "//ios/web/public/test",
     "//ios/web/public/test/fakes",
     "//ios/web/test/fakes:fakes",
+    "//ios/web/web_state:navigation_context",
     "//net",
     "//testing/gmock",
     "//testing/gtest",

ios/chrome/browser/tabs/DEPS

@@ -7,6 +7,7 @@ specific_include_rules = {
   ],
   "^tab_unittest\.mm$": [
     "+ios/web/web_state/ui/crw_web_controller.h",
+    "+ios/web/web_state/navigation_context_impl.h",
     "+ios/web/navigation/navigation_manager_impl.h",
     "+ios/web/web_state/web_state_impl.h",
     "+ios/web/test/fakes/crw_fake_back_forward_list.h",

ios/chrome/browser/tabs/tab_unittest.mm

@@ -48,6 +48,7 @@
 #import "ios/web/public/test/fakes/fake_navigation_context.h"
 #include "ios/web/public/test/test_web_thread_bundle.h"
 #import "ios/web/test/fakes/crw_fake_back_forward_list.h"
+#import "ios/web/web_state/navigation_context_impl.h"
 #import "ios/web/web_state/ui/crw_web_controller.h"
 #import "ios/web/web_state/web_state_impl.h"
 #import "net/base/mac/url_conversions.h"
@@ -237,9 +238,13 @@ class TabTest : public BlockCleanupTest,
                 NSString* title) {
     DCHECK_EQ(tab_.webState, web_state_impl_.get());
 
-    web::FakeNavigationContext context1;
-    context1.SetUrl(user_url);
-    web_state_impl_->OnNavigationStarted(&context1);
+    std::unique_ptr<web::NavigationContextImpl> context1 =
+        web::NavigationContextImpl::CreateNavigationContext(
+            web_state_impl_.get(), user_url,
+            /*has_user_gesture=*/true,
+            ui::PageTransition::PAGE_TRANSITION_AUTO_BOOKMARK,
+            /*is_renderer_initiated=*/true);
+    web_state_impl_->OnNavigationStarted(context1.get());
 
     web::Referrer empty_referrer;
     web_state_impl_->GetNavigationManagerImpl().AddPendingItem(
@@ -247,9 +252,13 @@ class TabTest : public BlockCleanupTest,
         web::NavigationInitiationType::RENDERER_INITIATED,
         web::NavigationManager::UserAgentOverrideOption::INHERIT);
 
-    web::FakeNavigationContext context2;
-    context2.SetUrl(redirect_url);
-    web_state_impl_->OnNavigationStarted(&context2);
+    std::unique_ptr<web::NavigationContextImpl> context2 =
+        web::NavigationContextImpl::CreateNavigationContext(
+            web_state_impl_.get(), redirect_url,
+            /*has_user_gesture=*/true,
+            ui::PageTransition::PAGE_TRANSITION_AUTO_BOOKMARK,
+            /*is_renderer_initiated=*/true);
+    web_state_impl_->OnNavigationStarted(context2.get());
 
     if (GetParam() == NavigationManagerChoice::WK_BASED) {
       [fake_wk_list_
@@ -257,9 +266,9 @@ class TabTest : public BlockCleanupTest,
     }
     web_state_impl_->GetNavigationManagerImpl().CommitPendingItem();
 
-    context2.SetHasCommitted(true);
+    context2->SetHasCommitted(true);
     web_state_impl_->UpdateHttpResponseHeaders(redirect_url);
-    web_state_impl_->OnNavigationFinished(&context2);
+    web_state_impl_->OnNavigationFinished(context2.get());
     web_state_impl_->SetIsLoading(true);
 
     base::string16 new_title = base::SysNSStringToUTF16(title);

ios/web/web_state/BUILD.gn

@@ -58,6 +58,7 @@ source_set("wk_web_view_security_util") {
 source_set("navigation_context") {
   deps = [
     "//base",
+    "//ios/web/navigation:core",
     "//ios/web/public",
   ]
 

ios/web/web_state/navigation_context_impl.h

@@ -85,6 +85,10 @@ class NavigationContextImpl : public NavigationContext {
   bool IsNativeContentPresented() const;
   void SetIsNativeContentPresented(bool is_native_content_presented);
 
+  // true if this navigation context is a placeholder navigation.
+  bool IsPlaceholderNavigation() const;
+  void SetPlaceholderNavigation(bool flag);
+
  private:
   NavigationContextImpl(WebState* web_state,
                         const GURL& url,
@@ -109,6 +113,7 @@ class NavigationContextImpl : public NavigationContext {
   bool is_loading_error_page_ = false;
   bool is_loading_html_string_ = false;
   bool is_native_content_presented_ = false;
+  bool is_placeholder_navigation_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(NavigationContextImpl);
 };

ios/web/web_state/navigation_context_impl.mm

@@ -175,6 +175,14 @@ void NavigationContextImpl::SetIsNativeContentPresented(
   is_native_content_presented_ = is_native_content_presented;
 }
 
+bool NavigationContextImpl::IsPlaceholderNavigation() const {
+  return is_placeholder_navigation_;
+}
+
+void NavigationContextImpl::SetPlaceholderNavigation(bool flag) {
+  is_placeholder_navigation_ = flag;
+}
+
 NavigationContextImpl::NavigationContextImpl(WebState* web_state,
                                              const GURL& url,
                                              bool has_user_gesture,

ios/web/web_state/web_state_impl.h

@@ -46,7 +46,7 @@ class BrowserState;
 struct ContextMenuParams;
 struct FaviconURL;
 struct LoadCommittedDetails;
-class NavigationContext;
+class NavigationContextImpl;
 class NavigationManager;
 class SessionCertificatePolicyCacheImpl;
 class WebInterstitialImpl;
@@ -78,12 +78,12 @@ class WebStateImpl : public WebState, public NavigationManagerDelegate {
   void SetWebController(CRWWebController* web_controller);
 
   // Notifies the observers that a navigation has started.
-  void OnNavigationStarted(web::NavigationContext* context);
+  void OnNavigationStarted(web::NavigationContextImpl* context);
 
   // Notifies the observers that a navigation has finished. For same-document
   // navigations notifies the observers about favicon URLs update using
   // candidates received in OnFaviconUrlUpdated.
-  void OnNavigationFinished(web::NavigationContext* context);
+  void OnNavigationFinished(web::NavigationContextImpl* context);
 
   // Called when current window's canGoBack / canGoForward state was changed.
   void OnBackForwardStateChanged();

ios/web/web_state/web_state_impl.mm

@@ -735,23 +735,27 @@ void WebStateImpl::TakeSnapshot(CGRect rect, SnapshotCallback callback) {
                 }];
 }
 
-void WebStateImpl::OnNavigationStarted(web::NavigationContext* context) {
+void WebStateImpl::OnNavigationStarted(web::NavigationContextImpl* context) {
   // Navigation manager loads internal URLs to restore session history and
   // create back-forward entries for Native View and WebUI. Do not trigger
   // external callbacks.
-  if (wk_navigation_util::IsWKInternalUrl(context->GetUrl()))
+  if (context->IsPlaceholderNavigation() ||
+      wk_navigation_util::IsRestoreSessionUrl(context->GetUrl())) {
     return;
+  }
 
   for (auto& observer : observers_)
     observer.DidStartNavigation(this, context);
 }
 
-void WebStateImpl::OnNavigationFinished(web::NavigationContext* context) {
+void WebStateImpl::OnNavigationFinished(web::NavigationContextImpl* context) {
   // Navigation manager loads internal URLs to restore session history and
   // create back-forward entries for Native View and WebUI. Do not trigger
   // external callbacks.
-  if (wk_navigation_util::IsWKInternalUrl(context->GetUrl()))
+  if (context->IsPlaceholderNavigation() ||
+      wk_navigation_util::IsRestoreSessionUrl(context->GetUrl())) {
     return;
+  }
 
   for (auto& observer : observers_)
     observer.DidFinishNavigation(this, context);

ios/web/web_state/web_state_impl_unittest.mm

@@ -415,7 +415,7 @@ TEST_P(WebStateImplTest, ObserverTest) {
   // Test that DidFinishNavigation() is called.
   ASSERT_FALSE(observer->did_finish_navigation_info());
   const GURL url("http://test");
-  std::unique_ptr<web::NavigationContext> context =
+  std::unique_ptr<NavigationContextImpl> context =
       NavigationContextImpl::CreateNavigationContext(
           web_state_.get(), url, /*has_user_gesture=*/true,
           ui::PageTransition::PAGE_TRANSITION_AUTO_BOOKMARK,
@@ -505,22 +505,27 @@ TEST_P(WebStateImplTest, ObserverTest) {
 // Tests that placeholder navigations are not visible to WebStateObservers.
 TEST_P(WebStateImplTest, PlaceholderNavigationNotExposedToObservers) {
   TestWebStateObserver observer(web_state_.get());
-  FakeNavigationContext context;
-  context.SetUrl(
-      wk_navigation_util::CreatePlaceholderUrlForUrl(GURL("chrome://newtab")));
-
+  GURL placeholder_url =
+      wk_navigation_util::CreatePlaceholderUrlForUrl(GURL("chrome://newtab"));
+  std::unique_ptr<NavigationContextImpl> context =
+      NavigationContextImpl::CreateNavigationContext(
+          web_state_.get(), placeholder_url,
+          /*has_user_gesture=*/true,
+          ui::PageTransition::PAGE_TRANSITION_AUTO_BOOKMARK,
+          /*is_renderer_initiated=*/true);
+  context->SetPlaceholderNavigation(true);
   // Test that OnPageLoaded() is not called.
-  web_state_->OnPageLoaded(context.GetUrl(), true /* load_success */);
+  web_state_->OnPageLoaded(placeholder_url, /*load_success=*/true);
   EXPECT_FALSE(observer.load_page_info());
-  web_state_->OnPageLoaded(context.GetUrl(), false /* load_success */);
+  web_state_->OnPageLoaded(placeholder_url, /*load_success=*/false);
   EXPECT_FALSE(observer.load_page_info());
 
   // Test that OnNavigationStarted() is not called.
-  web_state_->OnNavigationStarted(&context);
+  web_state_->OnNavigationStarted(context.get());
   EXPECT_FALSE(observer.did_start_navigation_info());
 
   // Test that OnNavigationFinished() is not called.
-  web_state_->OnNavigationFinished(&context);
+  web_state_->OnNavigationFinished(context.get());
   EXPECT_FALSE(observer.did_finish_navigation_info());
 }
 
@@ -666,8 +671,12 @@ TEST_P(WebStateImplTest, GlobalObserverTest) {
 
   // Test that DidStartNavigation() is called.
   EXPECT_FALSE(observer->did_start_navigation_called());
-  FakeNavigationContext context;
-  web_state_->OnNavigationStarted(&context);
+  std::unique_ptr<NavigationContextImpl> context =
+      NavigationContextImpl::CreateNavigationContext(
+          web_state_.get(), GURL::EmptyGURL(), /*has_user_gesture=*/true,
+          ui::PageTransition::PAGE_TRANSITION_AUTO_BOOKMARK,
+          /*is_renderer_initiated=*/true);
+  web_state_->OnNavigationStarted(context.get());
   EXPECT_TRUE(observer->did_start_navigation_called());
 
   // Test that WebStateDidStartLoading() is called.
@@ -927,9 +936,13 @@ TEST_P(WebStateImplTest, FaviconUpdateForSameDocumentNavigations) {
   auto observer = std::make_unique<TestWebStateObserver>(web_state_.get());
 
   // No callback if icons has not been fetched yet.
-  FakeNavigationContext context;
-  context.SetIsSameDocument(true);
-  web_state_->OnNavigationFinished(&context);
+  std::unique_ptr<NavigationContextImpl> context =
+      NavigationContextImpl::CreateNavigationContext(
+          web_state_.get(), GURL::EmptyGURL(),
+          /*has_user_gesture=*/false, ui::PageTransition::PAGE_TRANSITION_LINK,
+          /*is_renderer_initiated=*/false);
+  context->SetIsSameDocument(true);
+  web_state_->OnNavigationFinished(context.get());
   EXPECT_FALSE(observer->update_favicon_url_candidates_info());
 
   // Callback is called when icons were fetched.
@@ -942,7 +955,7 @@ TEST_P(WebStateImplTest, FaviconUpdateForSameDocumentNavigations) {
 
   // Callback is now called after same-document navigation.
   observer = std::make_unique<TestWebStateObserver>(web_state_.get());
-  web_state_->OnNavigationFinished(&context);
+  web_state_->OnNavigationFinished(context.get());
   ASSERT_TRUE(observer->update_favicon_url_candidates_info());
   ASSERT_EQ(1U,
             observer->update_favicon_url_candidates_info()->candidates.size());
@@ -959,14 +972,14 @@ TEST_P(WebStateImplTest, FaviconUpdateForSameDocumentNavigations) {
 
   // Document change navigation does not call callback.
   observer = std::make_unique<TestWebStateObserver>(web_state_.get());
-  context.SetIsSameDocument(false);
-  web_state_->OnNavigationFinished(&context);
+  context->SetIsSameDocument(false);
+  web_state_->OnNavigationFinished(context.get());
   EXPECT_FALSE(observer->update_favicon_url_candidates_info());
 
   // Previous candidates were invalidated by the document change. No callback
   // if icons has not been fetched yet.
-  context.SetIsSameDocument(true);
-  web_state_->OnNavigationFinished(&context);
+  context->SetIsSameDocument(true);
+  web_state_->OnNavigationFinished(context.get());
   EXPECT_FALSE(observer->update_favicon_url_candidates_info());
 }