Reland "Refactor SSLCertificateErrorPageController"

This reverts commit 30645e2f.

Reason for revert: Failures reproduce locally on commits prior to this CL.

Original change's description:
> Revert "Refactor SSLCertificateErrorPageController"
>
> This reverts commit 08c128cd.
>
> Reason for revert: Reverterd on suspicion of causing build failures on Mac 10.11
>
> TBR: carlosil@chromium.org
>
> failures:
> SSLClientSocketReadTest.Read/1
> SSLClientSocketReadTest.Read/0
> HTTPSEVCRLSetTest.FreshCRLSetNotCovered
> WebSocketEndToEndTest.BasicSmokeTest
> SSLClientSocketReadTest.DumpMemoryStats/0
> SSLClientSocketReadTest.DumpMemoryStats/1
> URLRequestTestFTP.FTPCheckWrongUser
> WebSocketEndToEndTest.TruncatedResponse
> WebSocketEndToEndTest.HstsWebSocketToWebSocket
> WebSocketEndToEndTest.TrailingWhitespace
> HTTPSOCSPTest.IntermediateResponseTooOld
> URLRequestTestFTP.FTPCheckWrongUserRestart
> URLRequestTestFTP.FTPCheckWrongPasswordRestart
> HTTPSOCSPTest.Revoked
> HTTPSFallbackTest.TLSv1_1NoFallback
> HTTPSFallbackTest.TLSv1_3Interference
> URLRequestTestFTP.FTPCheckWrongPassword
> WebSocketEndToEndTest.HeaderContinuations
> HTTPSCRLSetTest.ExpiredCRLSetAndRevoked
> HTTPSEVCRLSetTest.FreshCRLSetCovered
> SSLClientSocketReadTest.Read_DeleteWhilePendingFullDuplex/1
> SSLClientSocketReadTest.Read_DeleteWhilePendingFullDuplex/0
> OCSPVerify/HTTPSOCSPVerifyTest.VerifyResult/18
> HTTPSOCSPTest.Valid
> HTTPSEVCRLSetTest.ExpiredCRLSet
> OCSPVerify/HTTPSOCSPVerifyTest.VerifyResult/12
> OCSPVerify/HTTPSOCSPVerifyTest.VerifyResult/13
> OCSPVerify/HTTPSOCSPVerifyTest.VerifyResult/10
> OCSPVerify/HTTPSOCSPVerifyTest.VerifyResult/11
> OCSPVerify/HTTPSOCSPVerifyTest.VerifyResult/16
> … 70 more (100 total) …
>
> Original change's description:
> > Refactor SSLCertificateErrorPageController
> >
> > SSLCertificateErrorPageController is no longer only used on SSL errors,
> > renamed it to SecurityInterstitialErrorPageController, and removed the
> > net::Error code passed to EnablePageHelperFunctions since it's no
> > longer used.
> >
> > Bug: 910794
> > Change-Id: I718e0fcbcb2643cb0c47818dda1f317aef4e75cc
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1503910
> > Reviewed-by: Matt Menke <mmenke@chromium.org>
> > Reviewed-by: Scott Violet <sky@chromium.org>
> > Commit-Queue: Carlos IL <carlosil@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#638232}
>
> TBR=sky@chromium.org,mmenke@chromium.org,carlosil@chromium.org
>
> Change-Id: I39489effa3e0029b5ff65f5bc9cbd991aef1ddea
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 910794
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1507039
> Reviewed-by: Peter Williamson <petewil@chromium.org>
> Commit-Queue: Peter Williamson <petewil@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#638363}

TBR=sky@chromium.org,petewil@chromium.org,mmenke@chromium.org,carlosil@chromium.org

Change-Id: I5b9048e43ebc60964c44ebfd2c05207a22a549dc
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 910794
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1509352Reviewed-by: Carlos IL <carlosil@chromium.org>
Commit-Queue: Carlos IL <carlosil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#638773}

chrome/renderer/BUILD.gn

@@ -102,8 +102,8 @@ jumbo_static_library("renderer") {
     "sandbox_status_extension_android.h",
     "security_filter_peer.cc",
     "security_filter_peer.h",
-    "ssl/ssl_certificate_error_page_controller.cc",
-    "ssl/ssl_certificate_error_page_controller.h",
+    "security_interstitials/security_interstitial_page_controller.cc",
+    "security_interstitials/security_interstitial_page_controller.h",
     "supervised_user/supervised_user_error_page_controller.cc",
     "supervised_user/supervised_user_error_page_controller.h",
     "tts_dispatcher.cc",

chrome/renderer/net/net_error_helper.cc

@@ -24,7 +24,7 @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/renderer/chrome_render_thread_observer.h"
-#include "chrome/renderer/ssl/ssl_certificate_error_page_controller.h"
+#include "chrome/renderer/security_interstitials/security_interstitial_page_controller.h"
 #include "chrome/renderer/supervised_user/supervised_user_error_page_controller.h"
 #include "components/error_page/common/error.h"
 #include "components/error_page/common/error_page_params.h"
@@ -167,7 +167,7 @@ NetErrorHelper::NetErrorHelper(RenderFrame* render_frame)
     : RenderFrameObserver(render_frame),
       content::RenderFrameObserverTracker<NetErrorHelper>(render_frame),
       weak_controller_delegate_factory_(this),
-      weak_ssl_error_controller_delegate_factory_(this),
+      weak_security_interstitial_controller_delegate_factory_(this),
       weak_supervised_user_error_controller_delegate_factory_(this) {
   RenderThread::Get()->AddObserver(this);
   base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
@@ -324,7 +324,7 @@ void NetErrorHelper::DidCommitProvisionalLoad(bool is_same_document_navigation,
   // error page, the controller has not yet been attached, so this won't affect
   // it.
   weak_controller_delegate_factory_.InvalidateWeakPtrs();
-  weak_ssl_error_controller_delegate_factory_.InvalidateWeakPtrs();
+  weak_security_interstitial_controller_delegate_factory_.InvalidateWeakPtrs();
   weak_supervised_user_error_controller_delegate_factory_.InvalidateWeakPtrs();
 
   core_->OnCommitLoad(GetFrameType(render_frame()),
@@ -433,9 +433,10 @@ void NetErrorHelper::LoadErrorPage(const std::string& html,
                                  failed_url, true /* replace_current_item */);
 }
 
-void NetErrorHelper::EnablePageHelperFunctions(net::Error net_error) {
-  SSLCertificateErrorPageController::Install(
-      render_frame(), weak_ssl_error_controller_delegate_factory_.GetWeakPtr());
+void NetErrorHelper::EnablePageHelperFunctions() {
+  SecurityInterstitialPageController::Install(
+      render_frame(),
+      weak_security_interstitial_controller_delegate_factory_.GetWeakPtr());
   NetErrorPageController::Install(
       render_frame(), weak_controller_delegate_factory_.GetWeakPtr());
 

chrome/renderer/net/net_error_helper.h

@@ -18,7 +18,7 @@
 #include "chrome/common/supervised_user_commands.mojom.h"
 #include "chrome/renderer/net/net_error_helper_core.h"
 #include "chrome/renderer/net/net_error_page_controller.h"
-#include "chrome/renderer/ssl/ssl_certificate_error_page_controller.h"
+#include "chrome/renderer/security_interstitials/security_interstitial_page_controller.h"
 #include "chrome/renderer/supervised_user/supervised_user_error_page_controller.h"
 #include "chrome/renderer/supervised_user/supervised_user_error_page_controller_delegate.h"
 #include "components/error_page/common/net_error_info.h"
@@ -55,7 +55,7 @@ class NetErrorHelper
       public content::RenderThreadObserver,
       public NetErrorHelperCore::Delegate,
       public NetErrorPageController::Delegate,
-      public SSLCertificateErrorPageController::Delegate,
+      public SecurityInterstitialPageController::Delegate,
       public SupervisedUserErrorPageControllerDelegate,
       public chrome::mojom::NetworkDiagnosticsClient,
       public chrome::mojom::NavigationCorrector {
@@ -75,7 +75,7 @@ class NetErrorHelper
   void UpdateEasterEggHighScore(int high_score) override;
   void ResetEasterEggHighScore() override;
 
-  // SSLCertificateErrorPageController::Delegate implementation
+  // SecurityInterstitialPageController::Delegate implementation
   void SendCommand(
       security_interstitials::SecurityInterstitialCommand command) override;
 
@@ -130,7 +130,7 @@ class NetErrorHelper
       bool* auto_fetch_allowed,
       std::string* html) const override;
   void LoadErrorPage(const std::string& html, const GURL& failed_url) override;
-  void EnablePageHelperFunctions(net::Error net_error) override;
+  void EnablePageHelperFunctions() override;
   void UpdateErrorPage(const error_page::Error& error,
                        bool is_failed_post,
                        bool can_use_local_diagnostics_service) override;
@@ -207,8 +207,8 @@ class NetErrorHelper
   base::WeakPtrFactory<NetErrorPageController::Delegate>
       weak_controller_delegate_factory_;
 
-  base::WeakPtrFactory<SSLCertificateErrorPageController::Delegate>
-      weak_ssl_error_controller_delegate_factory_;
+  base::WeakPtrFactory<SecurityInterstitialPageController::Delegate>
+      weak_security_interstitial_controller_delegate_factory_;
 
   base::WeakPtrFactory<SupervisedUserErrorPageControllerDelegate>
       weak_supervised_user_error_controller_delegate_factory_;

chrome/renderer/net/net_error_helper_core.cc

@@ -680,8 +680,7 @@ void NetErrorHelperCore::OnFinishLoad(FrameType frame_type) {
   delegate_->SetIsShowingDownloadButton(
       committed_error_page_info_->download_button_in_page);
 
-  delegate_->EnablePageHelperFunctions(
-      static_cast<net::Error>(committed_error_page_info_->error.reason()));
+  delegate_->EnablePageHelperFunctions();
 
 #if defined(OS_ANDROID)
   if (committed_error_page_info_->offline_content_feature_state ==

chrome/renderer/net/net_error_helper_core.h

@@ -80,7 +80,7 @@ class NetErrorHelperCore {
 
     // Create extra Javascript bindings in the error page. Will only be invoked
     // after an error page has finished loading.
-    virtual void EnablePageHelperFunctions(net::Error net_error) = 0;
+    virtual void EnablePageHelperFunctions() = 0;
 
     // Updates the currently displayed error page with a new error code.  The
     // currently displayed error page must have finished loading, and must have

chrome/renderer/net/net_error_helper_core_unittest.cc

@@ -390,7 +390,7 @@ class NetErrorHelperCoreTest : public testing::Test,
     last_error_html_ = html;
   }
 
-  void EnablePageHelperFunctions(net::Error net_error) override {
+  void EnablePageHelperFunctions() override {
     enable_page_helper_functions_count_++;
   }
 

chrome/renderer/ssl/ssl_certificate_error_page_controller.cc → chrome/renderer/security_interstitials/security_interstitial_page_controller.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/renderer/ssl/ssl_certificate_error_page_controller.h"
+#include "chrome/renderer/security_interstitials/security_interstitial_page_controller.h"
 
 #include "components/security_interstitials/core/controller_client.h"
 #include "content/public/renderer/render_frame.h"
@@ -11,12 +11,12 @@
 #include "third_party/blink/public/web/blink.h"
 #include "third_party/blink/public/web/web_local_frame.h"
 
-gin::WrapperInfo SSLCertificateErrorPageController::kWrapperInfo = {
+gin::WrapperInfo SecurityInterstitialPageController::kWrapperInfo = {
     gin::kEmbedderNativeGin};
 
-SSLCertificateErrorPageController::Delegate::~Delegate() {}
+SecurityInterstitialPageController::Delegate::~Delegate() {}
 
-void SSLCertificateErrorPageController::Install(
+void SecurityInterstitialPageController::Install(
     content::RenderFrame* render_frame,
     base::WeakPtr<Delegate> delegate) {
   v8::Isolate* isolate = blink::MainThreadIsolate();
@@ -28,8 +28,9 @@ void SSLCertificateErrorPageController::Install(
 
   v8::Context::Scope context_scope(context);
 
-  gin::Handle<SSLCertificateErrorPageController> controller = gin::CreateHandle(
-      isolate, new SSLCertificateErrorPageController(delegate));
+  gin::Handle<SecurityInterstitialPageController> controller =
+      gin::CreateHandle(isolate,
+                        new SecurityInterstitialPageController(delegate));
   if (controller.IsEmpty())
     return;
 
@@ -38,76 +39,76 @@ void SSLCertificateErrorPageController::Install(
               controller.ToV8());
 }
 
-SSLCertificateErrorPageController::SSLCertificateErrorPageController(
+SecurityInterstitialPageController::SecurityInterstitialPageController(
     base::WeakPtr<Delegate> delegate)
     : delegate_(delegate) {}
 
-SSLCertificateErrorPageController::~SSLCertificateErrorPageController() {}
+SecurityInterstitialPageController::~SecurityInterstitialPageController() {}
 
-void SSLCertificateErrorPageController::DontProceed() {
+void SecurityInterstitialPageController::DontProceed() {
   SendCommand(
       security_interstitials::SecurityInterstitialCommand::CMD_DONT_PROCEED);
 }
 
-void SSLCertificateErrorPageController::Proceed() {
+void SecurityInterstitialPageController::Proceed() {
   SendCommand(security_interstitials::SecurityInterstitialCommand::CMD_PROCEED);
 }
 
-void SSLCertificateErrorPageController::ShowMoreSection() {
+void SecurityInterstitialPageController::ShowMoreSection() {
   SendCommand(security_interstitials::SecurityInterstitialCommand::
                   CMD_SHOW_MORE_SECTION);
 }
 
-void SSLCertificateErrorPageController::OpenHelpCenter() {
+void SecurityInterstitialPageController::OpenHelpCenter() {
   SendCommand(security_interstitials::SecurityInterstitialCommand::
                   CMD_OPEN_HELP_CENTER);
 }
 
-void SSLCertificateErrorPageController::OpenDiagnostic() {
+void SecurityInterstitialPageController::OpenDiagnostic() {
   SendCommand(
       security_interstitials::SecurityInterstitialCommand::CMD_OPEN_DIAGNOSTIC);
 }
 
-void SSLCertificateErrorPageController::Reload() {
+void SecurityInterstitialPageController::Reload() {
   SendCommand(security_interstitials::SecurityInterstitialCommand::CMD_RELOAD);
 }
 
-void SSLCertificateErrorPageController::OpenDateSettings() {
+void SecurityInterstitialPageController::OpenDateSettings() {
   SendCommand(security_interstitials::SecurityInterstitialCommand::
                   CMD_OPEN_DATE_SETTINGS);
 }
 
-void SSLCertificateErrorPageController::OpenLogin() {
+void SecurityInterstitialPageController::OpenLogin() {
   SendCommand(
       security_interstitials::SecurityInterstitialCommand::CMD_OPEN_LOGIN);
 }
 
-void SSLCertificateErrorPageController::DoReport() {
+void SecurityInterstitialPageController::DoReport() {
   SendCommand(
       security_interstitials::SecurityInterstitialCommand::CMD_DO_REPORT);
 }
 
-void SSLCertificateErrorPageController::DontReport() {
+void SecurityInterstitialPageController::DontReport() {
   SendCommand(
       security_interstitials::SecurityInterstitialCommand::CMD_DONT_REPORT);
 }
 
-void SSLCertificateErrorPageController::OpenReportingPrivacy() {
+void SecurityInterstitialPageController::OpenReportingPrivacy() {
   SendCommand(security_interstitials::SecurityInterstitialCommand::
                   CMD_OPEN_REPORTING_PRIVACY);
 }
 
-void SSLCertificateErrorPageController::OpenWhitepaper() {
+void SecurityInterstitialPageController::OpenWhitepaper() {
   SendCommand(
       security_interstitials::SecurityInterstitialCommand::CMD_OPEN_WHITEPAPER);
 }
 
-void SSLCertificateErrorPageController::ReportPhishingError() {
+void SecurityInterstitialPageController::ReportPhishingError() {
   SendCommand(security_interstitials::SecurityInterstitialCommand::
                   CMD_REPORT_PHISHING_ERROR);
 }
 
-void SSLCertificateErrorPageController::SendCommand(
+void SecurityInterstitialPageController::SendCommand(
     security_interstitials::SecurityInterstitialCommand command) {
   if (delegate_) {
     delegate_->SendCommand(command);
@@ -115,30 +116,31 @@ void SSLCertificateErrorPageController::SendCommand(
 }
 
 gin::ObjectTemplateBuilder
-SSLCertificateErrorPageController::GetObjectTemplateBuilder(
+SecurityInterstitialPageController::GetObjectTemplateBuilder(
     v8::Isolate* isolate) {
-  return gin::Wrappable<SSLCertificateErrorPageController>::
+  return gin::Wrappable<SecurityInterstitialPageController>::
       GetObjectTemplateBuilder(isolate)
           .SetMethod("dontProceed",
-                     &SSLCertificateErrorPageController::DontProceed)
-          .SetMethod("proceed", &SSLCertificateErrorPageController::Proceed)
+                     &SecurityInterstitialPageController::DontProceed)
+          .SetMethod("proceed", &SecurityInterstitialPageController::Proceed)
           .SetMethod("showMoreSection",
-                     &SSLCertificateErrorPageController::ShowMoreSection)
+                     &SecurityInterstitialPageController::ShowMoreSection)
           .SetMethod("openHelpCenter",
-                     &SSLCertificateErrorPageController::OpenHelpCenter)
+                     &SecurityInterstitialPageController::OpenHelpCenter)
           .SetMethod("openDiagnostic",
-                     &SSLCertificateErrorPageController::OpenDiagnostic)
-          .SetMethod("reload", &SSLCertificateErrorPageController::Reload)
+                     &SecurityInterstitialPageController::OpenDiagnostic)
+          .SetMethod("reload", &SecurityInterstitialPageController::Reload)
           .SetMethod("openDateSettings",
-                     &SSLCertificateErrorPageController::OpenDateSettings)
-          .SetMethod("openLogin", &SSLCertificateErrorPageController::OpenLogin)
-          .SetMethod("doReport", &SSLCertificateErrorPageController::DoReport)
+                     &SecurityInterstitialPageController::OpenDateSettings)
+          .SetMethod("openLogin",
+                     &SecurityInterstitialPageController::OpenLogin)
+          .SetMethod("doReport", &SecurityInterstitialPageController::DoReport)
           .SetMethod("dontReport",
-                     &SSLCertificateErrorPageController::DontReport)
+                     &SecurityInterstitialPageController::DontReport)
           .SetMethod("openReportingPrivacy",
-                     &SSLCertificateErrorPageController::OpenReportingPrivacy)
+                     &SecurityInterstitialPageController::OpenReportingPrivacy)
           .SetMethod("openWhitepaper",
-                     &SSLCertificateErrorPageController::OpenWhitepaper)
+                     &SecurityInterstitialPageController::OpenWhitepaper)
           .SetMethod("reportPhishingError",
-                     &SSLCertificateErrorPageController::ReportPhishingError);
+                     &SecurityInterstitialPageController::ReportPhishingError);
 }

chrome/renderer/ssl/ssl_certificate_error_page_controller.h → chrome/renderer/security_interstitials/security_interstitial_page_controller.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_RENDERER_SSL_SSL_CERTIFICATE_ERROR_PAGE_CONTROLLER_H_
-#define CHROME_RENDERER_SSL_SSL_CERTIFICATE_ERROR_PAGE_CONTROLLER_H_
+#ifndef CHROME_RENDERER_SECURITY_INTERSTITIALS_SECURITY_INTERSTITIAL_PAGE_CONTROLLER_H_
+#define CHROME_RENDERER_SECURITY_INTERSTITIALS_SECURITY_INTERSTITIAL_PAGE_CONTROLLER_H_
 
 #include "base/memory/weak_ptr.h"
 #include "components/security_interstitials/core/controller_client.h"
@@ -16,8 +16,8 @@ class RenderFrame;
 // This class makes various helper functions available to interstitials
 // when committed interstitials are on. It is bound to the JavaScript
 // window.certificateErrorPageController object.
-class SSLCertificateErrorPageController
-    : public gin::Wrappable<SSLCertificateErrorPageController> {
+class SecurityInterstitialPageController
+    : public gin::Wrappable<SecurityInterstitialPageController> {
  public:
   static gin::WrapperInfo kWrapperInfo;
 
@@ -40,8 +40,8 @@ class SSLCertificateErrorPageController
                       base::WeakPtr<Delegate> delegate);
 
  private:
-  explicit SSLCertificateErrorPageController(base::WeakPtr<Delegate> delegate);
-  ~SSLCertificateErrorPageController() override;
+  explicit SecurityInterstitialPageController(base::WeakPtr<Delegate> delegate);
+  ~SecurityInterstitialPageController() override;
 
   void DontProceed();
   void Proceed();
@@ -65,7 +65,7 @@ class SSLCertificateErrorPageController
 
   base::WeakPtr<Delegate> const delegate_;
 
-  DISALLOW_COPY_AND_ASSIGN(SSLCertificateErrorPageController);
+  DISALLOW_COPY_AND_ASSIGN(SecurityInterstitialPageController);
 };
 
-#endif  // CHROME_RENDERER_SSL_SSL_CERTIFICATE_ERROR_PAGE_CONTROLLER_H_
+#endif  // CHROME_RENDERER_SECURITY_INTERSTITIALS_SECURITY_INTERSTITIAL_PAGE_CONTROLLER_H_