Add extended reporting level in the update request

chrome/browser/safe_browsing/safe_browsing_service.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/safe_browsing/safe_browsing_service.h"
 
 #include <stddef.h>
 
 #include <vector>
 
@@ skipping 281 matching lines @@
 SafeBrowsingService* SafeBrowsingService::CreateSafeBrowsingService() {
   if (!factory_)
     factory_ = g_safe_browsing_service_factory_impl.Pointer();
   return factory_->CreateSafeBrowsingService();
 }
 
 SafeBrowsingService::SafeBrowsingService()
     : services_delegate_(ServicesDelegate::Create(this)),
       enabled_(false),
       enabled_by_prefs_(false),
+      maybe_enabled_extended_reporting_by_prefs_(false),
+      maybe_enabled_scout_reporting_by_prefs_(false),
       enabled_v4_only_(safe_browsing::V4FeatureList::IsV4OnlyEnabled()) {}
 
 SafeBrowsingService::~SafeBrowsingService() {
   // We should have already been shut down. If we're still enabled, then the
   // database isn't going to be closed properly, which could lead to corruption.
   DCHECK(!enabled_);
 }
 
 void SafeBrowsingService::Initialize() {
   // Ensure FileTypePolicies's Singleton is instantiated during startup.
@@ skipping 352 matching lines @@
     default:
       NOTREACHED();
   }
 }
 
 void SafeBrowsingService::AddPrefService(PrefService* pref_service) {
   DCHECK(prefs_map_.find(pref_service) == prefs_map_.end());
   std::unique_ptr<PrefChangeRegistrar> registrar =
       base::MakeUnique<PrefChangeRegistrar>();
   registrar->Init(pref_service);
-  registrar->Add(prefs::kSafeBrowsingEnabled,
-                 base::Bind(&SafeBrowsingService::RefreshState,
-                            base::Unretained(this)));
+  registrar->Add(
+      prefs::kSafeBrowsingEnabled,
+      base::Bind(&SafeBrowsingService::RefreshState, base::Unretained(this)));
   // ClientSideDetectionService will need to be refresh the models
   // renderers have if extended-reporting changes.
   registrar->Add(
       prefs::kSafeBrowsingExtendedReportingEnabled,
       base::Bind(&SafeBrowsingService::RefreshState, base::Unretained(this)));
   registrar->Add(
       prefs::kSafeBrowsingScoutReportingEnabled,
       base::Bind(&SafeBrowsingService::RefreshState, base::Unretained(this)));
   prefs_map_[pref_service] = std::move(registrar);
   RefreshState();
@@ skipping 27 matching lines @@
 std::unique_ptr<SafeBrowsingService::ShutdownSubscription>
 SafeBrowsingService::RegisterShutdownCallback(
     const base::Callback<void(void)>& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   return shutdown_callback_list_.Add(callback);
 }
 
 void SafeBrowsingService::RefreshState() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   // Check if any profile requires the service to be active.
-  bool enable = false;
+  enabled_by_prefs_ = false;
+  maybe_enabled_extended_reporting_by_prefs_ = false;

[lpz, 2017/01/25 16:24:47]

have you considered storing one field with the ExtendedReportingLevel enum here
rather than two bools? and just call
safe_browsing_prefs::GetExtendedReportingLevel below?

You would lose the ability to distinguish Scout from (scout with ER) vs (scout
without ER), but not sure that distinction matters? If it does matter,
GetExtendedReportingLevel could be extended do make that distinction.

One nice-to-have would be to avoid referencing
kSafeBrowsingExtendedReportingEnabled or kSafeBrowsingScoutReportingEnabled
outside of safe_browsing_prefs (if possible).

[vakh (use Gerrit instead), 2017/01/25 22:35:30]

Done.
Not that important.
Unfortunately it is already being used in this file but I am not adding another
use of it now.

+  maybe_enabled_scout_reporting_by_prefs_ = false;
   for (const auto& pref : prefs_map_) {
     if (pref.first->GetBoolean(prefs::kSafeBrowsingEnabled)) {
-      enable = true;
-      break;
+      enabled_by_prefs_ = true;
+      if (pref.first->GetBoolean(
+              prefs::kSafeBrowsingExtendedReportingEnabled)) {
+        maybe_enabled_extended_reporting_by_prefs_ = true;
+      }
+      if (pref.first->GetBoolean(prefs::kSafeBrowsingScoutReportingEnabled)) {
+        maybe_enabled_scout_reporting_by_prefs_ = true;
+      }
     }
   }
 
-  enabled_by_prefs_ = enable;
-
-  if (enable)
+  if (enabled_by_prefs_)
     Start();
   else
     Stop(false);
 
   state_callback_list_.Notify();
 
-  services_delegate_->RefreshState(enable);
+  services_delegate_->RefreshState(enabled_by_prefs_);
 }
 
 void SafeBrowsingService::SendSerializedDownloadReport(
     const std::string& report) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       base::Bind(&SafeBrowsingService::OnSendSerializedDownloadReport, this,
                  report));
 }
 
 void SafeBrowsingService::OnSendSerializedDownloadReport(
     const std::string& report) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   if (ping_manager())
     ping_manager()->ReportThreatDetails(report);
 }
 
 void SafeBrowsingService::ProcessResourceRequest(
     const ResourceRequestInfo& request) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   services_delegate_->ProcessResourceRequest(&request);
 }
 
 }  // namespace safe_browsing