Migrate *-ime-thread switches to feature API

The new Feature API is the simplest way to run Finch experiments with. Also, it gets propagated to the renderer. BUG=551193, 591149 Review URL: https://codereview.chromium.org/1771223002 Cr-Commit-Position: refs/heads/master@{#380256}
huningxin · Mar 9, 2016 · 7ded375 · 7ded375
1 parent 3cbcd14
commit 7ded375
Show file tree

Hide file tree

Showing 13 changed files with 35 additions and 36 deletions.
diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc
@@ -1806,8 +1806,7 @@ const FeatureEntry kFeatureEntries[] = {
 #if defined(OS_ANDROID)
     {"ime-thread", IDS_FLAGS_IME_THREAD_NAME,
      IDS_FLAGS_IME_THREAD_DESCRIPTION, kOsAndroid,
-     ENABLE_DISABLE_VALUE_TYPE(switches::kEnableImeThread,
-                               switches::kDisableImeThread)},
+     FEATURE_VALUE_TYPE(features::kImeThread)},
 #endif  // defined(OS_ANDROID)
 #if defined(OS_ANDROID)
     {"offline-pages-ntp", IDS_FLAGS_NTP_OFFLINE_PAGES_NAME,

diff --git a/content/browser/renderer_host/ime_adapter_android.cc b/content/browser/renderer_host/ime_adapter_android.cc
@@ -11,6 +11,7 @@
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
 #include "base/android/scoped_java_ref.h"
+#include "base/feature_list.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "content/browser/frame_host/frame_tree.h"
@@ -26,6 +27,7 @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/native_web_keyboard_event.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/common/content_features.h"
 #include "jni/ImeAdapter_jni.h"
 #include "third_party/WebKit/public/web/WebCompositionUnderline.h"
 #include "third_party/WebKit/public/web/WebInputEvent.h"
@@ -300,6 +302,12 @@ bool ImeAdapterAndroid::RequestTextInputStateUpdate(
   return true;
 }
 
+bool ImeAdapterAndroid::IsImeThreadEnabled(
+    JNIEnv* env,
+    const base::android::JavaParamRef<jobject>&) {
+  return base::FeatureList::IsEnabled(features::kImeThread);
+}
+
 void ImeAdapterAndroid::ResetImeAdapter(JNIEnv* env,
                                         const JavaParamRef<jobject>&) {
   java_ime_adapter_.reset();

diff --git a/content/browser/renderer_host/ime_adapter_android.h b/content/browser/renderer_host/ime_adapter_android.h
@@ -77,6 +77,7 @@ class ImeAdapterAndroid {
   void ResetImeAdapter(JNIEnv*, const base::android::JavaParamRef<jobject>&);
   bool RequestTextInputStateUpdate(JNIEnv*,
                                    const base::android::JavaParamRef<jobject>&);
+  bool IsImeThreadEnabled(JNIEnv*, const base::android::JavaParamRef<jobject>&);
 
   // Called from native -> java
   void CancelComposition();

diff --git a/content/browser/renderer_host/render_process_host_impl.cc b/content/browser/renderer_host/render_process_host_impl.cc
@@ -1568,8 +1568,6 @@ void RenderProcessHostImpl::PropagateBrowserCommandLineToRenderer(
     switches::kEnableUnifiedMediaPipeline,
     switches::kIPCSyncCompositing,
     switches::kRendererWaitForJavaDebugger,
-    switches::kEnableImeThread,
-    switches::kDisableImeThread,
 #endif
 #if defined(OS_MACOSX)
     // Allow this to be set when invoking the browser and relayed along.

diff --git a/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java b/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
@@ -18,14 +18,12 @@
 import android.view.inputmethod.BaseInputConnection;
 import android.view.inputmethod.EditorInfo;
 
-import org.chromium.base.CommandLine;
 import org.chromium.base.Log;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.blink_public.web.WebInputEventModifier;
 import org.chromium.blink_public.web.WebInputEventType;
-import org.chromium.content.common.ContentSwitches;
 import org.chromium.ui.base.ime.TextInputType;
 import org.chromium.ui.picker.InputDialogContainer;
 
@@ -120,25 +118,20 @@ public ImeAdapter(InputMethodManagerWrapper wrapper, ImeAdapterDelegate embedder
                 mViewEmbedder.getAttachedView().getResources().getConfiguration());
     }
 
-    void resetInputConnectionFactory() {
-        if (shouldUseImeThread()) {
+    private boolean isImeThreadEnabled() {
+        if (mNativeImeAdapterAndroid == 0) return false;
+        return nativeIsImeThreadEnabled(mNativeImeAdapterAndroid);
+    }
+
+    private void resetInputConnectionFactory() {
+        if (isImeThreadEnabled()) {
             mInputConnectionFactory =
                     new ThreadedInputConnectionFactory(mInputMethodManagerWrapper);
         } else {
             mInputConnectionFactory = new ReplicaInputConnection.Factory();
         }
     }
 
-    private boolean shouldUseImeThread() {
-        if (CommandLine.getInstance().hasSwitch(ContentSwitches.DISABLE_IME_THREAD)) {
-            return false;
-        }
-        if (CommandLine.getInstance().hasSwitch(ContentSwitches.ENABLE_IME_THREAD)) {
-            return true;
-        }
-        return false;
-    }
-
     /**
      * @see View#onCreateInputConnection(EditorInfo)
      */
@@ -625,4 +618,5 @@ private native void nativeDeleteSurroundingText(long nativeImeAdapterAndroid,
             int before, int after);
     private native void nativeResetImeAdapter(long nativeImeAdapterAndroid);
     private native boolean nativeRequestTextInputStateUpdate(long nativeImeAdapterAndroid);
+    private native boolean nativeIsImeThreadEnabled(long nativeImeAdapterAndroid);
 }
diff --git a/content/public/android/java/src/org/chromium/content/common/ContentSwitches.java b/content/public/android/java/src/org/chromium/content/common/ContentSwitches.java
@@ -87,10 +87,6 @@ public abstract class ContentSwitches {
     // Native switch kDownloadProcess
     public static final String SWITCH_DOWNLOAD_PROCESS = "download";
 
-    // Native switches to enable / disable IME's own thread instead of using main UI thread.
-    public static final String ENABLE_IME_THREAD = "enable-ime-thread";
-    public static final String DISABLE_IME_THREAD = "disable-ime-thread";
-
     // Prevent instantiation.
     private ContentSwitches() {}
 

diff --git a/content/public/common/content_features.cc b/content/public/common/content_features.cc
@@ -70,6 +70,11 @@ const base::Feature kWebFontsIntervention{"WebFontsIntervention",
                                           base::FEATURE_DISABLED_BY_DEFAULT};
 
 #if defined(OS_ANDROID)
+// Use IME's own thread instead of using main UI thread. It also means that
+// we will not use replica editor and do a round trip to renderer to synchronize
+// with Blink data.
+const base::Feature kImeThread{"ImeThread", base::FEATURE_DISABLED_BY_DEFAULT};
+
 // FeatureList definition for the Seccomp field trial.
 const base::Feature kSeccompSandboxAndroid{"SeccompSandboxAndroid",
                                            base::FEATURE_DISABLED_BY_DEFAULT};

diff --git a/content/public/common/content_features.h b/content/public/common/content_features.h
@@ -30,6 +30,7 @@ CONTENT_EXPORT extern const base::Feature kUpdateRendererPriorityOnStartup;
 CONTENT_EXPORT extern const base::Feature kWebFontsIntervention;
 
 #if defined(OS_ANDROID)
+CONTENT_EXPORT extern const base::Feature kImeThread;
 CONTENT_EXPORT extern const base::Feature kSeccompSandboxAndroid;
 #endif  // defined(OS_ANDROID)
 

diff --git a/content/public/common/content_switches.cc b/content/public/common/content_switches.cc
@@ -950,12 +950,6 @@ const char kRemoteDebuggingSocketName[]     = "remote-debugging-socket-name";
 // Block ChildProcessMain thread of the renderer's ChildProcessService until a
 // Java debugger is attached.
 const char kRendererWaitForJavaDebugger[] = "renderer-wait-for-java-debugger";
-
-// Use IME's own thread instead of using main UI thread. It also means that
-// we will not use replica editor and do a round trip to renderer to synchronize
-// with Blink data.
-const char kEnableImeThread[] = "enable-ime-thread";
-const char kDisableImeThread[] = "disable-ime-thread";
 #endif
 
 // Enable the aggressive flushing of DOM Storage to minimize data loss.

diff --git a/content/public/common/content_switches.h b/content/public/common/content_switches.h
@@ -274,8 +274,6 @@ CONTENT_EXPORT extern const char kHideScrollbars[];
 extern const char kNetworkCountryIso[];
 CONTENT_EXPORT extern const char kRemoteDebuggingSocketName[];
 CONTENT_EXPORT extern const char kRendererWaitForJavaDebugger[];
-CONTENT_EXPORT extern const char kEnableImeThread[];
-CONTENT_EXPORT extern const char kDisableImeThread[];
 #endif
 
 #if defined(OS_CHROMEOS)

diff --git a/content/renderer/render_widget.cc b/content/renderer/render_widget.cc
@@ -9,6 +9,7 @@
 #include "base/auto_reset.h"
 #include "base/bind.h"
 #include "base/command_line.h"
+#include "base/feature_list.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
@@ -38,6 +39,7 @@
 #include "content/common/input_messages.h"
 #include "content/common/swapped_out_messages.h"
 #include "content/common/view_messages.h"
+#include "content/public/common/content_features.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/context_menu_params.h"
 #include "content/renderer/cursor_utils.h"
@@ -1759,10 +1761,7 @@ void RenderWidget::set_next_paint_is_repaint_ack() {
 
 bool RenderWidget::IsUsingImeThread() {
 #if defined(OS_ANDROID)
-  return base::CommandLine::ForCurrentProcess()->HasSwitch(
-      switches::kEnableImeThread) &&
-      !base::CommandLine::ForCurrentProcess()->HasSwitch(
-      switches::kDisableImeThread);
+  return base::FeatureList::IsEnabled(features::kImeThread);
 #else
   return false;
 #endif

diff --git a/testing/variations/fieldtrial_testing_config_android.json b/testing/variations/fieldtrial_testing_config_android.json
@@ -123,6 +123,14 @@
             "group_name": "Enabled"
         }
     ],
+    "ImeThread": [
+        {
+            "enable_features": [
+                "ImeThread"
+            ],
+            "group_name": "Enabled"
+        }
+    ],
     "InvalidationsGCMUpstream": [
         {
             "group_name": "Enabled"

diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
@@ -72282,7 +72282,6 @@ To add a new entry, add it with any value and run test to compute valid value.
   <int value="-1930720286" label="nacl-debug-mask"/>
   <int value="-1928198763" label="enable-async-dns"/>
   <int value="-1925117279" label="disable-quic-https"/>
-  <int value="-1919308574" label="enable-ime-thread"/>
   <int value="-1915854488" label="enable-offline-pages"/>
   <int value="-1911153473" label="enable-easy-signin"/>
   <int value="-1892555086" label="disable-compositor-animation-timelines"/>
@@ -72780,7 +72779,6 @@ To add a new entry, add it with any value and run test to compute valid value.
   <int value="1820451991" label="enable-offline-auto-reload"/>
   <int value="1821723343" label="disable-saml-signin"/>
   <int value="1828660283" label="enable-webfonts-intervention-trigger"/>
-  <int value="1840930186" label="disable-ime-thread"/>
   <int value="1844110073" label="enable-app-view"/>
   <int value="1847024354" label="enable-hotword-hardware"/>
   <int value="1855524566" label="allow-insecure-websocket-from-https-origin"/>