[ios] Deactive the NTP as soon as we navigate away.

This enables us to remove the NTP as quickly as possible, without
needing to add a single-use WebStateObserver callback.  This only
affects the kBrowserContainerContainsNTP feature.

This CL also removes the use of -GetPendingItem in the NewTabPageTabHelper.

Bug: 826369
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: I0895b6109eead0aba3cd0f95bd8c1e97e1c4be28
Reviewed-on: https://chromium-review.googlesource.com/c/1283839
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600270}

ios/chrome/browser/ntp/new_tab_page_tab_helper.h

@@ -49,6 +49,11 @@ class NewTabPageTabHelper : public web::WebStateObserver,
   // Returns the current NTP controller.
   id<NewTabPageOwning> GetController() const;
 
+  // Disables this tab helper.  This is useful when navigating away from an NTP,
+  // so the tab helper can be disabled immediately, and before any potential
+  // WebStateObserver callback.
+  void Deactivate();
+
   // Returns the UIViewController for the current NTP.
   // TODO(crbug.com/826369): Currently there's a 1:1 relationship between the
   // webState and the NTP, so we can't delegate this coordinator to the BVC.
@@ -76,6 +81,9 @@ class NewTabPageTabHelper : public web::WebStateObserver,
   void DidStartNavigation(web::WebState* web_state,
                           web::NavigationContext* navigation_context) override;
 
+  // Enable or disable the tab helper.
+  void SetActive(bool active);
+
   // The Objective-C NTP coordinator instance.
   // TODO(crbug.com/826369): Currently there's a 1:1 relationship between the
   // webState and the NTP, so we can't delegate this coordinator to the BVC.
@@ -85,6 +93,9 @@ class NewTabPageTabHelper : public web::WebStateObserver,
   // Used to present and dismiss the NTP.
   __weak id<NewTabPageTabHelperDelegate> delegate_ = nil;
 
+  // The WebState with which this object is associated.
+  web::WebState* web_state_ = nullptr;
+
   DISALLOW_COPY_AND_ASSIGN(NewTabPageTabHelper);
 };
 

ios/chrome/browser/ntp/new_tab_page_tab_helper.mm

@@ -62,7 +62,7 @@ NewTabPageTabHelper::NewTabPageTabHelper(
        FakeboxFocuser,
        SnackbarCommands,
        UrlLoader> dispatcher)
-    : delegate_(delegate) {
+    : delegate_(delegate), web_state_(web_state) {
   DCHECK(delegate);
   DCHECK(base::FeatureList::IsEnabled(kBrowserContainerContainsNTP));
 
@@ -100,6 +100,10 @@ void NewTabPageTabHelper::DismissModals() const {
   return [ntp_coordinator_ dismissModals];
 }
 
+void NewTabPageTabHelper::Deactivate() {
+  SetActive(false);
+}
+
 #pragma mark - WebStateObserver
 
 void NewTabPageTabHelper::WebStateDestroyed(web::WebState* web_state) {
@@ -112,10 +116,16 @@ void NewTabPageTabHelper::DidStartNavigation(
   if (navigation_context->IsSameDocument()) {
     return;
   }
+
+  SetActive(navigation_context->GetUrl().GetOrigin() == kChromeUINewTabURL);
+}
+
+#pragma mark - Private
+
+void NewTabPageTabHelper::SetActive(bool active) {
   // Save the NTP scroll offset before we navigate away.
-  web::NavigationManager* manager = web_state->GetNavigationManager();
-  if (web_state->GetLastCommittedURL().GetOrigin() == kChromeUINewTabURL) {
-    DCHECK(IsActive());
+  web::NavigationManager* manager = web_state_->GetNavigationManager();
+  if (web_state_->GetLastCommittedURL().GetOrigin() == kChromeUINewTabURL) {
     web::NavigationItem* item = manager->GetLastCommittedItem();
     web::PageDisplayState displayState;
     CGPoint scrollOffset = ntp_coordinator_.scrollOffset;
@@ -126,10 +136,11 @@ void NewTabPageTabHelper::DidStartNavigation(
 
   bool was_active = IsActive();
 
-  // Start or stop the NTP.
-  web::NavigationItem* item = manager->GetPendingItem();
-  if (item && item->GetURL().GetOrigin() == kChromeUINewTabURL) {
-    item->SetTitle(l10n_util::GetStringUTF16(IDS_NEW_TAB_TITLE));
+  if (active) {
+    web::NavigationManager* manager = web_state_->GetNavigationManager();
+    web::NavigationItem* item = manager->GetPendingItem();
+    if (item)
+      item->SetTitle(l10n_util::GetStringUTF16(IDS_NEW_TAB_TITLE));
     [ntp_coordinator_ start];
   } else {
     [ntp_coordinator_ stop];

ios/chrome/browser/ntp/new_tab_page_tab_helper_unittest.mm

@@ -137,20 +137,20 @@ TEST_F(NewTabPageTabHelperTest, TestToggleToAndFromNTP) {
 
   GURL url(kChromeUINewTabURL);
   web::FakeNavigationContext context;
-  test_navigation_manager_->GetPendingItem()->SetURL(url);
+  context.SetUrl(url);
   test_web_state_.OnNavigationStarted(&context);
   EXPECT_TRUE(tab_helper()->IsActive());
 
   GURL not_ntp_url(kTestURL);
-  test_navigation_manager_->GetPendingItem()->SetURL(not_ntp_url);
+  context.SetUrl(not_ntp_url);
   test_web_state_.OnNavigationStarted(&context);
   EXPECT_FALSE(tab_helper()->IsActive());
 
-  test_navigation_manager_->GetPendingItem()->SetURL(url);
+  context.SetUrl(url);
   test_web_state_.OnNavigationStarted(&context);
   EXPECT_TRUE(tab_helper()->IsActive());
 
-  test_navigation_manager_->GetPendingItem()->SetURL(not_ntp_url);
+  context.SetUrl(not_ntp_url);
   test_web_state_.OnNavigationStarted(&context);
   EXPECT_FALSE(tab_helper()->IsActive());
 }

ios/chrome/browser/ui/browser_view_controller.mm

@@ -4303,6 +4303,15 @@ applicationCommandEndpoint:(id<ApplicationCommands>)applicationCommandEndpoint {
     return;
   }
 
+  web::WebState* webState = [_model currentTab].webState;
+  if (webState && params.url.GetOrigin() != kChromeUINewTabURL) {
+    NewTabPageTabHelper* NTPHelper =
+        NewTabPageTabHelper::FromWebState(webState);
+    if (NTPHelper && NTPHelper->IsActive()) {
+      NTPHelper->Deactivate();
+    }
+  }
+
   Tab* currentTab = [_model currentTab];
   DCHECK(currentTab);
   currentTab.navigationManager->LoadURLWithParams(params);