Enable Initialization of the Task Scheduler by Default in the Browser Process

chrome/browser/chrome_browser_main.cc

Index: chrome/browser/chrome_browser_main.cc
diff --git a/chrome/browser/chrome_browser_main.cc b/chrome/browser/chrome_browser_main.cc
index ef8d8ccada69f44a348f08479ff86379c8f99f86..0bc8c5ac9b3380c8f7049d4a8d236703d1941c3c 100644
--- a/chrome/browser/chrome_browser_main.cc
+++ b/chrome/browser/chrome_browser_main.cc
@@ -316,16 +316,18 @@ void AddFirstRunNewTabs(StartupBrowserCreator* browser_creator,
 #endif  // !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
 
 void MaybeInitializeTaskScheduler() {

[fdoray, 2016/11/15 18:40:17]

No Maybe

[robliao, 2016/11/16 19:04:17]

Done.

+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kDisableBrowserTaskScheduler)) {
+    return;
+  }

[fdoray, 2016/11/15 18:40:16]

To migrate WorkerPool call sites, TaskScheduler must always be initialized. Can
you always initialize TaskScheduler and re-purpose this flag to enable/disable
redirection only? You'll need to update
IDS_FLAGS_BROWSER_TASK_SCHEDULER_DESCRIPTION.

[gab, 2016/11/15 20:40:33]

Agreed, can't ever have a disabled scheduler.

As discussed offline, update definition, but don't change flag name to avoid
toggling people that already use it off of it.

[robliao, 2016/11/16 19:04:17]

Done.

[robliao, 2016/11/16 19:04:17]

Done.

+
   static constexpr char kFieldTrialName[] = "BrowserScheduler";
   std::map<std::string, std::string> variation_params;
   if (!variations::GetVariationParams(kFieldTrialName, &variation_params)) {
-    DCHECK(!base::CommandLine::ForCurrentProcess()->HasSwitch(
-        switches::kEnableBrowserTaskScheduler))
-        << "The Browser Task Scheduler remains disabled with "
-        << switches::kEnableBrowserTaskScheduler
-        << " because there is no available variation param for this build or "
-           " the task scheduler is disabled in chrome://flags.";
-    return;
+    variation_params["Background"] = "3;8;0.1;0;30000";
+    variation_params["BackgroundFileIO"] = "3;8;0.1;0;30000";
+    variation_params["Foreground"] = "8;32;0.3;0;30000";
+    variation_params["ForegroundFileIO"] = "8;32;0.3;0;30000";

[fdoray, 2016/11/15 18:40:16]

To avoid duplication between iOS and other platforms, could you move these
values to components/task_scheduler_util/initialization_util.cc? You could fall
back on default values when some params are missing/invalid. You could even
support a combination of default values and field trial values.

[gab, 2016/11/15 20:40:33]

Need different defaults for OS_ANDROID too to match current Finch config.

Also, it's weird that c/task_scheduler_util/initialization_util.cc is browser
process specific. Can we find a more browser-specific name?

[robliao, 2016/11/16 19:04:17]

That's a natural result of the content DEPS restriction. Added Browser to the
function name.

[robliao, 2016/11/16 19:04:17]

If some params are missing and invalid, we probably want to just use the default
ones wholesale. Merging two configurations could lead to an unexpected
configuration combination.

As for where this lives, it's possible that we would use different values for
iOS, and so we would have to split it out in the future, so keeping it separate
seems reasonable for now. If we find values converging, then we can consider
centralizing.

   }
 
   if (!task_scheduler_util::InitializeDefaultTaskScheduler(variation_params))