Make NetErrorHelper more OOPIF-friendly.

Now create one per frame instead of creating them only for main frames,
and get the WebFrame directly from a WebFrame rather than by going
through the RenderView.

BUG=543226,529976

Review URL: https://codereview.chromium.org/1406303002

Cr-Commit-Position: refs/heads/master@{#356395}

13 files changed, 59 insertions, 108 deletions

diff --git a/android_webview/renderer/aw_content_renderer_client.cc b/android_webview/renderer/aw_content_renderer_client.cc
index e560711..1950e18 100644
--- a/android_webview/renderer/aw_content_renderer_client.cc
+++ b/android_webview/renderer/aw_content_renderer_client.cc
@@ -173,8 +173,7 @@ bool AwContentRendererClient::HasErrorPage(int http_status_code,
 }
 
 void AwContentRendererClient::GetNavigationErrorStrings(
-    content::RenderView* /* render_view */,
-    blink::WebFrame* /* frame */,
+    content::RenderFrame* /* render_frame */,
     const blink::WebURLRequest& failed_request,
     const blink::WebURLError& error,
     std::string* error_html,
diff --git a/android_webview/renderer/aw_content_renderer_client.h b/android_webview/renderer/aw_content_renderer_client.h
index f04a308..499c63c 100644
--- a/android_webview/renderer/aw_content_renderer_client.h
+++ b/android_webview/renderer/aw_content_renderer_client.h
@@ -26,8 +26,7 @@ class AwContentRendererClient : public content::ContentRendererClient {
   void RenderFrameCreated(content::RenderFrame* render_frame) override;
   void RenderViewCreated(content::RenderView* render_view) override;
   bool HasErrorPage(int http_status_code, std::string* error_domain) override;
-  void GetNavigationErrorStrings(content::RenderView* render_view,
-                                 blink::WebFrame* frame,
+  void GetNavigationErrorStrings(content::RenderFrame* render_frame,
                                  const blink::WebURLRequest& failed_request,
                                  const blink::WebURLError& error,
                                  std::string* error_html,
diff --git a/chrome/renderer/chrome_content_renderer_client.cc b/chrome/renderer/chrome_content_renderer_client.cc
index 7910d84..448dec2 100644
--- a/chrome/renderer/chrome_content_renderer_client.cc
+++ b/chrome/renderer/chrome_content_renderer_client.cc
@@ -506,11 +506,9 @@ void ChromeContentRendererClient::RenderFrameCreated(
   new nacl::NaClHelper(render_frame);
 #endif
 
-  if (render_frame->IsMainFrame()) {
-    // Only attach NetErrorHelper to the main frame, since only the main frame
-    // should get error pages.
-    new NetErrorHelper(render_frame);
+  new NetErrorHelper(render_frame);
 
+  if (render_frame->IsMainFrame()) {
     // Only attach MainRenderFrameObserver to the main frame, since
     // we only want to log page load metrics for the main frame.
     new page_load_metrics::MetricsRenderFrameObserver(render_frame);
@@ -1070,21 +1068,16 @@ bool ChromeContentRendererClient::ShouldSuppressErrorPage(
   // Unit tests for ChromeContentRendererClient pass a NULL RenderFrame here.
   // Unfortunately it's very difficult to construct a mock RenderView, so skip
   // this functionality in this case.
-  if (render_frame) {
-    content::RenderView* render_view = render_frame->GetRenderView();
-    content::RenderFrame* main_render_frame = render_view->GetMainRenderFrame();
-    blink::WebFrame* web_frame = render_frame->GetWebFrame();
-    NetErrorHelper* net_error_helper = NetErrorHelper::Get(main_render_frame);
-    if (net_error_helper->ShouldSuppressErrorPage(web_frame, url))
-      return true;
+  if (render_frame &&
+      NetErrorHelper::Get(render_frame)->ShouldSuppressErrorPage(url)) {
+    return true;
   }
   // Do not flash an error page if the Instant new tab page fails to load.
   return SearchBouncer::GetInstance()->IsNewTabPage(url);
 }
 
 void ChromeContentRendererClient::GetNavigationErrorStrings(
-    content::RenderView* render_view,
-    blink::WebFrame* frame,
+    content::RenderFrame* render_frame,
     const blink::WebURLRequest& failed_request,
     const blink::WebURLError& error,
     std::string* error_html,
@@ -1094,16 +1087,8 @@ void ChromeContentRendererClient::GetNavigationErrorStrings(
   bool is_post = base::EqualsASCII(
       base::StringPiece16(failed_request.httpMethod()), "POST");
 
-  if (error_html) {
-    // TODO(ellyjones): change GetNavigationErrorStrings to take a RenderFrame
-    // instead of a RenderView, then pass that in.
-    // This is safe for now because we only install the NetErrorHelper on the
-    // main render frame anyway; see the TODO(ellyjones) in
-    // RenderFrameCreated.
-    content::RenderFrame* main_render_frame = render_view->GetMainRenderFrame();
-    NetErrorHelper* helper = NetErrorHelper::Get(main_render_frame);
-    helper->GetErrorHTML(frame, error, is_post, error_html);
-  }
+  if (error_html)
+    NetErrorHelper::Get(render_frame)->GetErrorHTML(error, is_post, error_html);
 
   if (error_description)
     *error_description = LocalizedError::GetErrorDetails(error, is_post);
diff --git a/chrome/renderer/chrome_content_renderer_client.h b/chrome/renderer/chrome_content_renderer_client.h
index 6549bdd..c2d1108 100644
--- a/chrome/renderer/chrome_content_renderer_client.h
+++ b/chrome/renderer/chrome_content_renderer_client.h
@@ -92,8 +92,7 @@ class ChromeContentRendererClient : public content::ContentRendererClient {
   bool HasErrorPage(int http_status_code, std::string* error_domain) override;
   bool ShouldSuppressErrorPage(content::RenderFrame* render_frame,
                                const GURL& url) override;
-  void GetNavigationErrorStrings(content::RenderView* render_view,
-                                 blink::WebFrame* frame,
+  void GetNavigationErrorStrings(content::RenderFrame* render_frame,
                                  const blink::WebURLRequest& failed_request,
                                  const blink::WebURLError& error,
                                  std::string* error_html,
diff --git a/chrome/renderer/net/net_error_helper.cc b/chrome/renderer/net/net_error_helper.cc
index 84f55e9b..af983a6 100644
--- a/chrome/renderer/net/net_error_helper.cc
+++ b/chrome/renderer/net/net_error_helper.cc
@@ -34,8 +34,8 @@
 #include "third_party/WebKit/public/platform/WebURLResponse.h"
 #include "third_party/WebKit/public/web/WebDataSource.h"
 #include "third_party/WebKit/public/web/WebDocument.h"
+#include "third_party/WebKit/public/web/WebFrame.h"
 #include "third_party/WebKit/public/web/WebLocalFrame.h"
-#include "third_party/WebKit/public/web/WebView.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/base/webui/jstemplate_builder.h"
 #include "url/gurl.h"
@@ -57,15 +57,16 @@ namespace {
 // suggestions.  If it takes too long, just use the local error page.
 const int kNavigationCorrectionFetchTimeoutSec = 3;
 
-NetErrorHelperCore::PageType GetLoadingPageType(const blink::WebFrame* frame) {
-  GURL url = frame->provisionalDataSource()->request().url();
+NetErrorHelperCore::PageType GetLoadingPageType(RenderFrame* render_frame) {
+  blink::WebFrame* web_frame = render_frame->GetWebFrame();
+  GURL url = web_frame->provisionalDataSource()->request().url();
   if (!url.is_valid() || url.spec() != kUnreachableWebDataURL)
     return NetErrorHelperCore::NON_ERROR_PAGE;
   return NetErrorHelperCore::ERROR_PAGE;
 }
 
-NetErrorHelperCore::FrameType GetFrameType(const blink::WebFrame* frame) {
-  if (!frame->parent())
+NetErrorHelperCore::FrameType GetFrameType(RenderFrame* render_frame) {
+  if (render_frame->IsMainFrame())
     return NetErrorHelperCore::MAIN_FRAME;
   return NetErrorHelperCore::SUB_FRAME;
 }
@@ -82,6 +83,8 @@ NetErrorHelper::NetErrorHelper(RenderFrame* render_frame)
       command_line->HasSwitch(switches::kEnableOfflineAutoReload);
   bool auto_reload_visible_only =
       command_line->HasSwitch(switches::kEnableOfflineAutoReloadVisibleOnly);
+  // TODO(mmenke): Consider only creating a NetErrorHelperCore for main frames.
+  // subframes don't need any of the NetErrorHelperCore's extra logic.
   core_.reset(new NetErrorHelperCore(this,
                                      auto_reload_enabled,
                                      auto_reload_visible_only,
@@ -102,8 +105,8 @@ void NetErrorHelper::TrackClick(int tracking_id) {
 }
 
 void NetErrorHelper::DidStartProvisionalLoad() {
-  blink::WebFrame* frame = render_frame()->GetWebFrame();
-  core_->OnStartLoad(GetFrameType(frame), GetLoadingPageType(frame));
+  core_->OnStartLoad(GetFrameType(render_frame()),
+                     GetLoadingPageType(render_frame()));
 }
 
 void NetErrorHelper::DidCommitProvisionalLoad(bool is_new_navigation,
@@ -113,13 +116,12 @@ void NetErrorHelper::DidCommitProvisionalLoad(bool is_new_navigation,
   // it.
   weak_controller_delegate_factory_.InvalidateWeakPtrs();
 
-  blink::WebFrame* frame = render_frame()->GetWebFrame();
-  core_->OnCommitLoad(GetFrameType(frame), frame->document().url());
+  core_->OnCommitLoad(GetFrameType(render_frame()),
+                      render_frame()->GetWebFrame()->document().url());
 }
 
 void NetErrorHelper::DidFinishLoad() {
-  blink::WebFrame* frame = render_frame()->GetWebFrame();
-  core_->OnFinishLoad(GetFrameType(frame));
+  core_->OnFinishLoad(GetFrameType(render_frame()));
 }
 
 void NetErrorHelper::OnStop() {
@@ -154,16 +156,15 @@ void NetErrorHelper::NetworkStateChanged(bool enabled) {
 }
 
 void NetErrorHelper::GetErrorHTML(
-    blink::WebFrame* frame,
     const blink::WebURLError& error,
     bool is_failed_post,
     std::string* error_html) {
-  core_->GetErrorHTML(GetFrameType(frame), error, is_failed_post, error_html);
+  core_->GetErrorHTML(GetFrameType(render_frame()), error, is_failed_post,
+                      error_html);
 }
 
-bool NetErrorHelper::ShouldSuppressErrorPage(blink::WebFrame* frame,
-                                             const GURL& url) {
-  return core_->ShouldSuppressErrorPage(GetFrameType(frame), url);
+bool NetErrorHelper::ShouldSuppressErrorPage(const GURL& url) {
+  return core_->ShouldSuppressErrorPage(GetFrameType(render_frame()), url);
 }
 
 void NetErrorHelper::GenerateLocalizedErrorPage(
@@ -202,13 +203,10 @@ void NetErrorHelper::GenerateLocalizedErrorPage(
   }
 }
 
-void NetErrorHelper::LoadErrorPageInMainFrame(const std::string& html,
-                                              const GURL& failed_url) {
-  blink::WebView* web_view = render_frame()->GetRenderView()->GetWebView();
-  if (!web_view)
-    return;
-  blink::WebFrame* frame = web_view->mainFrame();
-  frame->loadHTMLString(html, GURL(kUnreachableWebDataURL), failed_url, true);
+void NetErrorHelper::LoadErrorPage(const std::string& html,
+                                   const GURL& failed_url) {
+  render_frame()->GetWebFrame()->loadHTMLString(
+      html, GURL(kUnreachableWebDataURL), failed_url, true);
 }
 
 void NetErrorHelper::EnablePageHelperFunctions() {
@@ -251,11 +249,6 @@ void NetErrorHelper::FetchNavigationCorrections(
     const std::string& navigation_correction_request_body) {
   DCHECK(!correction_fetcher_.get());
 
-  blink::WebView* web_view = render_frame()->GetRenderView()->GetWebView();
-  if (!web_view)
-    return;
-  blink::WebFrame* frame = web_view->mainFrame();
-
   correction_fetcher_.reset(
       content::ResourceFetcher::Create(navigation_correction_url));
   correction_fetcher_->SetMethod("POST");
@@ -263,7 +256,7 @@ void NetErrorHelper::FetchNavigationCorrections(
   correction_fetcher_->SetHeader("Content-Type", "application/json");
 
   correction_fetcher_->Start(
-      frame,
+      render_frame()->GetWebFrame(),
       blink::WebURLRequest::RequestContextInternal,
       blink::WebURLRequest::FrameTypeNone,
       content::ResourceFetcher::PLATFORM_LOADER,
@@ -281,11 +274,6 @@ void NetErrorHelper::CancelFetchNavigationCorrections() {
 void NetErrorHelper::SendTrackingRequest(
     const GURL& tracking_url,
     const std::string& tracking_request_body) {
-  blink::WebView* web_view = render_frame()->GetRenderView()->GetWebView();
-  if (!web_view)
-    return;
-  blink::WebFrame* frame = web_view->mainFrame();
-
   // If there's already a pending tracking request, this will cancel it.
   tracking_fetcher_.reset(content::ResourceFetcher::Create(tracking_url));
   tracking_fetcher_->SetMethod("POST");
@@ -293,7 +281,7 @@ void NetErrorHelper::SendTrackingRequest(
   tracking_fetcher_->SetHeader("Content-Type", "application/json");
 
   tracking_fetcher_->Start(
-      frame,
+      render_frame()->GetWebFrame(),
       blink::WebURLRequest::RequestContextInternal,
       blink::WebURLRequest::FrameTypeTopLevel,
       content::ResourceFetcher::PLATFORM_LOADER,
diff --git a/chrome/renderer/net/net_error_helper.h b/chrome/renderer/net/net_error_helper.h
index cc1a728..3d4f535 100644
--- a/chrome/renderer/net/net_error_helper.h
+++ b/chrome/renderer/net/net_error_helper.h
@@ -43,7 +43,7 @@ class NetErrorHelper
       public error_page::NetErrorHelperCore::Delegate,
       public NetErrorPageController::Delegate {
  public:
-  explicit NetErrorHelper(content::RenderFrame* render_view);
+  explicit NetErrorHelper(content::RenderFrame* render_frame);
   ~NetErrorHelper() override;
 
   // NetErrorPageController::Delegate implementation
@@ -65,22 +65,16 @@ class NetErrorHelper
   // RenderProcessObserver implementation.
   void NetworkStateChanged(bool online) override;
 
-  // Examines |frame| and |error| to see if this is an error worthy of a DNS
-  // probe.  If it is, initializes |error_strings| based on |error|,
-  // |is_failed_post|, and |locale| with suitable strings and returns true.
-  // If not, returns false, in which case the caller should look up error
-  // strings directly using LocalizedError::GetNavigationErrorStrings.
-  //
-  // Updates the NetErrorHelper with the assumption the page will be loaded
-  // immediately.
-  void GetErrorHTML(blink::WebFrame* frame,
-                    const blink::WebURLError& error,
+  // Initializes |error_html| with the HTML of an error page in response to
+  // |error|.  Updates internals state with the assumption the page will be
+  // loaded immediately.
+  void GetErrorHTML(const blink::WebURLError& error,
                     bool is_failed_post,
                     std::string* error_html);
 
-  // Returns whether a load for |url| in |frame| should have its error page
-  // suppressed.
-  bool ShouldSuppressErrorPage(blink::WebFrame* frame, const GURL& url);
+  // Returns whether a load for |url| in the |frame| the NetErrorHelper is
+  // attached to should have its error page suppressed.
+  bool ShouldSuppressErrorPage(const GURL& url);
 
  private:
   // NetErrorHelperCore::Delegate implementation:
@@ -93,8 +87,7 @@ class NetErrorHelper
       bool* show_saved_copy_button_shown,
       bool* show_cached_copy_button_shown,
       std::string* html) const override;
-  void LoadErrorPageInMainFrame(const std::string& html,
-                                const GURL& failed_url) override;
+  void LoadErrorPage(const std::string& html, const GURL& failed_url) override;
   void EnablePageHelperFunctions() override;
   void UpdateErrorPage(const blink::WebURLError& error,
                        bool is_failed_post,
diff --git a/components/error_page/renderer/net_error_helper_core.cc b/components/error_page/renderer/net_error_helper_core.cc
index 60e2eeaf..e193073 100644
--- a/components/error_page/renderer/net_error_helper_core.cc
+++ b/components/error_page/renderer/net_error_helper_core.cc
@@ -790,9 +790,8 @@ void NetErrorHelperCore::OnNavigationCorrectionsFetched(
   // TODO(mmenke):  Once the new API is in place, look into replacing this
   //                double page load by just updating the error page, like DNS
   //                probes do.
-  delegate_->LoadErrorPageInMainFrame(
-      error_html,
-      pending_error_page_info_->error.unreachableURL);
+  delegate_->LoadErrorPage(error_html,
+                           pending_error_page_info_->error.unreachableURL);
 }
 
 blink::WebURLError NetErrorHelperCore::GetUpdatedError(
diff --git a/components/error_page/renderer/net_error_helper_core.h b/components/error_page/renderer/net_error_helper_core.h
index be88ccc..6a3dc7b 100644
--- a/components/error_page/renderer/net_error_helper_core.h
+++ b/components/error_page/renderer/net_error_helper_core.h
@@ -67,9 +67,9 @@ class NetErrorHelperCore {
         bool* show_cached_copy_button_shown,
         std::string* html) const = 0;
 
-    // Loads the given HTML in the main frame for use as an error page.
-    virtual void LoadErrorPageInMainFrame(const std::string& html,
-                                          const GURL& failed_url) = 0;
+    // Loads the given HTML in the frame for use as an error page.
+    virtual void LoadErrorPage(const std::string& html,
+                               const GURL& failed_url) = 0;
 
     // Create extra Javascript bindings in the error page. Will only be invoked
     // after an error page has finished loading.
@@ -131,14 +131,9 @@ class NetErrorHelperCore {
                      bool is_visible);
   ~NetErrorHelperCore();
 
-  // Examines |frame| and |error| to see if this is an error worthy of a DNS
-  // probe.  If it is, initializes |error_strings| based on |error|,
-  // |is_failed_post|, and |locale| with suitable strings and returns true.
-  // If not, returns false, in which case the caller should look up error
-  // strings directly using LocalizedError::GetNavigationErrorStrings.
-  //
-  // Updates the NetErrorHelper with the assumption the page will be loaded
-  // immediately.
+  // Initializes |error_html| with the HTML of an error page in response to
+  // |error|.  Updates internals state with the assumption the page will be
+  // loaded immediately.
   void GetErrorHTML(FrameType frame_type,
                     const blink::WebURLError& error,
                     bool is_failed_post,
diff --git a/components/error_page/renderer/net_error_helper_core_unittest.cc b/components/error_page/renderer/net_error_helper_core_unittest.cc
index 20ccb91..6fa8caf 100644
--- a/components/error_page/renderer/net_error_helper_core_unittest.cc
+++ b/components/error_page/renderer/net_error_helper_core_unittest.cc
@@ -334,8 +334,7 @@ class NetErrorHelperCoreTest : public testing::Test,
     *html = ErrorToString(error, is_failed_post);
   }
 
-  void LoadErrorPageInMainFrame(const std::string& html,
-                                const GURL& failed_url) override {
+  void LoadErrorPage(const std::string& html, const GURL& failed_url) override {
     error_html_update_count_++;
     last_error_html_ = html;
   }
diff --git a/content/public/renderer/content_renderer_client.h b/content/public/renderer/content_renderer_client.h
index d419f78..f55c691 100644
--- a/content/public/renderer/content_renderer_client.h
+++ b/content/public/renderer/content_renderer_client.h
@@ -133,8 +133,7 @@ class CONTENT_EXPORT ContentRendererClient {
   // (lack of information on the error code) so the caller should take care to
   // initialize the string values with safe defaults before the call.
   virtual void GetNavigationErrorStrings(
-      content::RenderView* render_view,
-      blink::WebFrame* frame,
+      content::RenderFrame* render_frame,
       const blink::WebURLRequest& failed_request,
       const blink::WebURLError& error,
       std::string* error_html,
diff --git a/content/renderer/browser_render_view_browsertest.cc b/content/renderer/browser_render_view_browsertest.cc
index f24b4a0..123b238 100644
--- a/content/renderer/browser_render_view_browsertest.cc
+++ b/content/renderer/browser_render_view_browsertest.cc
@@ -49,8 +49,7 @@ class TestShellContentRendererClient : public ShellContentRendererClient {
         latest_error_reason_(0),
         latest_error_stale_copy_in_cache_(false) {}
 
-  void GetNavigationErrorStrings(content::RenderView* render_view,
-                                 blink::WebFrame* frame,
+  void GetNavigationErrorStrings(content::RenderFrame* render_frame,
                                  const blink::WebURLRequest& failed_request,
                                  const blink::WebURLError& error,
                                  std::string* error_html,
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index 8489199..76b7da5 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -1891,7 +1891,7 @@ void RenderFrameImpl::LoadNavigationErrorPage(
     bool replace) {
   std::string error_html;
   GetContentClient()->renderer()->GetNavigationErrorStrings(
-      render_view(), frame_, failed_request, error, &error_html, NULL);
+      this, failed_request, error, &error_html, nullptr);
 
   frame_->loadHTMLString(error_html,
                          GURL(kUnreachableWebDataURL),
@@ -3081,11 +3081,10 @@ void RenderFrameImpl::didFailLoad(blink::WebLocalFrame* frame,
   const WebURLRequest& failed_request = ds->request();
   base::string16 error_description;
   GetContentClient()->renderer()->GetNavigationErrorStrings(
-      render_view_.get(),
-      frame,
+      this,
       failed_request,
       error,
-      NULL,
+      nullptr,
       &error_description);
   Send(new FrameHostMsg_DidFailLoadWithError(routing_id_,
                                              failed_request.url(),
@@ -5064,8 +5063,7 @@ void RenderFrameImpl::SendFailedProvisionalLoad(
   FrameHostMsg_DidFailProvisionalLoadWithError_Params params;
   params.error_code = error.reason;
   GetContentClient()->renderer()->GetNavigationErrorStrings(
-      render_view_.get(), frame, request, error, NULL,
-      &params.error_description);
+      this, request, error, nullptr, &params.error_description);
   params.url = error.unreachableURL;
   params.showing_repost_interstitial = show_repost_interstitial;
   params.was_ignored_by_handler = error.wasIgnoredByHandler;
diff --git a/content/renderer/render_view_browsertest.cc b/content/renderer/render_view_browsertest.cc
index a7e700f..47a4ef6 100644
--- a/content/renderer/render_view_browsertest.cc
+++ b/content/renderer/render_view_browsertest.cc
@@ -2005,8 +2005,7 @@ class RendererErrorPageTest : public RenderViewImplTest {
       return url == GURL("http://example.com/suppress");
     }
 
-    void GetNavigationErrorStrings(content::RenderView* render_view,
-                                   blink::WebFrame* frame,
+    void GetNavigationErrorStrings(content::RenderFrame* render_frame,
                                    const blink::WebURLRequest& failed_request,
                                    const blink::WebURLError& error,
                                    std::string* error_html,