Implement a skeleton of the Superfish interstitial

chrome/browser/ssl/superfish_blocking_page.cc

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/ssl/superfish_blocking_page.h"
+
+#include "chrome/browser/interstitials/chrome_metrics_helper.h"
+#include "chrome/common/pref_names.h"
+#include "components/prefs/pref_service.h"
+#include "components/security_interstitials/content/security_interstitial_controller_client.h"
+#include "components/security_interstitials/core/superfish_error_ui.h"
+#include "content/public/browser/interstitial_page_delegate.h"
+#include "content/public/browser/web_contents.h"
+
+// static
+content::InterstitialPageDelegate::TypeID
+    SuperfishBlockingPage::kTypeForTesting =
+        &SuperfishBlockingPage::kTypeForTesting;
+
+// static
+SuperfishBlockingPage* SuperfishBlockingPage::Create(
+    content::WebContents* web_contents,
+    int cert_error,
+    const net::SSLInfo& ssl_info,
+    const GURL& request_url,
+    int options_mask,
+    const base::Time& time_triggered,
+    std::unique_ptr<SSLCertReporter> ssl_cert_reporter,
+    const base::Callback<void(content::CertificateRequestResultType)>&
+        callback) {
+  Profile* profile =
+      Profile::FromBrowserContext(web_contents->GetBrowserContext());
+  if (profile &&
+      !profile->GetPrefs()->GetBoolean(prefs::kSSLErrorOverrideAllowed)) {
+    options_mask |= security_interstitials::SSLErrorUI::HARD_OVERRIDE_DISABLED;
+  }
+  // Superfish interstitials are not overridable.
+  options_mask &= ~security_interstitials::SSLErrorUI::SOFT_OVERRIDE_ENABLED;

[meacer, 2017/06/22 19:06:19]

rant: These flags names are really confusing. HARD_OVERRIDE_DISABLED and
SOFT_OVERRIDE_ENABLED? We should choose a convention and stick with it.

[estark, 2017/06/22 22:01:05]

Agreed. Great interstitial refactor will probably be a good time to revisit
this; I find all these overridable settings super confusing.

Fortunately I deleted this code based on your suggestion below so you no longer
get to apply this rant to code I'm adding :P

+
+  std::unique_ptr<ChromeMetricsHelper> metrics_helper(CreateMetricsHelper(
+      web_contents, cert_error, request_url, false /* overridable */));
+  metrics_helper.get()->StartRecordingCaptivePortalMetrics(
+      false /* overridable */);
+
+  SuperfishBlockingPage* page = new SuperfishBlockingPage(
+      web_contents, ssl_info, request_url, options_mask, time_triggered,
+      std::move(ssl_cert_reporter), false /* overridable */,
+      std::move(metrics_helper), callback);
+  page->SetErrorUI(base::MakeUnique<security_interstitials::SuperfishErrorUI>(
+      request_url, cert_error, ssl_info, options_mask, time_triggered,
+      page->controller()));
+  return page;
+}
+
+content::InterstitialPageDelegate::TypeID
+SuperfishBlockingPage::GetTypeForTesting() const {
+  return SuperfishBlockingPage::kTypeForTesting;

[meacer, 2017/06/22 19:06:19]

It looks like the main reason we have a separate SuperfishBlockingPage is to be
able to distinguish between interstitials and being able to change override
options. Is that correct?

If so, I wonder if we can refactor the code so that we only have one
SSLBlockingPage, and two different SSLErrorUI classes for generic and superfish
interstitials. (I'm slightly worried about the code duplication in the
constructor. It seems to be a cause of histogram bugs when we miss one of the
blocking pages while changing the code)

[estark, 2017/06/22 22:01:05]

Good idea, thanks. Done.

+}
+
+SuperfishBlockingPage::SuperfishBlockingPage(
+    content::WebContents* web_contents,
+    const net::SSLInfo& ssl_info,
+    const GURL& request_url,
+    int options_mask,
+    const base::Time& time_triggered,
+    std::unique_ptr<SSLCertReporter> ssl_cert_reporter,
+    bool overridable,
+    std::unique_ptr<ChromeMetricsHelper> metrics_helper,
+    const base::Callback<void(content::CertificateRequestResultType)>& callback)
+    : SSLBlockingPage(web_contents,
+                      ssl_info,
+                      request_url,
+                      options_mask,
+                      time_triggered,
+                      std::move(ssl_cert_reporter),
+                      overridable,
+                      std::move(metrics_helper),
+                      callback) {
+  // Creating an interstitial without showing (e.g. from chrome://interstitials)
+  // it leaks memory, so don't create it here.
+}