Move alternate error page loading out of WebFrame.

Make the RenderView be in charge of loading alternate error pages.

While working on this change, I noticed several related bugs:

1-  Loading an URL with an invalid host name from the new tab page results in
an error page.  If you hit back and then forward, you will be left with an
empty location bar.  In a debug build this trips an assertion in
ClassifyNavigation because the given page_id is -1.  This problem is caused by
not duplicating the NavigationState of the failed load when creating a load for
the error page.  Hence, the pending_page_id of the forward navigation is lost.

2-  Loading an URL with an invalid host name as a subframe results in an extra
navigation in session history.  One navigation for the main frame and one
navigation for the error page load.  This is another symptom of the problem
described in #1.  However, the solution is different.  Here, we need to know
that the subframe load is an AUTO_SUBFRAME load so that we load the error page
using 'replace' semantics (so that WebCore does not generate a new session
history entry).

Finally, I decided to restrict alternative DNS error pages to only work for the
main frame to match what we do for alternative 404 error pages.  It doesn't seem
worth it to show link doctor results for subframes since their primary purpose
is to assist people who mis-type an URL.

R=tony,brettw
BUG=15648
TEST=covered by errorpage_uitest.cc

Review URL: http://codereview.chromium.org/159575

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@22253 0039d316-1c4b-4281-b951-d872f2087c98

13 files changed, 280 insertions, 155 deletions

diff --git a/chrome/browser/errorpage_uitest.cc b/chrome/browser/errorpage_uitest.cc
index 9b2c489..b72308c 100644
--- a/chrome/browser/errorpage_uitest.cc
+++ b/chrome/browser/errorpage_uitest.cc
@@ -3,35 +3,141 @@
 // found in the LICENSE file.
 
 #include "base/string_util.h"
+#include "chrome/test/automation/tab_proxy.h"
 #include "chrome/test/ui/ui_test.h"
 #include "chrome/browser/net/url_request_failed_dns_job.h"
+#include "chrome/browser/net/url_request_mock_http_job.h"
 #include "net/url_request/url_request_unittest.h"
 
 class ErrorPageTest : public UITest {
+ protected:
+  bool WaitForTitleMatching(const std::wstring& title) {
+    for (int i = 0; i < 100; ++i) {
+      if (GetActiveTabTitle() == title)
+        return true;
+      PlatformThread::Sleep(sleep_timeout_ms() / 10);
+    }
+    return false;
+  }
+  bool WaitForTitleContaining(const std::string& title_substring) {
+    for (int i = 0; i < 100; ++i) {
+      std::wstring title = GetActiveTabTitle();
+      if (title.find(UTF8ToWide(title_substring)) != std::wstring::npos)
+        return true;
+      PlatformThread::Sleep(sleep_timeout_ms() / 10);
+    }
+    return false;
+  }
 };
 
-TEST_F(ErrorPageTest, DNSError) {
+TEST_F(ErrorPageTest, DNSError_Basic) {
   GURL test_url(URLRequestFailedDnsJob::kTestUrl);
-  std::wstring test_host = UTF8ToWide(test_url.host());
+
   NavigateToURL(test_url);
 
-  // Verify that the url is in the title.  Since it's set via Javascript, we
-  // need to give it a chance to run.
-  int i;
-  std::wstring title;
-  for (i = 0; i < 10; ++i) {
-    PlatformThread::Sleep(sleep_timeout_ms());
-    title = GetActiveTabTitle();
-    if (title.find(test_host) != std::wstring::npos) {
-      // Success, bail out.
-      break;
-    }
-  }
+  EXPECT_TRUE(WaitForTitleContaining(test_url.host()));
+}
 
-  if (i == 10) {
-    FAIL() << "failed to get error page title; got " << title;
-  }
-};
+TEST_F(ErrorPageTest, DNSError_GoBack1) {
+  // Test that a DNS error occuring in the main frame does not result in an
+  // additional session history entry.
+  GURL test_url(URLRequestFailedDnsJob::kTestUrl);
+
+  NavigateToURL(URLRequestMockHTTPJob::GetMockUrl(L"title2.html"));
+  NavigateToURL(test_url);
+  EXPECT_TRUE(WaitForTitleContaining(test_url.host()));
+
+  GetActiveTab()->GoBack();
+
+  EXPECT_TRUE(WaitForTitleMatching(L"Title Of Awesomeness"));
+}
+
+TEST_F(ErrorPageTest, DNSError_GoBack2) {
+  // Test that a DNS error occuring in the main frame does not result in an
+  // additional session history entry.
+  GURL test_url(URLRequestFailedDnsJob::kTestUrl);
+
+  NavigateToURL(URLRequestMockHTTPJob::GetMockUrl(L"title2.html"));
+  NavigateToURL(test_url);
+  EXPECT_TRUE(WaitForTitleContaining(test_url.host()));
+  NavigateToURL(URLRequestMockHTTPJob::GetMockUrl(L"title3.html"));
+
+  GetActiveTab()->GoBack();
+  EXPECT_TRUE(WaitForTitleContaining(test_url.host()));
+  GetActiveTab()->GoBack();
+
+  EXPECT_TRUE(WaitForTitleMatching(L"Title Of Awesomeness"));
+}
+
+TEST_F(ErrorPageTest, DNSError_GoBack2AndForward) {
+  // Test that a DNS error occuring in the main frame does not result in an
+  // additional session history entry.
+
+  GURL test_url(URLRequestFailedDnsJob::kTestUrl);
+
+  NavigateToURL(URLRequestMockHTTPJob::GetMockUrl(L"title2.html"));
+  NavigateToURL(test_url);
+  EXPECT_TRUE(WaitForTitleContaining(test_url.host()));
+  NavigateToURL(URLRequestMockHTTPJob::GetMockUrl(L"title3.html"));
+
+  GetActiveTab()->GoBack();
+  EXPECT_TRUE(WaitForTitleContaining(test_url.host()));
+  GetActiveTab()->GoBack();
+  GetActiveTab()->GoForward();
+
+  EXPECT_TRUE(WaitForTitleContaining(test_url.host()));
+}
+
+TEST_F(ErrorPageTest, DNSError_GoBack2Forward2) {
+  // Test that a DNS error occuring in the main frame does not result in an
+  // additional session history entry.
+
+  GURL test_url(URLRequestFailedDnsJob::kTestUrl);
+
+  NavigateToURL(URLRequestMockHTTPJob::GetMockUrl(L"title3.html"));
+  NavigateToURL(test_url);
+  EXPECT_TRUE(WaitForTitleContaining(test_url.host()));
+  NavigateToURL(URLRequestMockHTTPJob::GetMockUrl(L"title2.html"));
+
+  GetActiveTab()->GoBack();
+  EXPECT_TRUE(WaitForTitleContaining(test_url.host()));
+  GetActiveTab()->GoBack();
+  GetActiveTab()->GoForward();
+  EXPECT_TRUE(WaitForTitleContaining(test_url.host()));
+  GetActiveTab()->GoForward();
+
+  EXPECT_TRUE(WaitForTitleMatching(L"Title Of Awesomeness"));
+}
+
+TEST_F(ErrorPageTest, IFrameDNSError_Basic) {
+  NavigateToURL(URLRequestMockHTTPJob::GetMockUrl(L"iframe_dns_error.html"));
+  EXPECT_TRUE(WaitForTitleMatching(L"Blah"));
+}
+
+TEST_F(ErrorPageTest, IFrameDNSError_GoBack) {
+  // Test that a DNS error occuring in an iframe does not result in an
+  // additional session history entry.
+
+  NavigateToURL(URLRequestMockHTTPJob::GetMockUrl(L"title2.html"));
+  NavigateToURL(URLRequestMockHTTPJob::GetMockUrl(L"iframe_dns_error.html"));
+
+  GetActiveTab()->GoBack();
+
+  EXPECT_TRUE(WaitForTitleMatching(L"Title Of Awesomeness"));
+}
+
+TEST_F(ErrorPageTest, IFrameDNSError_GoBackAndForward) {
+  // Test that a DNS error occuring in an iframe does not result in an
+  // additional session history entry.
+
+  NavigateToURL(URLRequestMockHTTPJob::GetMockUrl(L"title2.html"));
+  NavigateToURL(URLRequestMockHTTPJob::GetMockUrl(L"iframe_dns_error.html"));
+
+  GetActiveTab()->GoBack();
+  GetActiveTab()->GoForward();
+
+  EXPECT_TRUE(WaitForTitleMatching(L"Blah"));
+}
 
 TEST_F(ErrorPageTest, IFrame404) {
   // iframes that have 404 pages should not trigger an alternate error page.
@@ -44,20 +150,5 @@ TEST_F(ErrorPageTest, IFrame404) {
   GURL test_url = server->TestServerPage("files/iframe404.html");
   NavigateToURL(test_url);
 
-  // Verify that the url is in the title.  Since it's set via Javascript, we
-  // need to give it a chance to run.
-  int i;
-  std::wstring title;
-  for (i = 0; i < 10; ++i) {
-    PlatformThread::Sleep(sleep_timeout_ms());
-    title = GetActiveTabTitle();
-    if (title == L"SUCCESS") {
-      // Success, bail out.
-      break;
-    }
-  }
-
-  if (i == 10) {
-    FAIL() << "iframe 404 didn't load properly";
-  }
-};
+  EXPECT_TRUE(WaitForTitleMatching(L"SUCCESS"));
+}
diff --git a/chrome/renderer/navigation_state.h b/chrome/renderer/navigation_state.h
index 771ef55..ec45808 100644
--- a/chrome/renderer/navigation_state.h
+++ b/chrome/renderer/navigation_state.h
@@ -9,6 +9,7 @@
 #include "base/time.h"
 #include "chrome/common/page_transition_types.h"
 #include "webkit/api/public/WebDataSource.h"
+#include "webkit/glue/alt_error_page_resource_fetcher.h"
 #include "webkit/glue/password_form.h"
 #include "webkit/glue/searchable_form_data.h"
 
@@ -36,9 +37,6 @@ class NavigationState : public WebKit::WebDataSource::ExtraData {
   // Contains the page_id for this navigation or -1 if there is none yet.
   int32 pending_page_id() const { return pending_page_id_; }
 
-  // Is this a new navigation?
-  bool is_new_navigation() const { return pending_page_id_ == -1; }
-
   // Contains the transition type that the browser specified when it
   // initiated the load.
   PageTransition::Type transition_type() const { return transition_type_; }
@@ -132,6 +130,13 @@ class NavigationState : public WebKit::WebDataSource::ExtraData {
     password_form_data_.reset(data);
   }
 
+  webkit_glue::AltErrorPageResourceFetcher* alt_error_page_fetcher() const {
+    return alt_error_page_fetcher_.get();
+  }
+  void set_alt_error_page_fetcher(webkit_glue::AltErrorPageResourceFetcher* f) {
+    alt_error_page_fetcher_.reset(f);
+  }
+
   const std::string& security_info() const {
     return security_info_;
   }
@@ -166,6 +171,7 @@ class NavigationState : public WebKit::WebDataSource::ExtraData {
   int32 pending_page_id_;
   scoped_ptr<webkit_glue::SearchableFormData> searchable_form_data_;
   scoped_ptr<webkit_glue::PasswordForm> password_form_data_;
+  scoped_ptr<webkit_glue::AltErrorPageResourceFetcher> alt_error_page_fetcher_;
   std::string security_info_;
 
   DISALLOW_COPY_AND_ASSIGN(NavigationState);
diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index 2c805ac..91126bd 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -98,6 +98,7 @@
 
 using base::Time;
 using base::TimeDelta;
+using webkit_glue::AltErrorPageResourceFetcher;
 using webkit_glue::AutofillForm;
 using webkit_glue::PasswordForm;
 using webkit_glue::PasswordFormDomManager;
@@ -1140,6 +1141,10 @@ void RenderView::DidStartProvisionalLoadForFrame(
 
     // Make sure redirect tracking state is clear for the new load.
     completed_client_redirect_src_ = GURL();
+  } else if (frame->GetParent()->IsLoading()) {
+    // Take note of AUTO_SUBFRAME loads here, so that we can know how to
+    // load an error page.  See DidFailProvisionalLoadWithError.
+    navigation_state->set_transition_type(PageTransition::AUTO_SUBFRAME);
   }
 
   Send(new ViewHostMsg_DidStartProvisionalLoadForFrame(
@@ -1211,33 +1216,33 @@ void RenderView::DidFailProvisionalLoadWithError(WebView* webview,
   // Make sure we never show errors in view source mode.
   frame->SetInViewSourceMode(false);
 
+  NavigationState* navigation_state = NavigationState::FromDataSource(ds);
+
   // If this is a failed back/forward/reload navigation, then we need to do a
   // 'replace' load.  This is necessary to avoid messing up session history.
   // Otherwise, we do a normal load, which simulates a 'go' navigation as far
   // as session history is concerned.
-  bool replace = !NavigationState::FromDataSource(ds)->is_new_navigation();
+  //
+  // AUTO_SUBFRAME loads should always be treated as loads that do not advance
+  // the page id.
+  //
+  bool replace =
+      navigation_state->pending_page_id() != -1 ||
+      navigation_state->transition_type() == PageTransition::AUTO_SUBFRAME;
 
-  // Use the alternate error page service if this is a DNS failure or
-  // connection failure.  ERR_CONNECTION_FAILED can be dropped once we no longer
-  // use winhttp.
-  int ec = error.reason;
-  if (ec == net::ERR_NAME_NOT_RESOLVED ||
-      ec == net::ERR_CONNECTION_FAILED ||
-      ec == net::ERR_CONNECTION_REFUSED ||
-      ec == net::ERR_ADDRESS_UNREACHABLE ||
-      ec == net::ERR_TIMED_OUT) {
-    const GURL& failed_url = error.unreachableURL;
-    const GURL& error_page_url = GetAlternateErrorPageURL(failed_url,
-        ec == net::ERR_NAME_NOT_RESOLVED ? WebViewDelegate::DNS_ERROR
-                                         : WebViewDelegate::CONNECTION_ERROR);
-    if (error_page_url.is_valid()) {
-      // Ask the WebFrame to fetch the alternate error page for us.
-      frame->LoadAlternateHTMLErrorPage(failed_request, error, error_page_url,
-          replace, GURL(kUnreachableWebDataURL));
-      return;
-    }
+  // If we failed on a browser initiated request, then make sure that our error
+  // page load is regarded as the same browser initiated request.
+  if (!navigation_state->is_content_initiated()) {
+    pending_navigation_state_.reset(NavigationState::CreateBrowserInitiated(
+        navigation_state->pending_page_id(),
+        navigation_state->transition_type(),
+        navigation_state->request_time()));
   }
 
+  // Provide the user with a more helpful error page?
+  if (MaybeLoadAlternateErrorPage(frame, error, replace))
+    return;
+
   // Fallback to a local error page.
   LoadNavigationErrorPage(frame, failed_request, error, std::string(),
                           replace);
@@ -1297,19 +1302,19 @@ void RenderView::DidCommitLoadForFrame(WebView *webview, WebFrame* frame,
                                           page_id_, true),
         kDelayForForcedCaptureMs);
   } else {
-    // Inspect the navigation_state on the main frame (set in our Navigate
-    // method) to see if the navigation corresponds to a session history
-    // navigation...  Note: |frame| may or may not be the toplevel frame, but
-    // for the case of capturing session history, the first committed frame
-    // suffices.  We keep track of whether we've seen this commit before so
-    // that only capture session history once per navigation.
+    // Inspect the navigation_state on this frame to see if the navigation
+    // corresponds to a session history navigation...  Note: |frame| may or
+    // may not be the toplevel frame, but for the case of capturing session
+    // history, the first committed frame suffices.  We keep track of whether
+    // we've seen this commit before so that only capture session history once
+    // per navigation.
     //
     // Note that we need to check if the page ID changed. In the case of a
     // reload, the page ID doesn't change, and UpdateSessionHistory gets the
     // previous URL and the current page ID, which would be wrong.
-    if (!navigation_state->is_new_navigation() &&
-        !navigation_state->request_committed() &&
-        page_id_ != navigation_state->pending_page_id()) {
+    if (navigation_state->pending_page_id() != -1 &&
+        navigation_state->pending_page_id() != page_id_ &&
+        !navigation_state->request_committed()) {
       // This is a successful session history navigation!
       UpdateSessionHistory(frame);
       page_id_ = navigation_state->pending_page_id();
@@ -2827,6 +2832,53 @@ void RenderView::OnDisassociateFromPopupCount() {
   decrement_shared_popup_at_destruction_ = false;
 }
 
+bool RenderView::MaybeLoadAlternateErrorPage(WebFrame* frame,
+                                             const WebURLError& error,
+                                             bool replace) {
+  // We only show alternate error pages in the main frame.  They are
+  // intended to assist the user when navigating, so there is not much
+  // value in showing them for failed subframes.  Ideally, we would be
+  // able to use the TYPED transition type for this, but that flag is
+  // not preserved across page reloads.
+  if (frame->GetParent())
+    return false;
+
+  // Use the alternate error page service if this is a DNS failure or
+  // connection failure.  ERR_CONNECTION_FAILED can be dropped once we no
+  // longer use winhttp.
+  int ec = error.reason;
+  if (ec != net::ERR_NAME_NOT_RESOLVED &&
+      ec != net::ERR_CONNECTION_FAILED &&
+      ec != net::ERR_CONNECTION_REFUSED &&
+      ec != net::ERR_ADDRESS_UNREACHABLE &&
+      ec != net::ERR_TIMED_OUT)
+    return false;
+
+  const GURL& error_page_url = GetAlternateErrorPageURL(error.unreachableURL,
+      ec == net::ERR_NAME_NOT_RESOLVED ? WebViewDelegate::DNS_ERROR
+                                       : WebViewDelegate::CONNECTION_ERROR);
+  if (!error_page_url.is_valid())
+    return false;
+
+  // Load an empty page first so there is an immediate response to the error,
+  // and then kick off a request for the alternate error page.
+  frame->LoadHTMLString(std::string(),
+                        GURL(kUnreachableWebDataURL),
+                        error.unreachableURL,
+                        replace);
+
+  // Now, create a fetcher for the error page and associate it with the data
+  // source we just created via the LoadHTMLString call.  That way if another
+  // navigation occurs, the fetcher will get destroyed.
+  NavigationState* navigation_state =
+      NavigationState::FromDataSource(frame->GetProvisionalDataSource());
+  navigation_state->set_alt_error_page_fetcher(
+      new AltErrorPageResourceFetcher(
+          error_page_url, frame, error,
+          NewCallback(this, &RenderView::AltErrorPageFinished)));
+  return true;
+}
+
 std::string RenderView::GetAltHTMLForTemplate(
     const DictionaryValue& error_strings, int template_resource_id) const {
   const StringPiece template_html(
@@ -2843,6 +2895,13 @@ std::string RenderView::GetAltHTMLForTemplate(
       template_html, &error_strings, "t");
 }
 
+void RenderView::AltErrorPageFinished(WebFrame* frame,
+                                      const WebURLError& original_error,
+                                      const std::string& html) {
+  // Here, we replace the blank page we loaded previously.
+  LoadNavigationErrorPage(frame, WebURLRequest(), original_error, html, true);
+}
+
 void RenderView::OnMoveOrResizeStarted() {
   if (webview())
     webview()->HideAutofillPopup();
diff --git a/chrome/renderer/render_view.h b/chrome/renderer/render_view.h
index 91c69039..37c40a0 100644
--- a/chrome/renderer/render_view.h
+++ b/chrome/renderer/render_view.h
@@ -609,8 +609,14 @@ class RenderView : public RenderWidget,
   // Locates a sub frame with given xpath
   WebFrame* GetChildFrame(const std::wstring& frame_xpath) const;
 
-  std::string GetAltHTMLForTemplate(const DictionaryValue& error_strings,
-                                    int template_resource_id) const;
+  // Alternate error page helpers.
+  bool MaybeLoadAlternateErrorPage(
+      WebFrame* frame, const WebKit::WebURLError& error, bool replace);
+  std::string GetAltHTMLForTemplate(
+      const DictionaryValue& error_strings, int template_resource_id) const;
+  void AltErrorPageFinished(
+      WebFrame* frame, const WebKit::WebURLError& original_error,
+      const std::string& html);
 
   virtual void DidAddHistoryItem();
 
diff --git a/chrome/test/data/iframe_dns_error.html b/chrome/test/data/iframe_dns_error.html
new file mode 100644
index 0000000..8a68349
--- /dev/null
+++ b/chrome/test/data/iframe_dns_error.html
@@ -0,0 +1,9 @@
+<html>
+<head>
+<title>Blah</title>
+</head>
+<body>
+This frame will fail to load with a DNS error:
+<iframe src="http://url.handled.by.fake.dns/"></iframe>
+</body>
+</html>
diff --git a/chrome/test/data/title3.html b/chrome/test/data/title3.html
new file mode 100644
index 0000000..b340fdb
--- /dev/null
+++ b/chrome/test/data/title3.html
@@ -0,0 +1,4 @@
+<html>
+<head><title>Title Of More Awesomeness</title></head>
+<body>This page has a title.</body>
+</html>
diff --git a/webkit/glue/alt_error_page_resource_fetcher.cc b/webkit/glue/alt_error_page_resource_fetcher.cc
index 5d1a3dc..6c8bad9 100644
--- a/webkit/glue/alt_error_page_resource_fetcher.cc
+++ b/webkit/glue/alt_error_page_resource_fetcher.cc
@@ -18,10 +18,11 @@ static const int kDownloadTimeoutSec = 3;
 AltErrorPageResourceFetcher::AltErrorPageResourceFetcher(
     const GURL& url,
     WebFrame* frame,
-    const GURL& unreachable_url,
+    const WebURLError& original_error,
     Callback* callback)
-    : callback_(callback),
-      unreachable_url_(unreachable_url) {
+    : frame_(frame),
+      callback_(callback),
+      original_error_(original_error) {
   fetcher_.reset(new ResourceFetcherWithTimeout(
       url, frame, kDownloadTimeoutSec,
       NewCallback(this, &AltErrorPageResourceFetcher::OnURLFetchComplete)));
@@ -39,9 +40,9 @@ void AltErrorPageResourceFetcher::OnURLFetchComplete(
     const std::string& data) {
   // A null response indicates a network error.
   if (!response.isNull() && response.httpStatusCode() == 200) {
-    callback_->Run(unreachable_url_, data);
+    callback_->Run(frame_, original_error_, data);
   } else {
-    callback_->Run(unreachable_url_, std::string());
+    callback_->Run(frame_, original_error_, std::string());
   }
 }
 
diff --git a/webkit/glue/alt_error_page_resource_fetcher.h b/webkit/glue/alt_error_page_resource_fetcher.h
index 3d50972..d591e02 100644
--- a/webkit/glue/alt_error_page_resource_fetcher.h
+++ b/webkit/glue/alt_error_page_resource_fetcher.h
@@ -8,6 +8,7 @@
 #include "base/scoped_ptr.h"
 #include "base/task.h"
 #include "googleurl/src/gurl.h"
+#include "webkit/api/public/WebURLError.h"
 
 class WebFrame;
 
@@ -16,7 +17,6 @@ class WebURLResponse;
 }
 
 namespace webkit_glue {
-
 class ResourceFetcherWithTimeout;
 
 // Used for downloading alternate dns error pages. Once downloading is done
@@ -24,13 +24,14 @@ class ResourceFetcherWithTimeout;
 class AltErrorPageResourceFetcher {
  public:
   // This will be called when the alternative error page has been fetched,
-  // successfully or not.  If there is a failure, the second parameter (the
+  // successfully or not.  If there is a failure, the third parameter (the
   // data) will be empty.
-  typedef Callback2<const GURL&, const std::string&>::Type Callback;
+  typedef Callback3<
+      WebFrame*, const WebKit::WebURLError&, const std::string&>::Type Callback;
 
   AltErrorPageResourceFetcher(const GURL& url,
                               WebFrame* frame,
-                              const GURL& unreachable_url,
+                              const WebKit::WebURLError& original_error,
                               Callback* callback);
   ~AltErrorPageResourceFetcher();
 
@@ -44,12 +45,12 @@ class AltErrorPageResourceFetcher {
   // Does the actual fetching.
   scoped_ptr<ResourceFetcherWithTimeout> fetcher_;
 
+  WebFrame* frame_;
   scoped_ptr<Callback> callback_;
 
-  // The "unreachable url" associated with this load.  If there's an error
-  // talking with the alt 404 page server, we need this to complete the
-  // original load.
-  GURL unreachable_url_;
+  // The error associated with this load.  If there's an error talking with the
+  // alt error page server, we need this to complete the original load.
+  WebKit::WebURLError original_error_;
 
   DISALLOW_COPY_AND_ASSIGN(AltErrorPageResourceFetcher);
 };
diff --git a/webkit/glue/webframe.h b/webkit/glue/webframe.h
index fc09458..554bf90 100644
--- a/webkit/glue/webframe.h
+++ b/webkit/glue/webframe.h
@@ -118,16 +118,6 @@ class WebFrame {
       const WebKit::WebURL& unreachable_url = WebKit::WebURL(),
       bool replace = false) = 0;
 
-  // Asks the WebFrame to try and download the alternate error page.  We notify
-  // the WebViewDelegate of the results so it can decide whether or not to show
-  // something to the user (e.g., a local error page or the alternate error
-  // page).
-  virtual void LoadAlternateHTMLErrorPage(const WebKit::WebURLRequest& request,
-                                          const WebKit::WebURLError& error,
-                                          const GURL& error_page_url,
-                                          bool replace,
-                                          const GURL& fake_url) = 0;
-
   // Called to associate the WebURLRequest with this frame.  The request will
   // be modified to inherit parameters that allow it to be loaded.  This method
   // ends up triggering WebViewDelegate::WillSendRequest.
@@ -213,6 +203,9 @@ class WebFrame {
   //  - (void)stopLoading;
   virtual void StopLoading() = 0;
 
+  // Returns true if this frame is loading its main resource or a subresource.
+  virtual bool IsLoading() const = 0;
+
   // Returns the frame that opened this frame, or NULL if this window has no
   // opener.
   virtual WebFrame* GetOpener() const = 0;
diff --git a/webkit/glue/webframe_impl.cc b/webkit/glue/webframe_impl.cc
index eed9443..3564e45 100644
--- a/webkit/glue/webframe_impl.cc
+++ b/webkit/glue/webframe_impl.cc
@@ -152,7 +152,6 @@ MSVC_POP_WARNING();
 #include "webkit/api/public/WebScriptSource.h"
 #include "webkit/api/public/WebSize.h"
 #include "webkit/api/public/WebURLError.h"
-#include "webkit/glue/alt_error_page_resource_fetcher.h"
 #include "webkit/glue/chrome_client_impl.h"
 #include "webkit/glue/dom_operations.h"
 #include "webkit/glue/dom_operations_private.h"
@@ -604,6 +603,12 @@ void WebFrameImpl::StopLoading() {
   frame_->loader()->stopLoading(false);
 }
 
+bool WebFrameImpl::IsLoading() const {
+  if (!frame_)
+    return false;
+  return frame_->loader()->isLoading();
+}
+
 WebFrame* WebFrameImpl::GetOpener() const {
   if (frame_) {
     Frame* opener = frame_->loader()->opener();
@@ -1504,13 +1509,7 @@ void WebFrameImpl::Paint(skia::PlatformCanvas* canvas, const WebRect& rect) {
   }
 }
 
-bool WebFrameImpl::IsLoading() {
-  // I'm assuming this does what we want.
-  return frame_->loader()->isLoading();
-}
-
 void WebFrameImpl::Closing() {
-  alt_error_page_fetcher_.reset();
   frame_ = NULL;
 }
 
@@ -1528,11 +1527,6 @@ void WebFrameImpl::DidReceiveData(DocumentLoader* loader,
 
   // NOTE: mac only does this if there is a document
   frame_->loader()->addData(data, length);
-
-  // It's possible that we get a DNS failure followed by a second load that
-  // succeeds before we hear back from the alternate error page server.  In
-  // that case, cancel the alt error page download.
-  alt_error_page_fetcher_.reset();
 }
 
 void WebFrameImpl::DidFail(const ResourceError& error, bool was_provisional) {
@@ -1549,25 +1543,6 @@ void WebFrameImpl::DidFail(const ResourceError& error, bool was_provisional) {
   }
 }
 
-void WebFrameImpl::LoadAlternateHTMLErrorPage(const WebURLRequest& request,
-                                              const WebURLError& error,
-                                              const GURL& error_page_url,
-                                              bool replace,
-                                              const GURL& fake_url) {
-  // Load alternate HTML in place of the previous request.  We use a fake_url
-  // for the Base URL.  That prevents other web content from the same origin
-  // as the failed URL to script the error page.
-
-  LoadHTMLString("",  // Empty document
-                 fake_url,
-                 error.unreachableURL,
-                 replace);
-
-  alt_error_page_fetcher_.reset(new AltErrorPageResourceFetcher(
-      error_page_url, this, error.unreachableURL,
-      NewCallback(this, &WebFrameImpl::AltErrorPageFinished)));
-}
-
 void WebFrameImpl::DispatchWillSendRequest(WebURLRequest* request) {
   ResourceResponse response;
   frame_->loader()->client()->dispatchWillSendRequest(NULL, 0,
@@ -1847,13 +1822,3 @@ void WebFrameImpl::LoadJavaScriptURL(const KURL& url) {
     frame_->loader()->end();
   }
 }
-
-void WebFrameImpl::AltErrorPageFinished(const GURL& unreachable_url,
-                                        const std::string& html) {
-  WebViewDelegate* d = GetWebViewImpl()->delegate();
-  if (!d)
-    return;
-  WebURLError error;
-  error.unreachableURL = unreachable_url;
-  d->LoadNavigationErrorPage(this, WebURLRequest(), error, html, true);
-}
diff --git a/webkit/glue/webframe_impl.h b/webkit/glue/webframe_impl.h
index d7f0c1d..79f2f01 100644
--- a/webkit/glue/webframe_impl.h
+++ b/webkit/glue/webframe_impl.h
@@ -62,10 +62,6 @@ class SubstituteData;
 struct WindowFeatures;
 }
 
-namespace webkit_glue {
-class AltErrorPageResourceFetcher;
-}
-
 // Implementation of WebFrame, note that this is a reference counted object.
 class WebFrameImpl : public WebFrame, public base::RefCounted<WebFrameImpl> {
  public:
@@ -95,12 +91,6 @@ class WebFrameImpl : public WebFrame, public base::RefCounted<WebFrameImpl> {
       const WebKit::WebURL& base_url,
       const WebKit::WebURL& unreachable_url = WebKit::WebURL(),
       bool replace = false);
-  virtual void LoadAlternateHTMLErrorPage(
-      const WebKit::WebURLRequest& request,
-      const WebKit::WebURLError& error,
-      const GURL& error_page_url,
-      bool replace,
-      const GURL& fake_url);
   virtual void DispatchWillSendRequest(WebKit::WebURLRequest* request);
   virtual void ExecuteScript(const WebKit::WebScriptSource& source);
   virtual void ExecuteScriptInNewContext(
@@ -119,6 +109,7 @@ class WebFrameImpl : public WebFrame, public base::RefCounted<WebFrameImpl> {
   virtual WebKit::WebDataSource* GetDataSource() const;
   virtual WebKit::WebDataSource* GetProvisionalDataSource() const;
   virtual void StopLoading();
+  virtual bool IsLoading() const;
   virtual WebFrame* GetOpener() const;
   virtual WebFrame* GetParent() const;
   virtual WebFrame* GetTop() const;
@@ -208,8 +199,6 @@ class WebFrameImpl : public WebFrame, public base::RefCounted<WebFrameImpl> {
   void Layout();
   void Paint(skia::PlatformCanvas* canvas, const WebKit::WebRect& rect);
 
-  bool IsLoading();
-
   void CreateFrameView();
 
   // The plugin delegate is used to get notifications when downloads complete.
@@ -287,9 +276,6 @@ class WebFrameImpl : public WebFrame, public base::RefCounted<WebFrameImpl> {
                                          int active_match_ordinal,
                                          int request_id);
 
-  // Resource fetcher for downloading an alternate DNS error page.
-  scoped_ptr<webkit_glue::AltErrorPageResourceFetcher> alt_error_page_fetcher_;
-
   // Used to check for leaks of this object.
   static int live_object_count_;
 
diff --git a/webkit/glue/webframeloaderclient_impl.cc b/webkit/glue/webframeloaderclient_impl.cc
index 90f059e..3006fe4 100644
--- a/webkit/glue/webframeloaderclient_impl.cc
+++ b/webkit/glue/webframeloaderclient_impl.cc
@@ -354,8 +354,10 @@ void WebFrameLoaderClient::dispatchDidFinishLoading(DocumentLoader* loader,
     const GURL& url = GetAlt404PageUrl(loader);
     DCHECK(url.is_valid()) <<
         "URL changed? It was valid in dispatchDidReceiveResponse.";
+    WebURLError original_error;
+    original_error.unreachableURL = webkit_glue::KURLToWebURL(loader->url());
     alt_404_page_fetcher_.reset(new AltErrorPageResourceFetcher(
-        url, webframe_, webkit_glue::KURLToGURL(loader->url()),
+        url, webframe_, original_error,
         NewCallback(this, &WebFrameLoaderClient::Alt404PageFinished)));
   }
 
@@ -383,7 +385,8 @@ GURL WebFrameLoaderClient::GetAlt404PageUrl(DocumentLoader* loader) {
   return d->GetAlternateErrorPageURL(failedURL, WebViewDelegate::HTTP_404);
 }
 
-void WebFrameLoaderClient::Alt404PageFinished(const GURL& unreachable_url,
+void WebFrameLoaderClient::Alt404PageFinished(WebFrame* frame,
+                                              const WebURLError& original_error,
                                               const std::string& html) {
   // TODO(darin): Move this processing out to the embedder.
   if (!html.empty()) {
@@ -392,12 +395,11 @@ void WebFrameLoaderClient::Alt404PageFinished(const GURL& unreachable_url,
     WebViewDelegate* d = webframe_->GetWebViewImpl()->delegate();
     if (!d)
       return;
-    WebURLError error;
-    error.unreachableURL = unreachable_url;
-    d->LoadNavigationErrorPage(webframe_, WebURLRequest(), error, html, false);
+    d->LoadNavigationErrorPage(webframe_, WebURLRequest(), original_error, html,
+                               false);
   } else {
     // Fall back on original text
-    webframe_->LoadHTMLString(postponed_data_, unreachable_url);
+    webframe_->LoadHTMLString(postponed_data_, original_error.unreachableURL);
   }
 }
 
diff --git a/webkit/glue/webframeloaderclient_impl.h b/webkit/glue/webframeloaderclient_impl.h
index defa4d2..cdae117 100644
--- a/webkit/glue/webframeloaderclient_impl.h
+++ b/webkit/glue/webframeloaderclient_impl.h
@@ -206,7 +206,9 @@ class WebFrameLoaderClient : public WebCore::FrameLoaderClient {
  private:
   // Callback function for download of alternate 404 pages.  If the server is
   // down or we take too long to download the page, |html| will be empty.
-  void Alt404PageFinished(const GURL& unreachable_url, const std::string& html);
+  void Alt404PageFinished(WebFrame* frame,
+                          const WebKit::WebURLError& original_error,
+                          const std::string& html);
 
   void makeDocumentView();