Skip to content

Commit

Permalink
[WebLayer] Reload navigations on transient errors
Browse files Browse the repository at this point in the history
This copies more of the logic from chrome's NetErrorHelper that deals
with auto-reload on failed navigations. This should prevent the error
page from showing up when changing from wifi->cellular. This logic is
copied for now so we can merge to M83.

Deduplicating this logic is tracked in crbug.com/1073624.

Bug: 1072453
Change-Id: If623a563e8748a598b9430716d145c92447257be
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2159800
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#761685}
  • Loading branch information
clarkduvall authored and Commit Bot committed Apr 22, 2020
1 parent 0f125b6 commit 67648ea
Show file tree
Hide file tree
Showing 11 changed files with 382 additions and 15 deletions.
1 change: 1 addition & 0 deletions weblayer/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ source_set("weblayer_lib_base") {
"//components/crash/core/common",
"//components/embedder_support",
"//components/embedder_support/origin_trials",
"//components/error_page/common",
"//components/find_in_page",
"//components/infobars/core",
"//components/keyed_service/content",
Expand Down
51 changes: 51 additions & 0 deletions weblayer/browser/errorpage_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@
#include "weblayer/test/weblayer_browser_test.h"

#include "base/macros.h"
#include "base/test/bind_test_util.h"
#include "base/test/scoped_feature_list.h"
#include "content/public/test/url_loader_interceptor.h"
#include "net/test/url_request/url_request_failed_job.h"
#include "weblayer/common/features.h"
#include "weblayer/shell/browser/shell.h"
#include "weblayer/test/weblayer_browser_test_utils.h"

Expand Down Expand Up @@ -42,4 +46,51 @@ IN_PROC_BROWSER_TEST_F(ErrorPageBrowserTest, 404WithEmptyBody) {
NavigateAndWaitForFailure(error_page_url, shell());
}

class ErrorPageReloadBrowserTest : public ErrorPageBrowserTest {
public:
ErrorPageReloadBrowserTest() {
feature_list_.InitAndEnableFeature(features::kEnableAutoReload);
}

private:
base::test::ScopedFeatureList feature_list_;
};

class MockNetworkChangeNotifier : public net::NetworkChangeNotifier {
public:
ConnectionType GetCurrentConnectionType() const override {
return net::NetworkChangeNotifier::CONNECTION_4G;
}
};

IN_PROC_BROWSER_TEST_F(ErrorPageReloadBrowserTest, ReloadOnNetworkChanged) {
// Make sure the renderer thinks it's online, since that is a necessary
// condition for the reload.
net::NetworkChangeNotifier::DisableForTest disable;
MockNetworkChangeNotifier mock_notifier;

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.
bool first_try = true;
content::URLLoaderInterceptor interceptor(base::BindLambdaForTesting(
[&url, &first_try](content::URLLoaderInterceptor::RequestParams* params) {
if (params->url_request.url == url) {
if (first_try) {
first_try = false;
params->client->OnComplete(
network::URLLoaderCompletionStatus(net::ERR_NETWORK_CHANGED));
} else {
content::URLLoaderInterceptor::WriteResponse(
"weblayer/test/data/simple_page.html", params->client.get());
}
return true;
}
return false;
}));

NavigateAndWaitForCompletion(url, shell());
}

} // namespace weblayer
4 changes: 4 additions & 0 deletions weblayer/common/features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,9 @@ 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
1 change: 1 addition & 0 deletions weblayer/common/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ namespace features {
// Weblayer features in alphabetical order.

extern const base::Feature kWebLayerSafeBrowsing;
extern const base::Feature kEnableAutoReload;

} // namespace features
} // namespace weblayer
Expand Down
2 changes: 2 additions & 0 deletions weblayer/renderer/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ include_rules = [
"+components/autofill/content/renderer",
"+components/cdm/renderer",
"+components/content_settings/renderer",
"+components/error_page/common",
"+components/safe_browsing/content/common",
"+components/safe_browsing/content/renderer",
"+components/safe_browsing/core/common",
Expand All @@ -11,6 +12,7 @@ include_rules = [
"+components/spellcheck/renderer",
"+components/translate/content/renderer",
"+components/translate/core/common",
"+content/public/common",
"+content/public/renderer",
# needed for safebrowsing
"+mojo/public/cpp/bindings",
Expand Down
21 changes: 18 additions & 3 deletions weblayer/renderer/content_renderer_client_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "components/autofill/content/renderer/autofill_agent.h"
#include "components/autofill/content/renderer/password_autofill_agent.h"
#include "components/content_settings/renderer/content_settings_agent_impl.h"
#include "components/error_page/common/error.h"
#include "content/public/renderer/render_thread.h"
#include "third_party/blink/public/platform/platform.h"
#include "weblayer/common/features.h"
Expand Down Expand Up @@ -103,14 +104,28 @@ bool ContentRendererClientImpl::HasErrorPage(int http_status_code) {
return http_status_code >= 400;
}

bool ContentRendererClientImpl::ShouldSuppressErrorPage(
content::RenderFrame* render_frame,
const GURL& url) {
auto* error_page_helper = ErrorPageHelper::GetForFrame(render_frame);
if (error_page_helper)
return error_page_helper->ShouldSuppressErrorPage();
return false;
}

void ContentRendererClientImpl::PrepareErrorPage(
content::RenderFrame* render_frame,
const blink::WebURLError& error,
const std::string& http_method,
std::string* error_html) {
auto* ssl_helper = ErrorPageHelper::GetForFrame(render_frame);
if (ssl_helper)
ssl_helper->PrepareErrorPage();
auto* error_page_helper = ErrorPageHelper::GetForFrame(render_frame);
if (error_page_helper) {
error_page_helper->PrepareErrorPage(
error_page::Error::NetError(error.url(), error.reason(),
error.resolve_error_info(),
error.has_copy_in_cache()),
http_method == "POST");
}

#if defined(OS_ANDROID)
android_system_error_page::PopulateErrorPageHtml(error, error_html);
Expand Down
2 changes: 2 additions & 0 deletions weblayer/renderer/content_renderer_client_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ class ContentRendererClientImpl : public content::ContentRendererClient {
void RenderThreadStarted() override;
void RenderFrameCreated(content::RenderFrame* render_frame) override;
bool HasErrorPage(int http_status_code) override;
bool ShouldSuppressErrorPage(content::RenderFrame* render_frame,
const GURL& url) override;
void PrepareErrorPage(content::RenderFrame* render_frame,
const blink::WebURLError& error,
const std::string& http_method,
Expand Down
Loading

0 comments on commit 67648ea

Please sign in to comment.