[ios] Prevent taps in the NTP if the underlying NTP hasn't committed.

Sometimes the underlying ios/web page used for the NTP (about://newtab)
takes a long time to load.  Loading any page before the newtab is committed
will leave ios/web in a bad state.  Instead, block any most visited and
content suggestion taps until the underlying NTP is committed.

See: crbug.com/925304 for more context.  Remove this when ios/web supports
queueing multiple loads during this state.

Bug: 929148
Change-Id: I78bf8c58127c19f96432e68510c1565ad4b6d973
Reviewed-on: https://chromium-review.googlesource.com/c/1456645Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#629808}

ios/chrome/browser/ntp/BUILD.gn

@@ -10,6 +10,7 @@ source_set("ntp") {
   ]
   configs += [ "//build/config/compiler:enable_arc" ]
   deps = [
+    ":features",
     "//base:base",
     "//components/strings:components_strings_grit",
     "//ios/chrome/browser",
@@ -19,6 +20,16 @@ source_set("ntp") {
   ]
 }
 
+source_set("features") {
+  sources = [
+    "features.cc",
+    "features.h",
+  ]
+  deps = [
+    "//base",
+  ]
+}
+
 source_set("unit_tests") {
   configs += [ "//build/config/compiler:enable_arc" ]
   testonly = true

ios/chrome/browser/ntp/features.cc

+// Copyright 2019 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.
+
+#include "ios/chrome/browser/ntp/features.h"
+
+const base::Feature kBlockNewTabPagePendingLoad{
+    "BlockNewTabPagePendingLoad", base::FEATURE_ENABLED_BY_DEFAULT};

ios/chrome/browser/ntp/features.h

+// Copyright 2019 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_CHROME_BROWSER_NTP_FEATURES_H_
+#define IOS_CHROME_BROWSER_NTP_FEATURES_H_
+
+#include "base/feature_list.h"
+
+// Feature flag to enable NTP UI pending loader blocker.
+extern const base::Feature kBlockNewTabPagePendingLoad;
+
+#endif  // IOS_CHROME_BROWSER_NTP_FEATURES_H_

ios/chrome/browser/ntp/new_tab_page_tab_helper.h

@@ -6,8 +6,10 @@
 #define IOS_CHROME_BROWSER_NTP_NEW_TAB_PAGE_TAB_HELPER_H_
 
 #import <UIKit/UIKit.h>
+#include <memory>
 
 #include "base/macros.h"
+#include "base/timer/timer.h"
 #include "ios/web/public/web_state/web_state_observer.h"
 #import "ios/web/public/web_state/web_state_user_data.h"
 
@@ -31,6 +33,13 @@ class NewTabPageTabHelper : public web::WebStateObserver,
   // WebStateObserver callback.
   void Deactivate();
 
+  // Sometimes the underlying ios/web page used for the NTP (about://newtab)
+  // takes a long time to load.  Loading any page before the newtab is committed
+  // will leave ios/web in a bad state.  See: crbug.com/925304 for more context.
+  // Remove this when ios/web supports queueing multiple loads during this
+  // state.
+  bool IgnoreLoadRequests() const;
+
  private:
   NewTabPageTabHelper(web::WebState* web_state,
                       id<NewTabPageTabHelperDelegate> delegate);
@@ -52,6 +61,14 @@ class NewTabPageTabHelper : public web::WebStateObserver,
   // Returns true if an |url| is either chrome://newtab or about://newtab.
   bool IsNTPURL(const GURL& url);
 
+  // Sets the |ignore_load_requests_| flag to YES and starts the ignore load
+  // timer.
+  void EnableIgnoreLoadRequests();
+
+  // Sets the |ignore_load_requests_| flag to NO and stops the ignore load
+  // timer.
+  void DisableIgnoreLoadRequests();
+
   // Used to present and dismiss the NTP.
   __weak id<NewTabPageTabHelperDelegate> delegate_ = nil;
 
@@ -61,6 +78,13 @@ class NewTabPageTabHelper : public web::WebStateObserver,
   // |YES| if the current tab helper is active.
   BOOL active_;
 
+  // |YES| if the NTP's underlying ios/web page is still loading.
+  BOOL ignore_load_requests_ = NO;
+
+  // Ensure the ignore_load_requests_ flag is never set to NO for more than
+  // |kMaximumIgnoreLoadRequestsTime| seconds.
+  std::unique_ptr<base::OneShotTimer> ignore_load_requests_timer_ = nullptr;
+
   DISALLOW_COPY_AND_ASSIGN(NewTabPageTabHelper);
 };
 

ios/chrome/browser/ntp/new_tab_page_tab_helper.mm

@@ -12,6 +12,7 @@
 #include "base/strings/sys_string_conversions.h"
 #include "components/strings/grit/components_strings.h"
 #include "ios/chrome/browser/chrome_url_constants.h"
+#include "ios/chrome/browser/ntp/features.h"
 #include "ios/chrome/browser/ntp/new_tab_page_tab_helper_delegate.h"
 #include "ios/chrome/browser/ui/ui_feature_flags.h"
 #import "ios/web/public/navigation_item.h"
@@ -29,6 +30,10 @@ namespace {
 // |url::kAboutScheme|, there's no host value, only a path.  Use this value for
 // matching the NTP.
 const char kAboutNewTabPath[] = "//newtab/";
+
+// Maximum number of seconds for |ignore_load_requests_| to be set to YES.
+static const size_t kMaximumIgnoreLoadRequestsTime = 10;
+
 }  // namespace
 
 // static
@@ -58,6 +63,12 @@ NewTabPageTabHelper::NewTabPageTabHelper(
   if (active_) {
     UpdatePendingItem();
     [delegate_ newTabPageHelperDidChangeVisibility:this forWebState:web_state_];
+
+    // If about://newtab is currently loading but has not yet committed, block
+    // loads until it does commit.
+    if (!IsNTPURL(web_state->GetLastCommittedURL())) {
+      EnableIgnoreLoadRequests();
+    }
   }
 }
 
@@ -69,11 +80,39 @@ void NewTabPageTabHelper::Deactivate() {
   SetActive(false);
 }
 
+bool NewTabPageTabHelper::IgnoreLoadRequests() const {
+  return ignore_load_requests_;
+}
+
+void NewTabPageTabHelper::EnableIgnoreLoadRequests() {
+  if (!base::FeatureList::IsEnabled(kBlockNewTabPagePendingLoad))
+    return;
+
+  ignore_load_requests_ = YES;
+
+  // |ignore_load_requests_timer_| is deleted when the tab helper is deleted, so
+  // it's safe to use Unretained here.
+  ignore_load_requests_timer_.reset(new base::OneShotTimer());
+  ignore_load_requests_timer_->Start(
+      FROM_HERE, base::TimeDelta::FromSeconds(kMaximumIgnoreLoadRequestsTime),
+      base::BindOnce(&NewTabPageTabHelper::DisableIgnoreLoadRequests,
+                     base::Unretained(this)));
+}
+
+void NewTabPageTabHelper::DisableIgnoreLoadRequests() {
+  if (ignore_load_requests_timer_) {
+    ignore_load_requests_timer_->Stop();
+    ignore_load_requests_timer_.reset();
+  }
+  ignore_load_requests_ = NO;
+}
+
 #pragma mark - WebStateObserver
 
 void NewTabPageTabHelper::WebStateDestroyed(web::WebState* web_state) {
   web_state->RemoveObserver(this);
   SetActive(false);
+  DisableIgnoreLoadRequests();
 }
 
 void NewTabPageTabHelper::DidStartNavigation(
@@ -93,6 +132,7 @@ void NewTabPageTabHelper::DidFinishNavigation(
     return;
   }
 
+  DisableIgnoreLoadRequests();
   SetActive(IsNTPURL(web_state->GetLastCommittedURL()));
 }
 

ios/chrome/browser/ui/content_suggestions/BUILD.gn

@@ -40,6 +40,7 @@ source_set("content_suggestions") {
     "//ios/chrome/browser/browser_state",
     "//ios/chrome/browser/favicon",
     "//ios/chrome/browser/metrics:metrics_internal",
+    "//ios/chrome/browser/ntp",
     "//ios/chrome/browser/ntp_snippets",
     "//ios/chrome/browser/ntp_tiles",
     "//ios/chrome/browser/reading_list",

ios/chrome/browser/ui/content_suggestions/ntp_home_mediator.mm

@@ -14,6 +14,7 @@
 #include "components/strings/grit/components_strings.h"
 #include "ios/chrome/browser/chrome_url_constants.h"
 #import "ios/chrome/browser/metrics/new_tab_page_uma.h"
+#import "ios/chrome/browser/ntp/new_tab_page_tab_helper.h"
 #import "ios/chrome/browser/search_engines/search_engine_observer_bridge.h"
 #import "ios/chrome/browser/ui/alert_coordinator/alert_coordinator.h"
 #import "ios/chrome/browser/ui/commands/application_commands.h"
@@ -201,6 +202,11 @@ const char kNTPHelpURL[] =
 }
 
 - (void)openPageForItemAtIndexPath:(NSIndexPath*)indexPath {
+  NewTabPageTabHelper* NTPHelper =
+      NewTabPageTabHelper::FromWebState(self.webState);
+  if (NTPHelper && NTPHelper->IgnoreLoadRequests()) {
+    return;
+  }
   CollectionViewItem* item = [self.suggestionsViewController.collectionViewModel
       itemAtIndexPath:indexPath];
   ContentSuggestionsItem* suggestionItem =
@@ -256,6 +262,11 @@ const char kNTPHelpURL[] =
     return;
   }
 
+  NewTabPageTabHelper* NTPHelper =
+      NewTabPageTabHelper::FromWebState(self.webState);
+  if (NTPHelper && NTPHelper->IgnoreLoadRequests())
+    return;
+
   ContentSuggestionsMostVisitedItem* mostVisitedItem =
       base::mac::ObjCCastStrict<ContentSuggestionsMostVisitedItem>(item);
 
@@ -346,6 +357,10 @@ const char kNTPHelpURL[] =
 }
 
 - (void)handleLearnMoreTapped {
+  NewTabPageTabHelper* NTPHelper =
+      NewTabPageTabHelper::FromWebState(self.webState);
+  if (NTPHelper && NTPHelper->IgnoreLoadRequests())
+    return;
   GURL URL(kNTPHelpURL);
   ChromeLoadParams params(URL);
   [self.dispatcher loadURLWithParams:params];