chrome/browser/safe_browsing/certificate_reporting_metrics_provider.cc - Issue 2503243003: Wire up CertificateReportingService to handle report uploads

Side by Side Diff: chrome/browser/safe_browsing/certificate_reporting_metrics_provider.cc

Issue 2503243003: Wire up CertificateReportingService to handle report uploads (Closed)

Patch Set: jialiul, estark comments Created 4 years ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

« chrome/browser/chrome_content_browser_client.cc ('K') | « chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc ('k') | chrome/browser/safe_browsing/certificate_reporting_service.h » ('j') | chrome/browser/safe_browsing/certificate_reporting_service.h » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
1 // Copyright 2016 The Chromium Authors. All rights reserved.	1 // Copyright 2016 The Chromium Authors. All rights reserved.

2 // Use of this source code is governed by a BSD-style license that can be	2 // Use of this source code is governed by a BSD-style license that can be

3 // found in the LICENSE file.	3 // found in the LICENSE file.

4	4

5 #include "chrome/browser/safe_browsing/certificate_reporting_metrics_provider.h"	5 #include "chrome/browser/safe_browsing/certificate_reporting_metrics_provider.h"

6	6

7 #include "chrome/browser/browser_process.h"	7 #include "chrome/browser/browser_process.h"

8 #include "chrome/browser/profiles/profile_manager.h"	8 #include "chrome/browser/profiles/profile_manager.h"

9 #include "chrome/browser/safe_browsing/certificate_reporting_service.h"	9 #include "chrome/browser/safe_browsing/certificate_reporting_service.h"

10 #include "chrome/browser/safe_browsing/certificate_reporting_service_factory.h"	10 #include "chrome/browser/safe_browsing/certificate_reporting_service_factory.h"

11	11

12 CertificateReportingMetricsProvider::CertificateReportingMetricsProvider() {}	12 CertificateReportingMetricsProvider::CertificateReportingMetricsProvider() {}

13	13

14 CertificateReportingMetricsProvider::~CertificateReportingMetricsProvider() {}	14 CertificateReportingMetricsProvider::~CertificateReportingMetricsProvider() {}

15	15

16 void CertificateReportingMetricsProvider::ProvideGeneralMetrics(	16 void CertificateReportingMetricsProvider::ProvideGeneralMetrics(

17 metrics::ChromeUserMetricsExtension* unused) {}	17 metrics::ChromeUserMetricsExtension* unused) {

	18 ProfileManager* profile_manager = g_browser_process->profile_manager();

	19 if (!profile_manager)

	20 return;

	21 CertificateReportingService* service =

	22 CertificateReportingServiceFactory::GetForBrowserContext(
	estark 2016/12/16 23:39:04 I am very stupid for not realizing this earlier, b I am very stupid for not realizing this earlier, but I just realized that this is likely going to be crashy for the same reason that HttpsEngagementMetricsProvider was: https://crbug.com/671579 https://bugs.chromium.org/p/chromium/issues/detail?id=671579#c21 suggests that we should be listening to when profile initialization is complete. Do you have any idea how to do that...? meacer 2016/12/17 01:23:21 That's a scary bug. It looks like NOTIFICATION_PRO Show quoted text On 2016/12/16 23:39:04, estark wrote: > I am very stupid for not realizing this earlier, but I just realized that this > is likely going to be crashy for the same reason that > HttpsEngagementMetricsProvider was: https://crbug.com/671579 > > https://bugs.chromium.org/p/chromium/issues/detail?id=671579#c21 suggests that > we should be listening to when profile initialization is complete. Do you have > any idea how to do that...? That's a scary bug. It looks like NOTIFICATION_PROFILE_CREATED is sent after a profile is initialized, but I can't immediately tell if it's sent on a new profile creation or when an existing one is loaded. meacer 2016/12/21 23:11:05 I believe this is fixed now. The fix is based on h Show quoted text On 2016/12/17 01:23:21, Mustafa Emre Acer wrote: > On 2016/12/16 23:39:04, estark wrote: > > I am very stupid for not realizing this earlier, but I just realized that this > > is likely going to be crashy for the same reason that > > HttpsEngagementMetricsProvider was: https://crbug.com/671579 > > > > https://bugs.chromium.org/p/chromium/issues/detail?id=671579#c21 suggests that > > we should be listening to when profile initialization is complete. Do you have > > any idea how to do that...? > > That's a scary bug. It looks like NOTIFICATION_PROFILE_CREATED is sent after a > profile is initialized, but I can't immediately tell if it's sent on a new > profile creation or when an existing one is loaded. I believe this is fixed now. The fix is based on https://codereview.chromium.org/2531023002
	23 profile_manager->GetLastUsedProfile());

	24 if (service)

	25 service->SendPending();

	26 }

OLD	NEW