Wire up CertificateReportingService to handle report uploads

chrome/browser/safe_browsing/certificate_reporting_metrics_provider.cc

 // Copyright 2016 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/safe_browsing/certificate_reporting_metrics_provider.h"
 
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/safe_browsing/certificate_reporting_service.h"
 #include "chrome/browser/safe_browsing/certificate_reporting_service_factory.h"
 
 CertificateReportingMetricsProvider::CertificateReportingMetricsProvider() {}
 
 CertificateReportingMetricsProvider::~CertificateReportingMetricsProvider() {}
 
 void CertificateReportingMetricsProvider::ProvideGeneralMetrics(
-    metrics::ChromeUserMetricsExtension* unused) {}
+    metrics::ChromeUserMetricsExtension* unused) {
+  ProfileManager* profile_manager = g_browser_process->profile_manager();
+  if (!profile_manager)
+    return;
+
+  // Do not try to create profile here if it does not exist, because this method
+  // can be called during browser shutdown.
+  Profile* profile = profile_manager->GetProfileByPath(
+      profile_manager->GetLastUsedProfileDir(profile_manager->user_data_dir()));
+  if (!profile)
+    return;
+
+  CertificateReportingService* service =
+      CertificateReportingServiceFactory::GetForBrowserContext(profile);
+  if (service)
+    service->SendPending();

[Ilya Sherman, 2016/12/22 01:41:57]

Hmm, this doesn't look like it's actually related to UMA metrics.  Am I
misunderstanding something here?  Why is this being hooked up as a metrics
provider?

[meacer, 2016/12/22 18:50:04]

Sorry, I split this feature into multiple CLs and that caused loss of context.

The main feature is to retry uploads of invalid SSL certificate reports.
Currently, the reports are uploaded only once when the user sees an SSL error.
This isn't always sufficient though, as the user might be behind a captive
portal which blocks all connections to the internet. For that reason, this
feature queues failed uploads and retries them at a later time. All of this
logic is in CertificateReportingService.

However, CertificateReportingService doesn't do any scheduling. We wanted to
re-use the scheduling logic of MetricsService instead. What happens here is that
whenever this metrics provider is asked to provide metrics, the
CertificateReportingService tries uploading queued (i.e. pending) reports.

This is very much like HttpsEngagementMetricsProvider which also relies on the
metrics service to do the scheduling. It's not related to UMA at all, the
histogram mentioned in the CL is an existing histogram moved from
SafeBrowsingPingManager.

Hope that helps, I can explain further if needed.

[Ilya Sherman, 2016/12/22 22:00:08]

Okay, that kind of makes sense.  I'm not 100% sure that it's a good idea to
piggyback off of the UMA scheduler, but I also can't think of any specific
reason why it's a bad idea, so maybe it's okay.  Please add documentation to the
code, though, to explain that this is what's happening.  Otherwise, I could
imagine a future reader of this code being quite confused =)

[meacer, 2016/12/22 22:23:36]

Sure, expanded the documentation a bit. Hopefully it's clearer now. 

+}