WebLayer: add a NavigationParams option to disable auto reload.

Bug: 1126205
Change-Id: I0409fa9fa12f272077bbef6f107ff81fe0866132
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2401975
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805964}

weblayer/BUILD.gn

@@ -207,6 +207,8 @@ source_set("weblayer_lib_base") {
     "browser/navigation_error_navigation_throttle.h",
     "browser/navigation_impl.cc",
     "browser/navigation_impl.h",
+    "browser/navigation_ui_data_impl.cc",
+    "browser/navigation_ui_data_impl.h",
     "browser/no_state_prefetch/prerender_link_manager_factory.cc",
     "browser/no_state_prefetch/prerender_link_manager_factory.h",
     "browser/no_state_prefetch/prerender_manager_delegate_impl.cc",

weblayer/browser/content_browser_client_impl.cc

@@ -81,6 +81,7 @@
 #include "weblayer/browser/i18n_util.h"
 #include "weblayer/browser/navigation_controller_impl.h"
 #include "weblayer/browser/navigation_error_navigation_throttle.h"
+#include "weblayer/browser/navigation_ui_data_impl.h"
 #include "weblayer/browser/no_state_prefetch/prerender_manager_factory.h"
 #include "weblayer/browser/no_state_prefetch/prerender_utils.h"
 #include "weblayer/browser/page_specific_content_settings_delegate.h"
@@ -164,6 +165,16 @@ bool IsSafebrowsingSupported() {
   return false;
 }
 
+bool IsNetworkErrorAutoReloadEnabled() {
+  const base::CommandLine& command_line =
+      *base::CommandLine::ForCurrentProcess();
+  if (command_line.HasSwitch(switches::kEnableAutoReload))
+    return true;
+  if (command_line.HasSwitch(switches::kDisableAutoReload))
+    return false;
+  return true;
+}
+
 bool IsInHostedApp(content::WebContents* web_contents) {
   return false;
 }
@@ -624,10 +635,16 @@ ContentBrowserClientImpl::CreateThrottlesForNavigation(
   std::vector<std::unique_ptr<content::NavigationThrottle>> throttles;
 
   if (handle->IsInMainFrame()) {
-    auto auto_reload_throttle =
-        error_page::NetErrorAutoReloader::MaybeCreateThrottleFor(handle);
-    if (auto_reload_throttle)
-      throttles.push_back(std::move(auto_reload_throttle));
+    NavigationUIDataImpl* navigation_ui_data =
+        static_cast<NavigationUIDataImpl*>(handle->GetNavigationUIData());
+    if ((!navigation_ui_data ||
+         !navigation_ui_data->disable_network_error_auto_reload()) &&
+        IsNetworkErrorAutoReloadEnabled()) {
+      auto auto_reload_throttle =
+          error_page::NetErrorAutoReloader::MaybeCreateThrottleFor(handle);
+      if (auto_reload_throttle)
+        throttles.push_back(std::move(auto_reload_throttle));
+    }
 
     // MetricsNavigationThrottle requires that it runs before
     // NavigationThrottles that may delay or cancel navigations, so only

weblayer/browser/errorpage_browsertest.cc

@@ -8,10 +8,12 @@
 #include "base/test/bind_test_util.h"
 #include "base/test/scoped_feature_list.h"
 #include "components/error_page/content/browser/net_error_auto_reloader.h"
+#include "content/public/test/browser_test_utils.h"
 #include "content/public/test/url_loader_interceptor.h"
 #include "net/test/url_request/url_request_failed_job.h"
 #include "weblayer/browser/tab_impl.h"
-#include "weblayer/common/features.h"
+#include "weblayer/public/common/switches.h"
+#include "weblayer/public/navigation_controller.h"
 #include "weblayer/shell/browser/shell.h"
 #include "weblayer/test/weblayer_browser_test_utils.h"
 
@@ -50,8 +52,49 @@ IN_PROC_BROWSER_TEST_F(ErrorPageBrowserTest, 404WithEmptyBody) {
 
 class ErrorPageReloadBrowserTest : public ErrorPageBrowserTest {
  public:
-  ErrorPageReloadBrowserTest() {
-    feature_list_.InitAndEnableFeature(features::kEnableAutoReload);
+  ErrorPageReloadBrowserTest() = default;
+
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    command_line->AppendSwitch(switches::kEnableAutoReload);
+    ErrorPageBrowserTest::SetUpCommandLine(command_line);
+  }
+
+  // Helper to perform navigations, whether successful or intercepted for
+  // simulated failure. Note that this asynchronously initiates the navigation
+  // and then waits only for the *navigation* to finish; this is in contrast to
+  // common test utilities which wait for loading to finish. It matters because
+  // most of NetErrorAutoReloader's interesting behavior is triggered at
+  // navigation completion and tests may want to observe the immediate side
+  // effects, such as the scheduling of an auto-reload timer.
+  //
+  // Return true if the navigation was successful, or false if it failed.
+  bool Navigate(const GURL& url, bool disable_network_error_auto_reload = false)
+      WARN_UNUSED_RESULT {
+    content::TestNavigationManager navigation(web_contents(), url);
+    NavigationController::NavigateParams params;
+    params.disable_network_error_auto_reload =
+        disable_network_error_auto_reload;
+    shell()->tab()->GetNavigationController()->Navigate(url, params);
+    navigation.WaitForNavigationFinished();
+    return navigation.was_successful();
+  }
+
+  // Returns the time-delay of the currently scheduled auto-reload task, if one
+  // is scheduled. If no auto-reload is scheduled, this returns null.
+  base::Optional<base::TimeDelta> GetCurrentAutoReloadDelay() {
+    auto* auto_reloader =
+        error_page::NetErrorAutoReloader::FromWebContents(web_contents());
+    if (!auto_reloader)
+      return base::nullopt;
+    const base::Optional<base::OneShotTimer>& timer =
+        auto_reloader->next_reload_timer_for_testing();
+    if (!timer)
+      return base::nullopt;
+    return timer->GetCurrentDelay();
+  }
+
+  content::WebContents* web_contents() {
+    return static_cast<TabImpl*>(shell()->tab())->web_contents();
   }
 
  private:
@@ -59,17 +102,15 @@ class ErrorPageReloadBrowserTest : public ErrorPageBrowserTest {
 };
 
 IN_PROC_BROWSER_TEST_F(ErrorPageReloadBrowserTest, ReloadOnNetworkChanged) {
+  ASSERT_TRUE(embedded_test_server()->Start());
   // Ensure that the NetErrorAutoReloader believes it's online, otherwise it
   // does not attempt auto-reload on error pages.
-  content::WebContents* web_contents =
-      static_cast<TabImpl*>(shell()->tab())->web_contents();
-  error_page::NetErrorAutoReloader::CreateForWebContents(web_contents);
+  error_page::NetErrorAutoReloader::CreateForWebContents(web_contents());
   auto* reloader =
-      error_page::NetErrorAutoReloader::FromWebContents(web_contents);
+      error_page::NetErrorAutoReloader::FromWebContents(web_contents());
   reloader->DisableConnectionChangeObservationForTesting();
   reloader->OnConnectionChanged(network::mojom::ConnectionType::CONNECTION_4G);
 
-  ASSERT_TRUE(embedded_test_server()->Start());
   GURL url = embedded_test_server()->GetURL("/error_page");
   // We send net::ERR_NETWORK_CHANGED on the first load, and the reload should
   // get a net::OK response.
@@ -93,4 +134,48 @@ IN_PROC_BROWSER_TEST_F(ErrorPageReloadBrowserTest, ReloadOnNetworkChanged) {
   NavigateAndWaitForCompletion(url, shell());
 }
 
+// By default auto reload is enabled.
+IN_PROC_BROWSER_TEST_F(ErrorPageReloadBrowserTest, AutoReloadDefault) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  // Ensure that the NetErrorAutoReloader believes it's online, otherwise it
+  // does not attempt auto-reload on error pages.
+  error_page::NetErrorAutoReloader::CreateForWebContents(web_contents());
+  auto* reloader =
+      error_page::NetErrorAutoReloader::FromWebContents(web_contents());
+  reloader->DisableConnectionChangeObservationForTesting();
+  reloader->OnConnectionChanged(network::mojom::ConnectionType::CONNECTION_4G);
+
+  GURL url = embedded_test_server()->GetURL("/error_page");
+  content::URLLoaderInterceptor interceptor(base::BindLambdaForTesting(
+      [&url](content::URLLoaderInterceptor::RequestParams* params) {
+        if (params->url_request.url == url) {
+          params->client->OnComplete(
+              network::URLLoaderCompletionStatus(net::ERR_NETWORK_CHANGED));
+          return true;
+        }
+        return false;
+      }));
+
+  EXPECT_FALSE(Navigate(url));
+  EXPECT_EQ(error_page::NetErrorAutoReloader::GetNextReloadDelayForTesting(0),
+            GetCurrentAutoReloadDelay());
+}
+
+IN_PROC_BROWSER_TEST_F(ErrorPageReloadBrowserTest, AutoReloadDisabled) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  GURL url = embedded_test_server()->GetURL("/error_page");
+  content::URLLoaderInterceptor interceptor(base::BindLambdaForTesting(
+      [&url](content::URLLoaderInterceptor::RequestParams* params) {
+        if (params->url_request.url == url) {
+          params->client->OnComplete(
+              network::URLLoaderCompletionStatus(net::ERR_NETWORK_CHANGED));
+          return true;
+        }
+        return false;
+      }));
+
+  EXPECT_FALSE(Navigate(url, true));
+  EXPECT_EQ(base::nullopt, GetCurrentAutoReloadDelay());
+}
+
 }  // namespace weblayer

weblayer/browser/java/org/chromium/weblayer_private/NavigationControllerImpl.java

@@ -36,15 +36,16 @@ public final class NavigationControllerImpl extends INavigationController.Stub {
         if (WebLayerFactoryImpl.getClientMajorVersion() < 83) {
             assert params == null;
         }
-        navigate2(uri, params == null ? false : params.mShouldReplaceCurrentEntry, false);
+        navigate2(uri, params == null ? false : params.mShouldReplaceCurrentEntry, false, false);
     }
 
     @Override
     public void navigate2(String uri, boolean shouldReplaceCurrentEntry,
-            boolean disableIntentProcessing) throws RemoteException {
+            boolean disableIntentProcessing, boolean disableNetworkErrorAutoReload)
+            throws RemoteException {
         StrictModeWorkaround.apply();
         NavigationControllerImplJni.get().navigate(mNativeNavigationController, uri,
-                shouldReplaceCurrentEntry, disableIntentProcessing);
+                shouldReplaceCurrentEntry, disableIntentProcessing, disableNetworkErrorAutoReload);
     }
 
     @Override
@@ -181,7 +182,8 @@ public final class NavigationControllerImpl extends INavigationController.Stub {
                 long nativeNavigationControllerImpl, NavigationControllerImpl caller);
         long getNavigationController(long tab);
         void navigate(long nativeNavigationControllerImpl, String uri,
-                boolean shouldReplaceCurrentEntry, boolean disableIntentProcessing);
+                boolean shouldReplaceCurrentEntry, boolean disableIntentProcessing,
+                boolean disableNetworkErrorAutoReload);
         void goBack(long nativeNavigationControllerImpl);
         void goForward(long nativeNavigationControllerImpl);
         boolean canGoBack(long nativeNavigationControllerImpl);

weblayer/browser/java/org/chromium/weblayer_private/interfaces/INavigationController.aidl

@@ -37,8 +37,9 @@ interface INavigationController {
   // Added in 85.
   boolean isNavigationEntrySkippable(int index) = 13;
 
-  // Added in 87.
+  // Added in 86.
   void navigate2(in String uri,
                  in boolean shouldReplaceEntry,
-                 in boolean disableIntentProcessing) = 14;
+                 in boolean disableIntentProcessing,
+                 in boolean disableNetworkErrorAutoReload) = 14;
 }

weblayer/browser/navigation_controller_impl.cc

@@ -14,6 +14,7 @@
 #include "content/public/browser/navigation_throttle.h"
 #include "content/public/browser/web_contents.h"
 #include "ui/base/page_transition_types.h"
+#include "weblayer/browser/navigation_ui_data_impl.h"
 #include "weblayer/browser/tab_impl.h"
 #include "weblayer/public/navigation_observer.h"
 
@@ -147,10 +148,12 @@ void NavigationControllerImpl::SetNavigationControllerImpl(
   java_controller_ = java_controller;
 }
 
-void NavigationControllerImpl::Navigate(JNIEnv* env,
-                                        const JavaParamRef<jstring>& url,
-                                        jboolean should_replace_current_entry,
-                                        jboolean disable_intent_processing) {
+void NavigationControllerImpl::Navigate(
+    JNIEnv* env,
+    const JavaParamRef<jstring>& url,
+    jboolean should_replace_current_entry,
+    jboolean disable_intent_processing,
+    jboolean disable_network_error_auto_reload) {
   auto params = std::make_unique<content::NavigationController::LoadURLParams>(
       GURL(base::android::ConvertJavaStringToUTF8(env, url)));
   params->should_replace_current_entry = should_replace_current_entry;
@@ -161,6 +164,9 @@ void NavigationControllerImpl::Navigate(JNIEnv* env,
   params->transition_type = disable_intent_processing
                                 ? ui::PAGE_TRANSITION_TYPED
                                 : ui::PAGE_TRANSITION_LINK;
+  if (disable_network_error_auto_reload)
+    params->navigation_ui_data = std::make_unique<NavigationUIDataImpl>(true);
+
   DoNavigate(std::move(params));
 }
 
@@ -228,6 +234,10 @@ void NavigationControllerImpl::Navigate(
       std::make_unique<content::NavigationController::LoadURLParams>(url);
   load_params->should_replace_current_entry =
       params.should_replace_current_entry;
+  if (params.disable_network_error_auto_reload) {
+    load_params->navigation_ui_data =
+        std::make_unique<NavigationUIDataImpl>(true);
+  }
   DoNavigate(std::move(load_params));
 }
 

weblayer/browser/navigation_controller_impl.h

@@ -52,7 +52,8 @@ class NavigationControllerImpl : public NavigationController,
   void Navigate(JNIEnv* env,
                 const base::android::JavaParamRef<jstring>& url,
                 jboolean should_replace_current_entry,
-                jboolean disableIntentProcessing);
+                jboolean disable_intent_processing,
+                jboolean disable_network_error_auto_reload);
   void GoBack(JNIEnv* env) { GoBack(); }
   void GoForward(JNIEnv* env) { GoForward(); }
   bool CanGoBack(JNIEnv* env) { return CanGoBack(); }

weblayer/browser/navigation_ui_data_impl.cc

+// Copyright 2020 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 "weblayer/browser/navigation_ui_data_impl.h"
+
+namespace weblayer {
+
+NavigationUIDataImpl::NavigationUIDataImpl(
+    bool disable_network_error_auto_reload)
+    : disable_network_error_auto_reload_(disable_network_error_auto_reload) {}
+
+NavigationUIDataImpl::~NavigationUIDataImpl() = default;
+
+std::unique_ptr<content::NavigationUIData> NavigationUIDataImpl::Clone() {
+  return std::make_unique<NavigationUIDataImpl>(
+      disable_network_error_auto_reload_);
+}
+
+}  // namespace weblayer

weblayer/browser/navigation_ui_data_impl.h

+// Copyright 2020 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 WEBLAYER_BROWSER_NAVIGATION_UI_DATA_IMPL_H_
+#define WEBLAYER_BROWSER_NAVIGATION_UI_DATA_IMPL_H_
+
+#include "content/public/browser/navigation_ui_data.h"
+
+namespace weblayer {
+
+// Data that we pass to content::NavigationController::LoadURLWithParams
+// and can access from content::NavigationHandle later.
+class NavigationUIDataImpl : public content::NavigationUIData {
+ public:
+  explicit NavigationUIDataImpl(bool disable_network_error_auto_reload);
+  NavigationUIDataImpl(const NavigationUIDataImpl&) = delete;
+  NavigationUIDataImpl& operator=(const NavigationUIDataImpl&) = delete;
+  ~NavigationUIDataImpl() override;
+
+  // content::NavigationUIData implementation:
+  std::unique_ptr<content::NavigationUIData> Clone() override;
+
+  bool disable_network_error_auto_reload() const {
+    return disable_network_error_auto_reload_;
+  }
+
+ private:
+  bool disable_network_error_auto_reload_;
+};
+
+}  // namespace weblayer
+
+#endif  // WEBLAYER_BROWSER_NAVIGATION_UI_DATA_IMPL_H_

weblayer/common/features.cc

@@ -13,9 +13,5 @@ namespace features {
 const base::Feature kWebLayerSafeBrowsing{"WebLayerSafeBrowsing",
                                           base::FEATURE_ENABLED_BY_DEFAULT};
 
-// Enable auto-reload of error pages.
-const base::Feature kEnableAutoReload{"EnableAutoReload",
-                                      base::FEATURE_ENABLED_BY_DEFAULT};
-
 }  // namespace features
 }  // namespace weblayer

weblayer/common/features.h

@@ -13,7 +13,6 @@ namespace features {
 // Weblayer features in alphabetical order.
 
 extern const base::Feature kWebLayerSafeBrowsing;
-extern const base::Feature kEnableAutoReload;
 
 }  // namespace features
 }  // namespace weblayer

weblayer/public/common/switches.cc

@@ -7,6 +7,12 @@
 namespace weblayer {
 namespace switches {
 
+// Disable auto-reload of error pages.
+const char kDisableAutoReload[] = "disable-auto-reload";
+
+// Enable auto-reload of error pages.
+const char kEnableAutoReload[] = "enable-auto-reload";
+
 // Makes WebLayer Shell use the given path for its data directory.
 const char kWebLayerUserDataDir[] = "weblayer-user-data-dir";
 

weblayer/public/common/switches.h

@@ -8,6 +8,8 @@
 namespace weblayer {
 namespace switches {
 
+extern const char kDisableAutoReload[];
+extern const char kEnableAutoReload[];
 extern const char kWebLayerUserDataDir[];
 
 }  // namespace switches

weblayer/public/java/org/chromium/weblayer/NavigateParams.java

@@ -15,6 +15,7 @@ public class NavigateParams {
     private org.chromium.weblayer_private.interfaces.NavigateParams mInterfaceParams =
             new org.chromium.weblayer_private.interfaces.NavigateParams();
     private boolean mIntentProcessingDisabled;
+    private boolean mNetworkErrorAutoReloadDisabled;
 
     /**
      * A Builder class to help create NavigateParams.
@@ -54,16 +55,31 @@ public class NavigateParams {
          * depends upon a wide range of state (such as the uri being navigated to, navigation
          * stack...).
          *
-         * @since 87
+         * @since 86
          */
         @NonNull
         public Builder disableIntentProcessing() {
-            if (WebLayer.getSupportedMajorVersionInternal() < 87) {
+            if (WebLayer.getSupportedMajorVersionInternal() < 86) {
                 throw new UnsupportedOperationException();
             }
             mParams.mIntentProcessingDisabled = true;
             return this;
         }
+
+        /**
+         * Disables auto-reload for this navigation if the network is down and comes back later.
+         *          Auto-reload is enabled by default.
+         *
+         * @since 86
+         */
+        @NonNull
+        public Builder disableNetworkErrorAutoReload() {
+            if (WebLayer.getSupportedMajorVersionInternal() < 86) {
+                throw new UnsupportedOperationException();
+            }
+            mParams.mNetworkErrorAutoReloadDisabled = true;
+            return this;
+        }
     }
 
     org.chromium.weblayer_private.interfaces.NavigateParams toInterfaceParams() {
@@ -82,14 +98,28 @@ public class NavigateParams {
     /**
      * Returns true if intent processing is disabled.
      *
-     * @return Whether intent process is disabled.
+     * @return Whether intent processing is disabled.
      *
-     * @since 87
+     * @since 86
      */
     public boolean isIntentProcessingDisabled() {
-        if (WebLayer.getSupportedMajorVersionInternal() < 87) {
+        if (WebLayer.getSupportedMajorVersionInternal() < 86) {
             throw new UnsupportedOperationException();
         }
         return mIntentProcessingDisabled;
     }
+
+    /**
+     * Returns true if auto reload for network errors is disabled.
+     *
+     * @return Whether auto reload for network errors is disabled.
+     *
+     * @since 86
+     */
+    public boolean isNetworkErrorAutoReloadDisabled() {
+        if (WebLayer.getSupportedMajorVersionInternal() < 86) {
+            throw new UnsupportedOperationException();
+        }
+        return mNetworkErrorAutoReloadDisabled;
+    }
 }

weblayer/public/java/org/chromium/weblayer/NavigationController.java

@@ -59,13 +59,14 @@ public class NavigationController {
             throw new UnsupportedOperationException();
         }
         try {
-            if (params == null || WebLayer.getSupportedMajorVersionInternal() < 87) {
+            if (params == null || WebLayer.getSupportedMajorVersionInternal() < 86) {
                 mNavigationController.navigate(
                         uri.toString(), params == null ? null : params.toInterfaceParams());
             } else {
                 mNavigationController.navigate2(uri.toString(),
                         params == null ? false : params.getShouldReplaceCurrentEntry(),
-                        params == null ? false : params.isIntentProcessingDisabled());
+                        params == null ? false : params.isIntentProcessingDisabled(),
+                        params == null ? false : params.isNetworkErrorAutoReloadDisabled());
             }
         } catch (RemoteException e) {
             throw new APICallException(e);

weblayer/public/navigation_controller.h

@@ -19,6 +19,7 @@ class NavigationController {
   // |NavigationController::LoadURLParams|.
   struct NavigateParams {
     bool should_replace_current_entry = false;
+    bool disable_network_error_auto_reload = false;
   };
 
   virtual ~NavigationController() = default;

weblayer/test/weblayer_browser_test.cc

@@ -17,11 +17,6 @@ namespace weblayer {
 
 WebLayerBrowserTest::WebLayerBrowserTest() {
   CreateTestServer(base::FilePath(FILE_PATH_LITERAL("weblayer/test/data")));
-
-  // Disable auto reload since most browser tests do not expect error pages to
-  // reload automatically. Tests that want auto reload can explicitly enable
-  // this feature.
-  feature_list_.InitAndDisableFeature(features::kEnableAutoReload);
 }
 
 WebLayerBrowserTest::~WebLayerBrowserTest() = default;
@@ -30,6 +25,11 @@ void WebLayerBrowserTest::SetUp() {
   base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
   command_line->AppendSwitch(switches::kNoInitialNavigation);
 
+  // Disable auto reload since most browser tests do not expect error pages to
+  // reload automatically. Tests that want auto reload can explicitly append
+  // switches::kEnableAutoReload, which will override the disable here.
+  command_line->AppendSwitch(switches::kDisableAutoReload);
+
   if (start_in_incognito_mode_)
     command_line->AppendSwitch(switches::kStartInIncognito);