android: Enable ServiceGroupImportance

Remove the separate base::Feature, and tie enabling this to
when site isolation is enabled.

Bug: 946216
Change-Id: Ic14262b2fde78858b8f67f4e84ed35e199c2d3b4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2354271
Commit-Queue: Bo <boliu@chromium.org>
Reviewed-by: Brian White <bcwhite@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797987}

content/browser/BUILD.gn

@@ -375,7 +375,6 @@ source_set("browser") {
     "android/browser_startup_controller.cc",
     "android/browser_startup_controller.h",
     "android/content_feature_list.cc",
-    "android/content_feature_list.h",
     "android/content_startup_flags.cc",
     "android/content_startup_flags.h",
     "android/content_url_loader_factory.cc",

content/browser/android/content_feature_list.cc

@@ -2,8 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "content/browser/android/content_feature_list.h"
-
 #include "base/android/jni_string.h"
 #include "base/feature_list.h"
 #include "base/notreached.h"
@@ -26,7 +24,6 @@ const base::Feature* kFeaturesExposedToJava[] = {
     &features::kBackgroundMediaRendererHasModerateBinding,
     &features::kWebBluetoothNewPermissionsBackend,
     &features::kWebNfc,
-    &kServiceGroupImportance,
 };
 
 const base::Feature* FindFeatureExposedToJava(const std::string& feature_name) {
@@ -41,10 +38,6 @@ const base::Feature* FindFeatureExposedToJava(const std::string& feature_name) {
 
 }  // namespace
 
-// Alphabetical:
-const base::Feature kServiceGroupImportance{"ServiceGroupImportance",
-                                            base::FEATURE_DISABLED_BY_DEFAULT};
-
 static jboolean JNI_ContentFeatureListImpl_IsEnabled(
     JNIEnv* env,
     const JavaParamRef<jstring>& jfeature_name) {

content/browser/android/content_feature_list.h

-// Copyright 2017 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.
-
-#ifndef CONTENT_BROWSER_ANDROID_CONTENT_FEATURE_LIST_H_
-#define CONTENT_BROWSER_ANDROID_CONTENT_FEATURE_LIST_H_
-
-#include "base/feature_list.h"
-
-namespace content {
-namespace android {
-
-// Alphabetical:
-extern const base::Feature kServiceGroupImportance;
-
-}  // namespace android
-}  // namespace content
-
-#endif  // CONTENT_BROWSER_ANDROID_CONTENT_FEATURE_LIST_H_

content/browser/child_process_launcher_helper_android.cc

@@ -22,6 +22,7 @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/child_process_launcher_utils.h"
 #include "content/public/browser/render_process_host.h"
+#include "content/public/browser/site_isolation_policy.h"
 #include "content/public/common/content_descriptors.h"
 #include "content/public/common/content_switches.h"
 #include "sandbox/policy/switches.h"
@@ -214,6 +215,15 @@ static void JNI_ChildProcessLauncherHelperImpl_SetTerminationInfo(
   info->best_effort_reverse_rank = reverse_rank;
 }
 
+static jboolean
+JNI_ChildProcessLauncherHelperImpl_ServiceGroupImportanceEnabled(JNIEnv* env) {
+  // Not this is called on the launcher thread, not UI thread.
+  return SiteIsolationPolicy::AreIsolatedOriginsEnabled() ||
+         SiteIsolationPolicy::UseDedicatedProcessesForAllSites() ||
+         SiteIsolationPolicy::AreDynamicIsolatedOriginsEnabled() ||
+         SiteIsolationPolicy::ArePreloadedIsolatedOriginsEnabled();
+}
+
 // static
 bool ChildProcessLauncherHelper::TerminateProcess(const base::Process& process,
                                                   int exit_code) {

content/browser/renderer_host/render_widget_host_view_android.cc

@@ -39,7 +39,6 @@
 #include "components/viz/common/surfaces/parent_local_surface_id_allocator.h"
 #include "content/browser/accessibility/browser_accessibility_manager_android.h"
 #include "content/browser/accessibility/web_contents_accessibility_android.h"
-#include "content/browser/android/content_feature_list.h"
 #include "content/browser/android/gesture_listener_manager.h"
 #include "content/browser/android/ime_adapter_android.h"
 #include "content/browser/android/overscroll_controller_android.h"

content/browser/web_contents/web_contents_view_android.cc

@@ -12,7 +12,6 @@
 #include "base/optional.h"
 #include "cc/layers/layer.h"
 #include "content/browser/accessibility/browser_accessibility_manager_android.h"
-#include "content/browser/android/content_feature_list.h"
 #include "content/browser/android/content_ui_event_handler.h"
 #include "content/browser/android/gesture_listener_manager.h"
 #include "content/browser/android/select_popup.h"

content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncherHelperImpl.java

@@ -73,8 +73,8 @@ public final class ChildProcessLauncherHelperImpl {
     // To be conservative, only delay removing binding in the initial second of the process.
     private static final int TIMEOUT_FOR_DELAY_BINDING_REMOVE_MS = 1000;
 
-    // Flag to check features after native is initialized.
-    private static boolean sCheckedFeatures;
+    // Flag to check if ServiceGroupImportance should be enabled after native is initialized.
+    private static boolean sCheckedServiceGroupImportance;
 
     // A warmed-up connection to a sandboxed service.
     private static SpareChildConnection sSpareSandboxedConnection;
@@ -268,10 +268,10 @@ public final class ChildProcessLauncherHelperImpl {
                 commandLine, filesToBeMapped, sandboxed, canUseWarmUpConnection, binderCallback);
         helper.start();
 
-        if (!sCheckedFeatures) {
-            sCheckedFeatures = true;
+        if (sandboxed && !sCheckedServiceGroupImportance) {
+            sCheckedServiceGroupImportance = true;
             if (sSandboxedChildConnectionRanking != null
-                    && ContentFeatureList.isEnabled(ContentFeatureList.SERVICE_GROUP_IMPORTANCE)) {
+                    && ChildProcessLauncherHelperImplJni.get().serviceGroupImportanceEnabled()) {
                 sSandboxedChildConnectionRanking.enableServiceGroupImportance();
             }
         }
@@ -766,5 +766,7 @@ public final class ChildProcessLauncherHelperImpl {
         void setTerminationInfo(long termiantionInfoPtr, @ChildBindingState int bindingState,
                 boolean killedByUs, boolean cleanExit, boolean exceptionDuringInit,
                 int remainingStrong, int remainingModerate, int remainingWaived, int reverseRank);
+
+        boolean serviceGroupImportanceEnabled();
     }
 }

content/public/android/java/src/org/chromium/content_public/browser/ContentFeatureList.java

@@ -26,8 +26,6 @@ public class ContentFeatureList {
     public static final String BACKGROUND_MEDIA_RENDERER_HAS_MODERATE_BINDING =
             "BackgroundMediaRendererHasModerateBinding";
 
-    public static final String SERVICE_GROUP_IMPORTANCE = "ServiceGroupImportance";
-
     public static final String WEB_BLUETOOTH_NEW_PERMISSIONS_BACKEND =
             "WebBluetoothNewPermissionsBackend";
 

testing/variations/fieldtrial_testing_config.json

@@ -6641,22 +6641,6 @@
             ]
         }
     ],
-    "ServiceGroupImportance": [
-        {
-            "platforms": [
-                "android",
-                "android_weblayer"
-            ],
-            "experiments": [
-                {
-                    "name": "Enabled",
-                    "enable_features": [
-                        "ServiceGroupImportance"
-                    ]
-                }
-            ]
-        }
-    ],
     "ServiceManagerForBackgroundPrefetch": [
         {
             "platforms": [