Add chrome channel to cert logger reports

chrome/browser/ssl/cert_report_helper.cc

 // Copyright 2015 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/cert_report_helper.h"
 
 #include <utility>
 
 #include "base/logging.h"
 #include "base/metrics/field_trial.h"
 #include "base/rand_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ssl/ssl_cert_reporter.h"
+#include "chrome/common/channel_info.h"
 #include "chrome/common/pref_names.h"
 #include "components/prefs/pref_service.h"
 #include "components/safe_browsing/common/safe_browsing_prefs.h"
 #include "components/security_interstitials/core/controller_client.h"
 #include "components/security_interstitials/core/metrics_helper.h"
 #include "components/strings/grit/components_strings.h"
 #include "components/variations/variations_associated_data.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/web_contents.h"
 #include "ui/base/l10n/l10n_util.h"
@@ skipping 89 matching lines @@
   }
 
   if (!ShouldReportCertificateError())
     return;
 
   std::string serialized_report;
   certificate_reporting::ErrorReport report(request_url_.host(), ssl_info_);
 
   report.AddNetworkTimeInfo(g_browser_process->network_time_tracker());
 
+  report.AddChromeChannel(chrome::GetChannel());
+

[estark, 2017/07/01 17:19:01]

It would be nice to have a test for this line... but I can't think of any way to
test it. :( meacer, can you think of anything?

[Mustafa Acer, 2017/07/01 20:14:03]

The obvious way is to write a browser test that:
- Initiates an https server
- Enables the reporting option
- Visits the a broken SSL URL and navigates away to trigger a report
- Checks the report's contents

CertificateReportingServiceBrowserTest does something similar. The simplest test
there is OptedIn_ShouldSendSuccessfulReport, perhaps you can use it as a
starting point?

Actually, instead of setting up everything again in a new test file perhaps you
can just add a new test to CertificateReportingServiceBrowserTest and check the
report contents.

estark: Would that work?

[estark, 2017/07/02 00:09:12]

That sounds good! I was thinking that I'm not sure what chrome::GetChannel()
will return in a test, but maybe it doesn't matter; we can just check that the
report has a non-default value for the channel.

(The only other thing is that I'm not seeing that any of those
CertificateReportingServiceBrowserTests actually check the report contents.
There's a private CheckReports method but it doesn't look like it's called
anywhere; maybe that's supposed to be called from somewhere?)

(Also, sorry for the weekend emails, Sasha -- just wanted to make sure I wasn't
blocking this while I'm on vacation next week.)

[meacer, 2017/07/05 20:13:19]

Just chatted with Sasha. It seems easier to just augment
MockSSLCertReporter::ReportInvalidCertificateChain() and add channel information
to the report sent callback, and then check it in TestBrokenHTTPSReporting() in
ssl_browser_tests.cc. Do you concur?
I have little memory of this code, but I think I stopped checking  report
contents because this was already done elsewhere (SSLUITest and other places).
It made the tests simpler and avoided duplicate test cases. CheckReports() is a
remnant of that old code and is no longer necessary, I can delete it.

   report.SetInterstitialInfo(
       interstitial_reason_, user_proceeded,
       overridable_
           ? certificate_reporting::ErrorReport::INTERSTITIAL_OVERRIDABLE
           : certificate_reporting::ErrorReport::INTERSTITIAL_NOT_OVERRIDABLE,
       interstitial_time_);
 
   if (!report.Serialize(&serialized_report)) {
     LOG(ERROR) << "Failed to serialize certificate report.";
     return;
@@ skipping 40 matching lines @@
       if (sendingThreshold >= 0.0 && sendingThreshold <= 1.0)
         return base::RandDouble() <= sendingThreshold;
     }
   }
   return false;
 }
 
 bool CertReportHelper::IsPrefEnabled(const char* pref) {
   return GetProfile(web_contents_)->GetPrefs()->GetBoolean(pref);
 }