Lower the priority of environment and process data collection.

chrome/browser/safe_browsing/incident_reporting/environment_data_collection.cc

Index: chrome/browser/safe_browsing/incident_reporting/environment_data_collection.cc
diff --git a/chrome/browser/safe_browsing/incident_reporting/environment_data_collection.cc b/chrome/browser/safe_browsing/incident_reporting/environment_data_collection.cc
index f207502a3ac742370b8253646d1c654def211c02..069649e6d8739c317dc791fb4ee01420267ae134 100644
--- a/chrome/browser/safe_browsing/incident_reporting/environment_data_collection.cc
+++ b/chrome/browser/safe_browsing/incident_reporting/environment_data_collection.cc
@@ -8,9 +8,11 @@
 
 #include "base/cpu.h"
 #include "base/sys_info.h"
+#include "base/threading/platform_thread.h"
 #include "chrome/common/channel_info.h"
 #include "chrome/common/safe_browsing/csd.pb.h"
 #include "components/version_info/version_info.h"
+#include "content/public/browser/browser_thread.h"
 
 namespace safe_browsing {
 
@@ -62,6 +64,13 @@ void CollectProcessData(ClientIncidentReport_EnvironmentData_Process* process) {
 }  // namespace
 
 void CollectEnvironmentData(ClientIncidentReport_EnvironmentData* data) {
+  // Toggling priority only makes sense in a thread pool.
+  DCHECK(content::BrowserThread::GetBlockingPool()->RunsTasksOnCurrentThread());

[grt (UTC plus 2), 2015/12/09 14:35:25]

i don't like the dependence on BrowserThread here. gab: are you okay with
changing this to:
  DCHECK(base::SequencedWorkerPool::GetWorkerPoolForCurrentThread());

an alternative is to make a generic "run a callback at bg priority" wrapper that
is used by the IncidentReportingService for this specific task. the same wrapper
could be used for any other tasks elsewhere that could also benefit from running
at bg priority.

[gab, 2015/12/09 21:51:01]

Yea that works :-) (interestingly this call was recently added and not for this
purpose but nice to see that we can now make this check without depending on
BrowserThread :-))

Is this function called even when there is no incident? i.e. how likely is a
developer to hit this and realize they've broken something if this was to ever
run on a MessageLoop thread?
Yea but not sure I want a generic solution as (1) if too many callers do this we
could end up with all threads backgrounded which may hurt some other things
(e.g. async load of pref file for secondary profile is done on BlockingPool
today yet is a high priority task -- addressed by TaskScheduler API) and (2)
this will be supported by the TaskScheduler API which thus reduces the need to
introduce something for the long term here.

[proberge, 2015/12/10 21:00:57]

This doesn't run if there's no incident, but most developers will trigger this
code as there's a BinaryIncident for development builds.

+  // Lower priority for this task.
+  auto previous_priority = base::PlatformThread::GetCurrentThreadPriority();
+  base::PlatformThread::SetCurrentThreadPriority(
+      base::ThreadPriority::BACKGROUND);
+
   // OS
   {
     ClientIncidentReport_EnvironmentData_OS* os = data->mutable_os();
@@ -82,6 +91,9 @@ void CollectEnvironmentData(ClientIncidentReport_EnvironmentData* data) {
 
   // Process
   CollectProcessData(data->mutable_process());
+
+  // Reset the thread priority.
+  base::PlatformThread::SetCurrentThreadPriority(previous_priority);
 }
 
 #if !defined(OS_WIN)