Fix NetErrorTabHelper with PlzNavigate.
This includes:
-switching NetErrorTabHelper to use the new WebContentsObserver navigation callbacks that work with PlzNavigate
-fixing NavigationHandle::IsErrorPage to return true for reloads of error pages
This fixes
DnsProbeBrowserTest.CorrectionsDisabled
DnsProbeBrowserTest.CorrectionsLoadStopped
DnsProbeBrowserTest.CorrectionsLoadStoppedSlowProbe
DnsProbeBrowserTest.Incognito
DnsProbeBrowserTest.NoInternetProbeResultWithBrokenCorrections
DnsProbeBrowserTest.NoInternetProbeResultWithSlowBrokenCorrections
DnsProbeBrowserTest.NxdomainProbeResultWithWorkingCorrections
DnsProbeBrowserTest.NxdomainProbeResultWithWorkingSlowCorrections
DnsProbeBrowserTest.ProbesDisabled
DnsProbeBrowserTest.SyncFailureWithBrokenCorrections
ErrorPageNavigationCorrectionsFailTest.FetchCorrectionsFails
BUG=504347,647859
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
R=clamy@chromium.org, juliatuttle@chromium.org
Review URL: https://codereview.chromium.org/2345913002 .
Cr-Commit-Position: refs/heads/master@{#419483}
diff --git a/chrome/browser/net/net_error_tab_helper.cc b/chrome/browser/net/net_error_tab_helper.cc
index 3733c69f..c671f47e 100644
--- a/chrome/browser/net/net_error_tab_helper.cc
+++ b/chrome/browser/net/net_error_tab_helper.cc
@@ -105,78 +105,47 @@
client->SetCanShowNetworkDiagnosticsDialog(CanShowNetworkDiagnosticsDialog());
}
-void NetErrorTabHelper::DidStartNavigationToPendingEntry(
- const GURL& url,
- content::ReloadType reload_type) {
- DCHECK_CURRENTLY_ON(BrowserThread::UI);
-
- if (!is_error_page_)
+void NetErrorTabHelper::DidStartNavigation(
+ content::NavigationHandle* navigation_handle) {
+ if (!navigation_handle->IsInMainFrame())
return;
- // Only record reloads.
- if (reload_type != content::ReloadType::NONE) {
+ if (navigation_handle->IsErrorPage() &&
+ navigation_handle->GetPageTransition() == ui::PAGE_TRANSITION_RELOAD) {
error_page::RecordEvent(
error_page::NETWORK_ERROR_PAGE_BROWSER_INITIATED_RELOAD);
}
-}
-
-void NetErrorTabHelper::DidStartProvisionalLoadForFrame(
- content::RenderFrameHost* render_frame_host,
- const GURL& validated_url,
- bool is_error_page,
- bool is_iframe_srcdoc) {
- DCHECK_CURRENTLY_ON(BrowserThread::UI);
-
- if (render_frame_host->GetParent())
- return;
-
- is_error_page_ = is_error_page;
#if BUILDFLAG(ANDROID_JAVA_UI)
- UpdateHasOfflinePages(render_frame_host);
+ UpdateHasOfflinePages(navigation_handle->GetFrameTreeNodeId());
#endif // BUILDFLAG(ANDROID_JAVA_UI)
}
-void NetErrorTabHelper::DidCommitProvisionalLoadForFrame(
- content::RenderFrameHost* render_frame_host,
- const GURL& url,
- PageTransition transition_type) {
- DCHECK_CURRENTLY_ON(BrowserThread::UI);
-
- if (render_frame_host->GetParent())
+void NetErrorTabHelper::DidFinishNavigation(
+ content::NavigationHandle* navigation_handle) {
+ if (!navigation_handle->IsInMainFrame())
return;
+ if (IsDnsError(navigation_handle->GetNetErrorCode())) {
+ dns_error_active_ = true;
+ OnMainFrameDnsError();
+ }
+
// Resend status every time an error page commits; this is somewhat spammy,
// but ensures that the status will make it to the real error page, even if
// the link doctor loads a blank intermediate page or the tab switches
// renderer processes.
- if (is_error_page_ && dns_error_active_) {
+ if (navigation_handle->IsErrorPage() && dns_error_active_) {
dns_error_page_committed_ = true;
DVLOG(1) << "Committed error page; resending status.";
SendInfo();
- } else {
+ } else if (navigation_handle->HasCommitted() &&
+ !navigation_handle->IsErrorPage()) {
dns_error_active_ = false;
dns_error_page_committed_ = false;
}
}
-void NetErrorTabHelper::DidFailProvisionalLoad(
- content::RenderFrameHost* render_frame_host,
- const GURL& validated_url,
- int error_code,
- const base::string16& error_description,
- bool was_ignored_by_handler) {
- DCHECK_CURRENTLY_ON(BrowserThread::UI);
-
- if (render_frame_host->GetParent())
- return;
-
- if (IsDnsError(error_code)) {
- dns_error_active_ = true;
- OnMainFrameDnsError();
- }
-}
-
bool NetErrorTabHelper::OnMessageReceived(
const IPC::Message& message,
content::RenderFrameHost* render_frame_host) {
@@ -304,10 +273,9 @@
}
#if BUILDFLAG(ANDROID_JAVA_UI)
-void NetErrorTabHelper::UpdateHasOfflinePages(
- content::RenderFrameHost* render_frame_host) {
+void NetErrorTabHelper::UpdateHasOfflinePages(int frame_tree_node_id) {
// TODO(chili): remove entirely in M55 if AsyncLoading does not need this.
- SetHasOfflinePages(render_frame_host->GetFrameTreeNodeId(), false);
+ SetHasOfflinePages(frame_tree_node_id, false);
}
void NetErrorTabHelper::SetHasOfflinePages(int frame_tree_node_id,
diff --git a/chrome/browser/net/net_error_tab_helper.h b/chrome/browser/net/net_error_tab_helper.h
index 975ddf9..403c0ef 100644
--- a/chrome/browser/net/net_error_tab_helper.h
+++ b/chrome/browser/net/net_error_tab_helper.h
@@ -54,28 +54,10 @@
// content::WebContentsObserver implementation.
void RenderFrameCreated(content::RenderFrameHost* render_frame_host) override;
-
- void DidStartNavigationToPendingEntry(
- const GURL& url,
- content::ReloadType reload_type) override;
-
- void DidStartProvisionalLoadForFrame(
- content::RenderFrameHost* render_frame_host,
- const GURL& validated_url,
- bool is_error_page,
- bool is_iframe_srcdoc) override;
-
- void DidCommitProvisionalLoadForFrame(
- content::RenderFrameHost* render_frame_host,
- const GURL& url,
- ui::PageTransition transition_type) override;
-
- void DidFailProvisionalLoad(content::RenderFrameHost* render_frame_host,
- const GURL& validated_url,
- int error_code,
- const base::string16& error_description,
- bool was_ignored_by_handler) override;
-
+ void DidStartNavigation(
+ content::NavigationHandle* navigation_handle) override;
+ void DidFinishNavigation(
+ content::NavigationHandle* navigation_handle) override;
bool OnMessageReceived(const IPC::Message& message,
content::RenderFrameHost* render_frame_host) override;
@@ -113,7 +95,7 @@
// Relates to offline pages handling.
#if BUILDFLAG(ANDROID_JAVA_UI)
- void UpdateHasOfflinePages(content::RenderFrameHost* render_frame_host);
+ void UpdateHasOfflinePages(int frame_tree_node_id);
void SetHasOfflinePages(int frame_tree_node_id, bool has_offline_pages);
void ShowOfflinePages();
bool IsFromErrorPage() const;
diff --git a/chrome/browser/net/net_error_tab_helper_unittest.cc b/chrome/browser/net/net_error_tab_helper_unittest.cc
index 1f20962d..659ab306 100644
--- a/chrome/browser/net/net_error_tab_helper_unittest.cc
+++ b/chrome/browser/net/net_error_tab_helper_unittest.cc
@@ -8,11 +8,14 @@
#include "chrome/test/base/chrome_render_view_host_test_harness.h"
#include "components/error_page/common/net_error_info.h"
#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/navigation_handle.h"
#include "content/public/test/test_renderer_host.h"
#include "net/base/net_errors.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "ui/base/page_transition_types.h"
+#undef NO_ERROR
+
using chrome_browser_net::NetErrorTabHelper;
using error_page::DnsProbeStatus;
@@ -78,8 +81,7 @@
class NetErrorTabHelperTest : public ChromeRenderViewHostTestHarness {
protected:
enum MainFrame { SUB_FRAME, MAIN_FRAME };
- enum ErrorPage { NORMAL_PAGE, ERROR_PAGE };
- enum ErrorType { DNS_ERROR, OTHER_ERROR };
+ enum ErrorType { DNS_ERROR, OTHER_ERROR, NO_ERROR };
void SetUp() override {
ChromeRenderViewHostTestHarness::SetUp();
@@ -104,35 +106,20 @@
ChromeRenderViewHostTestHarness::TearDown();
}
- void StartProvisionalLoad(MainFrame main_frame, ErrorPage error_page) {
- tab_helper_->DidStartProvisionalLoadForFrame(
- (main_frame == MAIN_FRAME) ? main_rfh() : subframe_,
- bogus_url_, // validated_url
- (error_page == ERROR_PAGE),
- false); // is_iframe_srcdoc
- }
-
- void CommitProvisionalLoad(MainFrame main_frame) {
- tab_helper_->DidCommitProvisionalLoadForFrame(
- (main_frame == MAIN_FRAME) ? main_rfh() : subframe_,
- bogus_url_, // url
- ui::PAGE_TRANSITION_TYPED);
- }
-
- void FailProvisionalLoad(MainFrame main_frame, ErrorType error_type) {
- int net_error;
-
+ void DidFinishNavigation(MainFrame main_frame,
+ ErrorType error_type) {
+ net::Error net_error = net::OK;
if (error_type == DNS_ERROR)
net_error = net::ERR_NAME_NOT_RESOLVED;
else
net_error = net::ERR_TIMED_OUT;
-
- tab_helper_->DidFailProvisionalLoad(
- (main_frame == MAIN_FRAME) ? main_rfh() : subframe_,
- bogus_url_, // validated_url
- net_error,
- base::string16(),
- false);
+ std::unique_ptr<content::NavigationHandle> navigation_handle(
+ content::NavigationHandle::CreateNavigationHandleForTesting(
+ bogus_url_,
+ (main_frame == MAIN_FRAME) ? main_rfh() : subframe_,
+ true,
+ net_error));
+ // The destructor will call tab_helper_->DidFinishNavigation.
}
void FinishProbe(DnsProbeStatus status) { tab_helper_->FinishProbe(status); }
@@ -154,15 +141,13 @@
}
TEST_F(NetErrorTabHelperTest, MainFrameNonDnsError) {
- StartProvisionalLoad(MAIN_FRAME, NORMAL_PAGE);
- FailProvisionalLoad(MAIN_FRAME, OTHER_ERROR);
+ DidFinishNavigation(MAIN_FRAME, OTHER_ERROR);
EXPECT_FALSE(probe_running());
EXPECT_EQ(0, sent_count());
}
TEST_F(NetErrorTabHelperTest, NonMainFrameDnsError) {
- StartProvisionalLoad(SUB_FRAME, NORMAL_PAGE);
- FailProvisionalLoad(SUB_FRAME, DNS_ERROR);
+ DidFinishNavigation(SUB_FRAME, DNS_ERROR);
EXPECT_FALSE(probe_running());
EXPECT_EQ(0, sent_count());
}
@@ -172,90 +157,16 @@
// is going on, then fails over to the normal error page if and when Link
// Doctor fails to load or declines to provide a page.
-TEST_F(NetErrorTabHelperTest, ProbeResponseBeforeFirstCommit) {
- StartProvisionalLoad(MAIN_FRAME, NORMAL_PAGE);
- FailProvisionalLoad(MAIN_FRAME, DNS_ERROR);
- EXPECT_TRUE(probe_running());
- EXPECT_EQ(0, sent_count());
-
- StartProvisionalLoad(MAIN_FRAME, ERROR_PAGE);
- EXPECT_TRUE(probe_running());
- EXPECT_EQ(0, sent_count());
-
- FinishProbe(error_page::DNS_PROBE_FINISHED_NXDOMAIN);
- EXPECT_FALSE(probe_running());
- EXPECT_EQ(0, sent_count());
-
- CommitProvisionalLoad(MAIN_FRAME);
- EXPECT_FALSE(probe_running());
- EXPECT_EQ(1, sent_count());
- EXPECT_EQ(error_page::DNS_PROBE_FINISHED_NXDOMAIN, last_status_sent());
-
- StartProvisionalLoad(MAIN_FRAME, ERROR_PAGE);
- EXPECT_FALSE(probe_running());
- EXPECT_EQ(1, sent_count());
-
- CommitProvisionalLoad(MAIN_FRAME);
- EXPECT_FALSE(probe_running());
- EXPECT_EQ(2, sent_count());
- EXPECT_EQ(error_page::DNS_PROBE_FINISHED_NXDOMAIN, last_status_sent());
-}
-
-TEST_F(NetErrorTabHelperTest, ProbeResponseBetweenFirstAndSecondCommit) {
- StartProvisionalLoad(MAIN_FRAME, NORMAL_PAGE);
- FailProvisionalLoad(MAIN_FRAME, DNS_ERROR);
- EXPECT_TRUE(probe_running());
- EXPECT_EQ(0, sent_count());
-
- StartProvisionalLoad(MAIN_FRAME, ERROR_PAGE);
- EXPECT_TRUE(probe_running());
- EXPECT_EQ(0, sent_count());
-
- CommitProvisionalLoad(MAIN_FRAME);
+TEST_F(NetErrorTabHelperTest, ProbeResponse) {
+ DidFinishNavigation(MAIN_FRAME, DNS_ERROR);
EXPECT_TRUE(probe_running());
EXPECT_EQ(1, sent_count());
- EXPECT_EQ(error_page::DNS_PROBE_STARTED, last_status_sent());
FinishProbe(error_page::DNS_PROBE_FINISHED_NXDOMAIN);
EXPECT_FALSE(probe_running());
EXPECT_EQ(2, sent_count());
- EXPECT_EQ(error_page::DNS_PROBE_FINISHED_NXDOMAIN, last_status_sent());
- StartProvisionalLoad(MAIN_FRAME, ERROR_PAGE);
- EXPECT_FALSE(probe_running());
- EXPECT_EQ(2, sent_count());
-
- CommitProvisionalLoad(MAIN_FRAME);
- EXPECT_FALSE(probe_running());
- EXPECT_EQ(3, sent_count());
- EXPECT_EQ(error_page::DNS_PROBE_FINISHED_NXDOMAIN, last_status_sent());
-}
-
-TEST_F(NetErrorTabHelperTest, ProbeResponseAfterSecondCommit) {
- StartProvisionalLoad(MAIN_FRAME, NORMAL_PAGE);
- FailProvisionalLoad(MAIN_FRAME, DNS_ERROR);
- EXPECT_TRUE(probe_running());
- EXPECT_EQ(0, sent_count());
-
- StartProvisionalLoad(MAIN_FRAME, ERROR_PAGE);
- EXPECT_TRUE(probe_running());
- EXPECT_EQ(0, sent_count());
-
- CommitProvisionalLoad(MAIN_FRAME);
- EXPECT_TRUE(probe_running());
- EXPECT_EQ(1, sent_count());
- EXPECT_EQ(error_page::DNS_PROBE_STARTED, last_status_sent());
-
- StartProvisionalLoad(MAIN_FRAME, ERROR_PAGE);
- EXPECT_TRUE(probe_running());
- EXPECT_EQ(1, sent_count());
-
- CommitProvisionalLoad(MAIN_FRAME);
- EXPECT_TRUE(probe_running());
- EXPECT_EQ(2, sent_count());
- EXPECT_EQ(error_page::DNS_PROBE_STARTED, last_status_sent());
-
- FinishProbe(error_page::DNS_PROBE_FINISHED_NXDOMAIN);
+ DidFinishNavigation(MAIN_FRAME, NO_ERROR);
EXPECT_FALSE(probe_running());
EXPECT_EQ(3, sent_count());
EXPECT_EQ(error_page::DNS_PROBE_FINISHED_NXDOMAIN, last_status_sent());
@@ -264,129 +175,87 @@
// Send result even if a new page load has started; the error page is still
// visible, and the user might cancel the load.
TEST_F(NetErrorTabHelperTest, ProbeResponseAfterNewStart) {
- StartProvisionalLoad(MAIN_FRAME, NORMAL_PAGE);
- FailProvisionalLoad(MAIN_FRAME, DNS_ERROR);
- EXPECT_TRUE(probe_running());
- EXPECT_EQ(0, sent_count());
-
- StartProvisionalLoad(MAIN_FRAME, ERROR_PAGE);
- EXPECT_TRUE(probe_running());
- EXPECT_EQ(0, sent_count());
-
- CommitProvisionalLoad(MAIN_FRAME);
+ DidFinishNavigation(MAIN_FRAME, DNS_ERROR);
EXPECT_TRUE(probe_running());
EXPECT_EQ(1, sent_count());
EXPECT_EQ(error_page::DNS_PROBE_STARTED, last_status_sent());
- StartProvisionalLoad(MAIN_FRAME, ERROR_PAGE);
- EXPECT_TRUE(probe_running());
- EXPECT_EQ(1, sent_count());
-
- CommitProvisionalLoad(MAIN_FRAME);
+ DidFinishNavigation(MAIN_FRAME, DNS_ERROR);
EXPECT_TRUE(probe_running());
EXPECT_EQ(2, sent_count());
EXPECT_EQ(error_page::DNS_PROBE_STARTED, last_status_sent());
- StartProvisionalLoad(MAIN_FRAME, NORMAL_PAGE);
+ DidFinishNavigation(MAIN_FRAME, NO_ERROR);
EXPECT_TRUE(probe_running());
- EXPECT_EQ(2, sent_count());
+ EXPECT_EQ(3, sent_count());
FinishProbe(error_page::DNS_PROBE_FINISHED_NXDOMAIN);
EXPECT_FALSE(probe_running());
- EXPECT_EQ(3, sent_count());
+ EXPECT_EQ(4, sent_count());
EXPECT_EQ(error_page::DNS_PROBE_FINISHED_NXDOMAIN, last_status_sent());
}
// Don't send result if a new page has committed; the result would go to the
// wrong page, and the error page is gone anyway.
TEST_F(NetErrorTabHelperTest, ProbeResponseAfterNewCommit) {
- StartProvisionalLoad(MAIN_FRAME, NORMAL_PAGE);
- FailProvisionalLoad(MAIN_FRAME, DNS_ERROR);
- EXPECT_TRUE(probe_running());
- EXPECT_EQ(0, sent_count());
-
- StartProvisionalLoad(MAIN_FRAME, ERROR_PAGE);
- EXPECT_TRUE(probe_running());
- EXPECT_EQ(0, sent_count());
-
- CommitProvisionalLoad(MAIN_FRAME);
- EXPECT_TRUE(probe_running());
- EXPECT_EQ(1, sent_count());
- EXPECT_EQ(error_page::DNS_PROBE_STARTED, last_status_sent());
-
- StartProvisionalLoad(MAIN_FRAME, ERROR_PAGE);
+ DidFinishNavigation(MAIN_FRAME, DNS_ERROR);
EXPECT_TRUE(probe_running());
EXPECT_EQ(1, sent_count());
- CommitProvisionalLoad(MAIN_FRAME);
- EXPECT_TRUE(probe_running());
- EXPECT_EQ(2, sent_count());
- EXPECT_EQ(error_page::DNS_PROBE_STARTED, last_status_sent());
-
- StartProvisionalLoad(MAIN_FRAME, NORMAL_PAGE);
- EXPECT_TRUE(probe_running());
- EXPECT_EQ(2, sent_count());
-
- CommitProvisionalLoad(MAIN_FRAME);
+ DidFinishNavigation(MAIN_FRAME, NO_ERROR);
EXPECT_TRUE(probe_running());
EXPECT_EQ(2, sent_count());
FinishProbe(error_page::DNS_PROBE_FINISHED_NXDOMAIN);
EXPECT_FALSE(probe_running());
- EXPECT_EQ(2, sent_count());
+ EXPECT_EQ(3, sent_count());
}
TEST_F(NetErrorTabHelperTest, MultipleDnsErrorsWithProbesWithoutErrorPages) {
- StartProvisionalLoad(MAIN_FRAME, NORMAL_PAGE);
- FailProvisionalLoad(MAIN_FRAME, DNS_ERROR);
- EXPECT_TRUE(probe_running());
- EXPECT_EQ(0, sent_count());
-
- FinishProbe(error_page::DNS_PROBE_FINISHED_NXDOMAIN);
- EXPECT_FALSE(probe_running());
- EXPECT_EQ(0, sent_count());
-
- StartProvisionalLoad(MAIN_FRAME, NORMAL_PAGE);
- FailProvisionalLoad(MAIN_FRAME, DNS_ERROR);
- EXPECT_TRUE(probe_running());
- EXPECT_EQ(0, sent_count());
-
- FinishProbe(error_page::DNS_PROBE_FINISHED_NO_INTERNET);
- EXPECT_FALSE(probe_running());
- EXPECT_EQ(0, sent_count());
-}
-
-TEST_F(NetErrorTabHelperTest, MultipleDnsErrorsWithProbesAndErrorPages) {
- StartProvisionalLoad(MAIN_FRAME, NORMAL_PAGE);
- FailProvisionalLoad(MAIN_FRAME, DNS_ERROR);
- EXPECT_TRUE(probe_running());
- EXPECT_EQ(0, sent_count());
-
- StartProvisionalLoad(MAIN_FRAME, ERROR_PAGE);
- CommitProvisionalLoad(MAIN_FRAME);
+ DidFinishNavigation(MAIN_FRAME, DNS_ERROR);
EXPECT_TRUE(probe_running());
EXPECT_EQ(1, sent_count());
- EXPECT_EQ(error_page::DNS_PROBE_STARTED, last_status_sent());
FinishProbe(error_page::DNS_PROBE_FINISHED_NXDOMAIN);
EXPECT_FALSE(probe_running());
EXPECT_EQ(2, sent_count());
- EXPECT_EQ(error_page::DNS_PROBE_FINISHED_NXDOMAIN, last_status_sent());
- StartProvisionalLoad(MAIN_FRAME, NORMAL_PAGE);
- FailProvisionalLoad(MAIN_FRAME, DNS_ERROR);
- EXPECT_TRUE(probe_running());
- EXPECT_EQ(2, sent_count());
-
- StartProvisionalLoad(MAIN_FRAME, ERROR_PAGE);
- CommitProvisionalLoad(MAIN_FRAME);
+ DidFinishNavigation(MAIN_FRAME, DNS_ERROR);
EXPECT_TRUE(probe_running());
EXPECT_EQ(3, sent_count());
- EXPECT_EQ(error_page::DNS_PROBE_STARTED, last_status_sent());
FinishProbe(error_page::DNS_PROBE_FINISHED_NO_INTERNET);
EXPECT_FALSE(probe_running());
EXPECT_EQ(4, sent_count());
+}
+
+TEST_F(NetErrorTabHelperTest, MultipleDnsErrorsWithProbesAndErrorPages) {
+ DidFinishNavigation(MAIN_FRAME, DNS_ERROR);
+ EXPECT_TRUE(probe_running());
+ EXPECT_EQ(1, sent_count());
+
+ DidFinishNavigation(MAIN_FRAME, DNS_ERROR);
+ EXPECT_TRUE(probe_running());
+ EXPECT_EQ(2, sent_count());
+ EXPECT_EQ(error_page::DNS_PROBE_STARTED, last_status_sent());
+
+ FinishProbe(error_page::DNS_PROBE_FINISHED_NXDOMAIN);
+ EXPECT_FALSE(probe_running());
+ EXPECT_EQ(3, sent_count());
+ EXPECT_EQ(error_page::DNS_PROBE_FINISHED_NXDOMAIN, last_status_sent());
+
+ DidFinishNavigation(MAIN_FRAME, DNS_ERROR);
+ EXPECT_TRUE(probe_running());
+ EXPECT_EQ(4, sent_count());
+
+ DidFinishNavigation(MAIN_FRAME, DNS_ERROR);
+ EXPECT_TRUE(probe_running());
+ EXPECT_EQ(5, sent_count());
+ EXPECT_EQ(error_page::DNS_PROBE_STARTED, last_status_sent());
+
+ FinishProbe(error_page::DNS_PROBE_FINISHED_NO_INTERNET);
+ EXPECT_FALSE(probe_running());
+ EXPECT_EQ(6, sent_count());
EXPECT_EQ(error_page::DNS_PROBE_FINISHED_NO_INTERNET,
last_status_sent());
}
@@ -394,33 +263,25 @@
// If multiple DNS errors occur in a row before a probe result, don't start
// multiple probes.
TEST_F(NetErrorTabHelperTest, CoalesceFailures) {
- StartProvisionalLoad(MAIN_FRAME, NORMAL_PAGE);
- FailProvisionalLoad(MAIN_FRAME, DNS_ERROR);
- StartProvisionalLoad(MAIN_FRAME, ERROR_PAGE);
- CommitProvisionalLoad(MAIN_FRAME);
- EXPECT_TRUE(probe_running());
- EXPECT_EQ(1, sent_count());
- EXPECT_EQ(error_page::DNS_PROBE_STARTED, last_status_sent());
-
- StartProvisionalLoad(MAIN_FRAME, NORMAL_PAGE);
- FailProvisionalLoad(MAIN_FRAME, DNS_ERROR);
- StartProvisionalLoad(MAIN_FRAME, ERROR_PAGE);
- CommitProvisionalLoad(MAIN_FRAME);
+ DidFinishNavigation(MAIN_FRAME, DNS_ERROR);
+ DidFinishNavigation(MAIN_FRAME, DNS_ERROR);
EXPECT_TRUE(probe_running());
EXPECT_EQ(2, sent_count());
EXPECT_EQ(error_page::DNS_PROBE_STARTED, last_status_sent());
- StartProvisionalLoad(MAIN_FRAME, NORMAL_PAGE);
- FailProvisionalLoad(MAIN_FRAME, DNS_ERROR);
- StartProvisionalLoad(MAIN_FRAME, ERROR_PAGE);
- CommitProvisionalLoad(MAIN_FRAME);
+ DidFinishNavigation(MAIN_FRAME, DNS_ERROR);
EXPECT_TRUE(probe_running());
EXPECT_EQ(3, sent_count());
EXPECT_EQ(error_page::DNS_PROBE_STARTED, last_status_sent());
+ DidFinishNavigation(MAIN_FRAME, DNS_ERROR);
+ EXPECT_TRUE(probe_running());
+ EXPECT_EQ(4, sent_count());
+ EXPECT_EQ(error_page::DNS_PROBE_STARTED, last_status_sent());
+
FinishProbe(error_page::DNS_PROBE_FINISHED_NXDOMAIN);
EXPECT_FALSE(probe_running());
- EXPECT_EQ(4, sent_count());
+ EXPECT_EQ(5, sent_count());
EXPECT_EQ(error_page::DNS_PROBE_FINISHED_NXDOMAIN, last_status_sent());
}
diff --git a/content/browser/frame_host/navigation_handle_impl.cc b/content/browser/frame_host/navigation_handle_impl.cc
index b95c691e..cd75c0f 100644
--- a/content/browser/frame_host/navigation_handle_impl.cc
+++ b/content/browser/frame_host/navigation_handle_impl.cc
@@ -18,6 +18,7 @@
#include "content/public/browser/content_browser_client.h"
#include "content/public/common/browser_side_navigation_policy.h"
#include "content/public/common/content_client.h"
+#include "content/public/common/url_constants.h"
#include "net/url_request/redirect_info.h"
#include "url/gurl.h"
#include "url/url_constants.h"
@@ -461,7 +462,14 @@
render_frame_host_ = render_frame_host;
is_same_page_ = same_page;
- state_ = net_error_code_ == net::OK ? DID_COMMIT : DID_COMMIT_ERROR_PAGE;
+ // If an error page reloads, net_error_code might be 200 but we still want to
+ // count it as an error page.
+ if (params.base_url.spec() == kUnreachableWebDataURL ||
+ net_error_code_ != net::OK) {
+ state_ = DID_COMMIT_ERROR_PAGE;
+ } else {
+ state_ = DID_COMMIT;
+ }
}
NavigationThrottle::ThrottleCheckResult
diff --git a/content/public/browser/navigation_handle.cc b/content/public/browser/navigation_handle.cc
index 723593a..c8d2496 100644
--- a/content/public/browser/navigation_handle.cc
+++ b/content/public/browser/navigation_handle.cc
@@ -24,7 +24,9 @@
std::unique_ptr<NavigationHandle>
NavigationHandle::CreateNavigationHandleForTesting(
const GURL& url,
- RenderFrameHost* render_frame_host) {
+ RenderFrameHost* render_frame_host,
+ bool committed,
+ net::Error error) {
std::unique_ptr<NavigationHandleImpl> handle_impl =
NavigationHandleImpl::Create(
url, static_cast<RenderFrameHostImpl*>(render_frame_host)
@@ -33,6 +35,12 @@
false, // is_synchronous
false, // is_srcdoc
base::TimeTicks::Now(), 0);
+ handle_impl->set_render_frame_host(
+ static_cast<RenderFrameHostImpl*>(render_frame_host));
+ if (error != net::OK)
+ handle_impl->set_net_error_code(error);
+ if (committed)
+ handle_impl->CallDidCommitNavigationForTesting(url);
return std::unique_ptr<NavigationHandle>(std::move(handle_impl));
}
diff --git a/content/public/browser/navigation_handle.h b/content/public/browser/navigation_handle.h
index 3118ec6..3c00e4a 100644
--- a/content/public/browser/navigation_handle.h
+++ b/content/public/browser/navigation_handle.h
@@ -152,6 +152,8 @@
virtual bool HasCommitted() = 0;
// Whether the navigation resulted in an error page.
+ // Note that if an error page reloads, this will return true even though
+ // GetNetErrorCode will be net::OK.
virtual bool IsErrorPage() = 0;
// Returns the response headers for the request or nullptr if there are none.
@@ -175,7 +177,9 @@
static std::unique_ptr<NavigationHandle> CreateNavigationHandleForTesting(
const GURL& url,
- RenderFrameHost* render_frame_host);
+ RenderFrameHost* render_frame_host,
+ bool committed = false,
+ net::Error error = net::OK);
// Registers a NavigationThrottle for tests. The throttle can
// modify the request, pause the request or cancel the request. This will
diff --git a/content/test/web_contents_observer_sanity_checker.cc b/content/test/web_contents_observer_sanity_checker.cc
index e105985..1d85380f 100644
--- a/content/test/web_contents_observer_sanity_checker.cc
+++ b/content/test/web_contents_observer_sanity_checker.cc
@@ -201,9 +201,6 @@
CHECK(!(navigation_handle->HasCommitted() &&
!navigation_handle->IsErrorPage()) ||
navigation_handle->GetNetErrorCode() == net::OK);
- CHECK(!(navigation_handle->HasCommitted() &&
- navigation_handle->IsErrorPage()) ||
- navigation_handle->GetNetErrorCode() != net::OK);
CHECK_EQ(navigation_handle->GetWebContents(), web_contents());
CHECK(!navigation_handle->HasCommitted() ||
