Wire up CertificateReportingService to handle report uploads

chrome/browser/chrome_content_browser_client.cc

 // Copyright (c) 2012 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/chrome_content_browser_client.h"
 
 #include <map>
 #include <set>
 #include <utility>
 #include <vector>
@@ skipping 54 matching lines @@
 #include "chrome/browser/prerender/prerender_manager.h"
 #include "chrome/browser/prerender/prerender_manager_factory.h"
 #include "chrome/browser/prerender/prerender_message_filter.h"
 #include "chrome/browser/printing/printing_message_filter.h"
 #include "chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_io_data.h"
 #include "chrome/browser/renderer_host/chrome_navigation_ui_data.h"
 #include "chrome/browser/renderer_host/chrome_render_message_filter.h"
 #include "chrome/browser/renderer_host/pepper/chrome_browser_pepper_host_factory.h"
+#include "chrome/browser/safe_browsing/certificate_reporting_service.h"
+#include "chrome/browser/safe_browsing/certificate_reporting_service_factory.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"

[estark, 2016/12/16 01:57:58]

can be removed now?

[meacer, 2016/12/16 20:26:35]

Done.

 #include "chrome/browser/safe_browsing/ui_manager.h"

[estark, 2016/12/16 01:57:58]

ditto

[meacer, 2016/12/16 20:26:35]

Done.

 #include "chrome/browser/search/instant_service.h"
 #include "chrome/browser/search/instant_service_factory.h"
 #include "chrome/browser/search/search.h"
 #include "chrome/browser/speech/chrome_speech_recognition_manager_delegate.h"
 #include "chrome/browser/speech/tts_controller.h"
 #include "chrome/browser/speech/tts_message_filter.h"
 #include "chrome/browser/ssl/ssl_blocking_page.h"
 #include "chrome/browser/ssl/ssl_cert_reporter.h"
 #include "chrome/browser/ssl/ssl_client_certificate_selector.h"
 #include "chrome/browser/ssl/ssl_error_handler.h"
@@ skipping 47 matching lines @@
 #include "components/metrics/client_info.h"
 #include "components/net_log/chrome_net_log.h"
 #include "components/password_manager/content/browser/content_password_manager_driver_factory.h"
 #include "components/payments/payment_request.mojom.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/pref_service.h"
 #include "components/prefs/scoped_user_pref_update.h"
 #include "components/rappor/public/rappor_utils.h"
 #include "components/rappor/rappor_recorder_impl.h"
 #include "components/rappor/rappor_service_impl.h"
+#include "components/safe_browsing_db/safe_browsing_prefs.h"
 #include "components/security_interstitials/core/ssl_error_ui.h"
 #include "components/signin/core/common/profile_management_switches.h"
 #include "components/spellcheck/spellcheck_build_features.h"
 #include "components/startup_metric_utils/browser/startup_metric_host_impl.h"
 #include "components/task_scheduler_util/initialization/browser_util.h"
 #include "components/task_scheduler_util/variations/browser_variations_util.h"
 #include "components/translate/core/common/translate_switches.h"
 #include "components/url_formatter/url_fixer.h"
 #include "components/variations/variations_associated_data.h"
 #include "components/version_info/version_info.h"
@@ skipping 506 matching lines @@
   if (!render_frame_host)
     return;
   WebContents* web_contents =
       WebContents::FromRenderFrameHost(render_frame_host);
   FlashDownloadInterception::InterceptFlashDownloadNavigation(web_contents,
                                                               source_url);
 }
 #endif  // BUILDFLAG(ENABLE_PLUGINS)
 
 // An implementation of the SSLCertReporter interface used by
-// SSLErrorHandler. Uses the SafeBrowsing UI manager to send invalid
-// certificate reports.
-class SafeBrowsingSSLCertReporter : public SSLCertReporter {
+// SSLErrorHandler. Uses CertificateReportingService to send reports. The
+// service handles queueing and re-sending of failed reports. Each certificate
+// error creates a new instance of this class.
+class CertificateReportingServiceCertReporter : public SSLCertReporter {

[estark, 2016/12/16 01:57:58]

Hrmmmmm. There is not much reason for this SSLCertReporter interface to exist
anymore. CertReportHelper in chrome/browser/ssl could just using the
CertificateReportingService directly.

We originally did this SSLCertReporter interface to avoid introducing a c/b/ssl
-> c/b/safe_browsing dependency... but now c/b/ssl depends on c/b/safe_browsing
all over the place.

So we could get rid of this interface. Or we could leave it as you have it and
file a bug to clean up the c/b/ssl => c/b/safe_browsing dependency. (I don't
think it would be that hard to clean it up.) I have a slight preference for the
latter, since it is kinda weird that c/b/ssl depends on safebrowsing. Do you
have any preference?

[meacer, 2016/12/16 20:26:35]

I've been avoiding adding more c/b/sb dependency to ssl so I'd prefer cleaning
out the dependencies instead.

As far as I can tell, the only dependencies are in tests and one other place
which I removed in the next CL
(chrome/browser/ssl/certificate_reporting_test_utils.cc). The rest of the code
in ssl depends on components/safe_browsing_db instead. But maybe I'm missing
something?

[estark, 2016/12/16 23:39:03]

I think removing the safe_browsing_db ones would be a reasonable thing to do
too, if possible. I filed a bug to look into it:
https://bugs.chromium.org/p/chromium/issues/detail?id=675245

  public:
-  explicit SafeBrowsingSSLCertReporter(
-      const scoped_refptr<safe_browsing::SafeBrowsingUIManager>&
-          safe_browsing_ui_manager)
-      : safe_browsing_ui_manager_(safe_browsing_ui_manager) {}
-  ~SafeBrowsingSSLCertReporter() override {}
+  explicit CertificateReportingServiceCertReporter(
+      CertificateReportingService* service)
+      : service_(service) {}
+  ~CertificateReportingServiceCertReporter() override {}
 
   // SSLCertReporter implementation
   void ReportInvalidCertificateChain(
       const std::string& serialized_report) override {
-    if (safe_browsing_ui_manager_) {
-      safe_browsing_ui_manager_->ReportInvalidCertificateChain(
-          serialized_report, base::Bind(&base::DoNothing));
-    }
+    service_->Send(serialized_report);
   }
 
  private:
-  const scoped_refptr<safe_browsing::SafeBrowsingUIManager>
-      safe_browsing_ui_manager_;
+  CertificateReportingService* service_;
 };
 
 #if BUILDFLAG(ANDROID_JAVA_UI)
 void HandleSingleTabModeBlockOnUIThread(const BlockedWindowParams& params) {
   WebContents* web_contents = tab_util::GetWebContentsByFrameID(
       params.render_process_id(), params.opener_render_frame_id());
   if (!web_contents)
     return;
 
   SingleTabModeTabHelper::FromWebContents(web_contents)->HandleOpenUrl(params);
@@ skipping 1506 matching lines @@
   // Otherwise, display an SSL blocking page. The interstitial page takes
   // ownership of ssl_blocking_page.
   int options_mask = 0;
   if (overridable)
     options_mask |= SSLErrorUI::SOFT_OVERRIDE_ENABLED;
   if (strict_enforcement)
     options_mask |= SSLErrorUI::STRICT_ENFORCEMENT;
   if (expired_previous_decision)
     options_mask |= SSLErrorUI::EXPIRED_BUT_PREVIOUSLY_ALLOWED;
 
-  safe_browsing::SafeBrowsingService* safe_browsing_service =
-      g_browser_process->safe_browsing_service();
-  std::unique_ptr<SafeBrowsingSSLCertReporter> cert_reporter(
-      new SafeBrowsingSSLCertReporter(safe_browsing_service
-                                          ? safe_browsing_service->ui_manager()
-                                          : nullptr));
+  CertificateReportingService* cert_reporting_service =
+      CertificateReportingServiceFactory::GetForBrowserContext(
+          web_contents->GetBrowserContext());
+  std::unique_ptr<CertificateReportingServiceCertReporter> cert_reporter(
+      new CertificateReportingServiceCertReporter(cert_reporting_service));
+
   SSLErrorHandler::HandleSSLError(web_contents, cert_error, ssl_info,
                                   request_url, options_mask,
                                   std::move(cert_reporter), callback);
 }
 
 void ChromeContentBrowserClient::SelectClientCertificate(
     content::WebContents* web_contents,
     net::SSLCertRequestInfo* cert_request_info,
     std::unique_ptr<content::ClientCertificateDelegate> delegate) {
   prerender::PrerenderContents* prerender_contents =
@@ skipping 1115 matching lines @@
           GetBrowserSchedulerWorkerPoolParamsFromVariations();
   *index_to_traits_callback = base::Bind(&task_scheduler_util::initialization::
                                              BrowserWorkerPoolIndexForTraits);
 }
 
 void ChromeContentBrowserClient::
     PerformExperimentalTaskSchedulerRedirections() {
   task_scheduler_util::variations::
       MaybePerformBrowserTaskSchedulerRedirection();
 }