ThreadPool/Android: Add a flag/feature to enable background group

We are currently using just the foreground-thread-priority ThreadPool
group on Android. This patch adds commandline and feature flags to
support a field experiment that enables the background-thread-priority
group.

Because this ThreadPool setting is accessed before FieldTrials are
initialized, the FieldTrial setting is cached in a Java preference and
applied on the next start of the application. The applied field trial
group is then reported via a synthetic field trial.

Test: Locally via --enable/disable-features=BackgroundThreadPool
Bug: 1169694
Change-Id: I3c5da7ed8e5415e10bc605521e6b0ee48d76509c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2756513
Reviewed-by: Benoit L <lizeb@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Henrique Nakashima <hnakashima@chromium.org>
Commit-Queue: Eric Seckler <eseckler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#862883}

base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java

@@ -71,6 +71,9 @@ public class LibraryLoader {
     // Default sampling interval for reached code profiler in microseconds.
     private static final int DEFAULT_REACHED_CODE_SAMPLING_INTERVAL_US = 10000;
 
+    // Shared preferences key for the background thread pool setting.
+    private static final String BACKGROUND_THREAD_POOL_KEY = "background_thread_pool_enabled";
+
     // The singleton instance of LibraryLoader. Never null (not final for tests).
     private static LibraryLoader sInstance = new LibraryLoader();
 
@@ -587,6 +590,33 @@ public static int getReachedCodeSamplingIntervalUs() {
         }
     }
 
+    /**
+     * Enables the background priority thread pool group. The value comes from the
+     * "BackgroundThreadPool" finch experiment, and is pushed on every run, to take effect on the
+     * subsequent run. I.e. the effect of the finch experiment lags by one run, which is the best we
+     * can do considering that the thread pool has to be configured before finch is initialized.
+     * Note that since LibraryLoader is in //base, it can't depend on ChromeFeatureList, and has to
+     * rely on external code pushing the value.
+     *
+     * @param enabled whether to enable the background priority thread pool group.
+     */
+    public static void setBackgroundThreadPoolEnabledOnNextRuns(boolean enabled) {
+        SharedPreferences.Editor editor = ContextUtils.getAppSharedPreferences().edit();
+        editor.putBoolean(BACKGROUND_THREAD_POOL_KEY, enabled).apply();
+    }
+
+    /**
+     * @return whether the background priority thread pool group should be enabled. (see
+     *         setBackgroundThreadPoolEnabledOnNextRuns()).
+     */
+    @VisibleForTesting
+    public static boolean isBackgroundThreadPoolEnabled() {
+        try (StrictModeContext ignored = StrictModeContext.allowDiskReads()) {
+            return ContextUtils.getAppSharedPreferences().getBoolean(
+                    BACKGROUND_THREAD_POOL_KEY, false);
+        }
+    }
+
     private void loadWithChromiumLinker(ApplicationInfo appInfo, String library) {
         Linker linker = getLinker();
 
@@ -756,16 +786,23 @@ private void initializeAlreadyLocked() {
         if (mInitialized) return;
         assert mLibraryProcessType != LibraryProcessType.PROCESS_UNINITIALIZED;
 
-        // Add a switch for the reached code profiler as late as possible since it requires a read
-        // from the shared preferences. At this point the shared preferences are usually warmed up.
         if (mLibraryProcessType == LibraryProcessType.PROCESS_BROWSER) {
+            // Add a switch for the reached code profiler as late as possible since it requires a
+            // read from the shared preferences. At this point the shared preferences are usually
+            // warmed up.
             int reachedCodeSamplingIntervalUs = getReachedCodeSamplingIntervalUs();
             if (reachedCodeSamplingIntervalUs > 0) {
                 CommandLine.getInstance().appendSwitch(BaseSwitches.ENABLE_REACHED_CODE_PROFILER);
                 CommandLine.getInstance().appendSwitchWithValue(
                         BaseSwitches.REACHED_CODE_SAMPLING_INTERVAL_US,
                         Integer.toString(reachedCodeSamplingIntervalUs));
             }
+
+            // Similarly, append a switch to enable the background thread pool group if the cached
+            // preference indicates it should be enabled.
+            if (isBackgroundThreadPoolEnabled()) {
+                CommandLine.getInstance().appendSwitch(BaseSwitches.ENABLE_BACKGROUND_THREAD_POOL);
+            }
         }
 
         ensureCommandLineSwitchedAlreadyLocked();

base/base_switches.cc

@@ -31,6 +31,11 @@ const char kEnableFeatures[] = "enable-features";
 // Force low-end device mode when set.
 const char kEnableLowEndDeviceMode[]        = "enable-low-end-device-mode";
 
+// Enable the use of background thread priorities for background tasks in the
+// ThreadPool even on systems where it is disabled by default, e.g. due to
+// concerns about priority inversions.
+const char kEnableBackgroundThreadPool[] = "enable-background-thread-pool";
+
 // This option can be used to force field trials when testing changes locally.
 // The argument is a list of name and value pairs, separated by slashes. If a
 // trial name is prefixed with an asterisk, that trial will start activated.

base/base_switches.h

@@ -19,6 +19,7 @@ extern const char kDisableLowEndDeviceMode[];
 extern const char kEnableCrashReporter[];
 extern const char kEnableFeatures[];
 extern const char kEnableLowEndDeviceMode[];
+extern const char kEnableBackgroundThreadPool[];
 extern const char kForceFieldTrials[];
 extern const char kFullMemoryCrashReport[];
 extern const char kLogBestEffortTasks[];

base/task/thread_pool/environment_config.cc

@@ -4,6 +4,8 @@
 
 #include "base/task/thread_pool/environment_config.h"
 
+#include "base/base_switches.h"
+#include "base/command_line.h"
 #include "base/synchronization/lock.h"
 #include "base/threading/platform_thread.h"
 #include "build/build_config.h"
@@ -14,6 +16,12 @@ namespace internal {
 namespace {
 
 bool CanUseBackgroundPriorityForWorkerThreadImpl() {
+  // Commandline flag overrides (e.g. for experiments).
+  if (CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableBackgroundThreadPool)) {
+    return true;
+  }
+
   // When Lock doesn't handle multiple thread priorities, run all
   // WorkerThread with a normal priority to avoid priority inversion when a
   // thread running with a normal priority tries to acquire a lock held by a

chrome/browser/chrome_browser_field_trials.cc

@@ -29,8 +29,10 @@
 #if defined(OS_ANDROID)
 #include "base/android/build_info.h"
 #include "base/android/bundle_utils.h"
+#include "base/task/thread_pool/environment_config.h"
 #include "chrome/browser/chrome_browser_field_trials_mobile.h"
 #include "chrome/browser/flags/android/cached_feature_flags.h"
+#include "chrome/browser/flags/android/chrome_feature_list.h"
 #include "chrome/common/chrome_features.h"
 #endif
 
@@ -110,26 +112,59 @@ void ChromeBrowserFieldTrials::RegisterSyntheticTrials() {
         kReachedCodeProfilerTrial, reached_code_profiler_group);
   }
 
-  const char* group_name;
-  bool java_feature_enabled =
-      chrome::android::IsJavaDrivenFeatureEnabled(features::kEarlyLibraryLoad);
-  bool feature_enabled =
-      base::FeatureList::IsEnabled(features::kEarlyLibraryLoad);
-  // Use the default group if cc and java feature values don't agree (can happen
-  // on first startup after feature is enabled by Finch), or the feature is not
-  // overridden by Finch.
-  if (feature_enabled != java_feature_enabled ||
-      !base::FeatureList::GetInstance()->IsFeatureOverridden(
-          features::kEarlyLibraryLoad.name)) {
-    group_name = "Default";
-  } else if (java_feature_enabled) {
-    group_name = "Enabled";
-  } else {
-    group_name = "Disabled";
+  {
+    // EarlyLibraryLoadSynthetic field trial.
+    const char* group_name;
+    bool java_feature_enabled = chrome::android::IsJavaDrivenFeatureEnabled(
+        features::kEarlyLibraryLoad);
+    bool feature_enabled =
+        base::FeatureList::IsEnabled(features::kEarlyLibraryLoad);
+    // Use the default group if cc and java feature values don't agree (can
+    // happen on first startup after feature is enabled by Finch), or the
+    // feature is not overridden by Finch.
+    if (feature_enabled != java_feature_enabled ||
+        !base::FeatureList::GetInstance()->IsFeatureOverridden(
+            features::kEarlyLibraryLoad.name)) {
+      group_name = "Default";
+    } else if (java_feature_enabled) {
+      group_name = "Enabled";
+    } else {
+      group_name = "Disabled";
+    }
+    static constexpr char kEarlyLibraryLoadTrial[] =
+        "EarlyLibraryLoadSynthetic";
+    ChromeMetricsServiceAccessor::RegisterSyntheticFieldTrial(
+        kEarlyLibraryLoadTrial, group_name);
+  }
+
+  {
+    // BackgroundThreadPoolSynthetic field trial.
+    const char* group_name;
+    // Target group as indicated by finch feature.
+    bool feature_enabled =
+        base::FeatureList::IsEnabled(chrome::android::kBackgroundThreadPool);
+    bool feature_overridden =
+        base::FeatureList::GetInstance()->IsFeatureOverridden(
+            chrome::android::kBackgroundThreadPool.name);
+    // The finch feature value is cached by Java in a setting and applied via a
+    // command line flag. Check if this has happened -- it may not have happened
+    // if this is the first startup after the feature is enabled.
+    bool actually_enabled =
+        base::internal::CanUseBackgroundPriorityForWorkerThread();
+    // Use the default group if either the feature wasn't overridden by Finch,
+    // or if the feature target state and actual state don't agree.
+    if (actually_enabled != feature_enabled || !feature_overridden) {
+      group_name = "Default";
+    } else if (feature_enabled) {
+      group_name = "Enabled";
+    } else {
+      group_name = "Disabled";
+    }
+    static constexpr char kBackgroundThreadPoolTrial[] =
+        "BackgroundThreadPoolSynthetic";
+    ChromeMetricsServiceAccessor::RegisterSyntheticFieldTrial(
+        kBackgroundThreadPoolTrial, group_name);
   }
-  static constexpr char kEarlyLibraryLoadTrial[] = "EarlyLibraryLoadSynthetic";
-  ChromeMetricsServiceAccessor::RegisterSyntheticFieldTrial(
-      kEarlyLibraryLoadTrial, group_name);
 
   // If isolated splits are enabled at build time, Monochrome and Trichrome will
   // have a different bundle layout, so measure N+ even though isolated splits

chrome/browser/flags/android/chrome_feature_list.cc

@@ -138,6 +138,7 @@ const base::Feature* kFeaturesExposedToJava[] = {
     &kAssistantIntentPageUrl,
     &kAssistantIntentTranslateInfo,
     &kAppLaunchpad,
+    &kBackgroundThreadPool,
     &kBentoOffline,
     &kCastDeviceFilter,
     &kCloseTabSuggestions,
@@ -363,6 +364,9 @@ const base::Feature kAppLaunchpad{"AppLaunchpad",
 const base::Feature kBackgroundTaskComponentUpdate{
     "BackgroundTaskComponentUpdate", base::FEATURE_DISABLED_BY_DEFAULT};
 
+const base::Feature kBackgroundThreadPool{"BackgroundThreadPool",
+                                          base::FEATURE_DISABLED_BY_DEFAULT};
+
 const base::Feature kBentoOffline{"BentoOffline",
                                   base::FEATURE_DISABLED_BY_DEFAULT};
 

chrome/browser/flags/android/chrome_feature_list.h

@@ -24,6 +24,7 @@ extern const base::Feature kAssistantIntentPageUrl;
 extern const base::Feature kAssistantIntentTranslateInfo;
 extern const base::Feature kAppLaunchpad;
 extern const base::Feature kBackgroundTaskComponentUpdate;
+extern const base::Feature kBackgroundThreadPool;
 extern const base::Feature kBentoOffline;
 extern const base::Feature kCloseTabSuggestions;
 extern const base::Feature kCriticalPersistedTabData;

chrome/browser/flags/android/java/src/org/chromium/chrome/browser/flags/CachedFeatureFlags.java

@@ -256,6 +256,10 @@ public static void cacheAdditionalNativeFlags() {
                 ChromeFeatureList.isEnabled(ChromeFeatureList.REACHED_CODE_PROFILER),
                 ChromeFeatureList.getFieldTrialParamByFeatureAsInt(
                         ChromeFeatureList.REACHED_CODE_PROFILER, "sampling_interval_us", 0));
+
+        // Similarly, propagate the BACKGROUND_THREAD_POOL feature value to LibraryLoader.
+        LibraryLoader.setBackgroundThreadPoolEnabledOnNextRuns(
+                ChromeFeatureList.isEnabled(ChromeFeatureList.BACKGROUND_THREAD_POOL));
     }
 
     /**

chrome/browser/flags/android/java/src/org/chromium/chrome/browser/flags/ChromeFeatureList.java

@@ -232,6 +232,7 @@ public static boolean getFieldTrialParamByFeatureAsBoolean(
     public static final String AUTOFILL_REFRESH_STYLE_ANDROID = "AutofillRefreshStyleAndroid";
     public static final String AUTOFILL_KEYBOARD_ACCESSORY = "AutofillKeyboardAccessory";
     public static final String APP_LAUNCHPAD = "AppLaunchpad";
+    public static final String BACKGROUND_THREAD_POOL = "BackgroundThreadPool";
     public static final String BENTO_OFFLINE = "BentoOffline";
     public static final String BIOMETRIC_TOUCH_TO_FILL = "BiometricTouchToFill";
     public static final String CAPTIVE_PORTAL_CERTIFICATE_LIST = "CaptivePortalCertificateList";

content/browser/gpu/gpu_process_host.cc

@@ -248,6 +248,7 @@ static const char* const kSwitchNames[] = {
     switches::kDisableShaderNameHashing,
     switches::kDisableSkiaRuntimeOpts,
     switches::kDisableWebRtcHWEncoding,
+    switches::kEnableBackgroundThreadPool,
     switches::kEnableGpuRasterization,
     switches::kEnableLogging,
     switches::kEnableDeJelly,

content/browser/renderer_host/render_process_host_impl.cc

@@ -3184,6 +3184,7 @@ void RenderProcessHostImpl::PropagateBrowserCommandLineToRenderer(
     switches::kDomAutomationController,
     switches::kEnableAccessibilityObjectModel,
     switches::kEnableAutomation,
+    switches::kEnableBackgroundThreadPool,
     switches::kEnableExperimentalAccessibilityLanguageDetection,
     switches::kEnableExperimentalAccessibilityLabelsDebugging,
     switches::kEnableExperimentalWebPlatformFeatures,

content/browser/utility_process_host.cc

@@ -240,6 +240,7 @@ bool UtilityProcessHost::StartProcess() {
       os_crypt::switches::kUseMockKeychain,
 #endif
       switches::kDisableTestCerts,
+      switches::kEnableBackgroundThreadPool,
       switches::kEnableExperimentalCookieFeatures,
       switches::kEnableLogging,
       switches::kForceTextDirection,