[Android] Omnibox Placeholder Experiment

chrome/android/java/src/org/chromium/chrome/browser/omnibox/UrlBarFieldTrial.java

Index: chrome/android/java/src/org/chromium/chrome/browser/omnibox/UrlBarFieldTrial.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/omnibox/UrlBarFieldTrial.java b/chrome/android/java/src/org/chromium/chrome/browser/omnibox/UrlBarFieldTrial.java
new file mode 100644
index 0000000000000000000000000000000000000000..f0aed4c514737a5d2f420514eaea6f65fe41ccd9
--- /dev/null
+++ b/chrome/android/java/src/org/chromium/chrome/browser/omnibox/UrlBarFieldTrial.java
@@ -0,0 +1,98 @@
+// 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.
+
+package org.chromium.chrome.browser.omnibox;
+
+import android.content.res.Resources;
+
+import org.chromium.base.CommandLine;
+import org.chromium.base.ContextUtils;
+import org.chromium.base.FieldTrialList;
+import org.chromium.chrome.R;
+import org.chromium.chrome.browser.ChromeSwitches;
+import org.chromium.chrome.browser.preferences.ChromePreferenceManager;
+
+/**
+ * Provides Field Trial support for Clank Omnibox Placeholder Experiment

[mdjones, 2017/05/26 16:14:34]

nit: punctuation.

[huayinz, 2017/05/26 17:21:36]

Done.

+ */
+

[mdjones, 2017/05/26 16:14:34]

nit: remove extra new-line.

[huayinz, 2017/05/26 17:21:36]

Done.

+public class UrlBarFieldTrial {
+    private static final String FIELD_TRIAL_NAME = "ClankOmniboxPlaceholderExperiment";
+    private static final String GROUP_SEARCH_OR_TYPE_URL = "SearchOrTypeUrl";
+    private static final String GROUP_SEARCH_OR_TYPE_WEBSITE_NAME = "SearchOrTypeWebsiteName";
+    private static final String GROUP_SEARCH_THE_WEB = "SearchTheWeb";
+    private static final String GROUP_ENTER_A_SEARCH_OR_WEBSITE = "EnterASearchOrWebsite";
+    private static final String GROUP_SEARCH_NEWS = "SearchNews";
+    private static final String GROUP_SEARCH_RECIPES = "SearchRecipes";
+    private static final String GROUP_SEARCH_WEATHER = "SearchWeather";
+    private static final String GROUP_BLANK = "Blank";
+
+    private static String sCachedHint = null;
+
+    // prevent initiate
+    private UrlBarFieldTrial() {}

[mdjones, 2017/05/26 16:14:34]

nit: /** Prevent initialization of this class. */

[huayinz, 2017/05/26 17:21:36]

Done.

+
+    /**
+     * Caches hint text shown in omnibox url bar to shared preferences.
+     */
+    public static void cacheUrlBarHint() {

[mdjones, 2017/05/26 16:14:34]

We should name everything consistently; UrlBar -> Omnibox, Placeholder -> Hint,
etc.

[huayinz, 2017/05/26 17:21:36]

Done.

+        String groupName = FieldTrialList.findFullName(FIELD_TRIAL_NAME);
+        String hint;
+
+        CommandLine instance = CommandLine.getInstance();

[mdjones, 2017/05/26 16:14:34]

It might be worth checking CommandLine.isInitialized() before attempting to
access this (since flags are native).

[huayinz, 2017/05/26 17:21:36]

Done.

+        if (instance.hasSwitch(ChromeSwitches.OMNIBOX_PLACEHOLDER_EXPERIMENT_SWITCH)) {
+            groupName =
+                    instance.getSwitchValue(ChromeSwitches.OMNIBOX_PLACEHOLDER_EXPERIMENT_SWITCH);
+        }
+
+        Resources resources = ContextUtils.getApplicationContext().getResources();
+        switch (groupName) {
+            case GROUP_SEARCH_OR_TYPE_URL:
+                hint = resources.getString(R.string.search_or_type_url);
+                break;
+            case GROUP_SEARCH_OR_TYPE_WEBSITE_NAME:
+                hint = resources.getString(R.string.search_or_type_website_name);
+                break;
+            case GROUP_SEARCH_THE_WEB:
+                hint = resources.getString(R.string.search_the_web);
+                break;
+            case GROUP_ENTER_A_SEARCH_OR_WEBSITE:
+                hint = resources.getString(R.string.enter_a_search_or_website);
+                break;
+            case GROUP_SEARCH_NEWS:
+                hint = resources.getString(R.string.search_news);
+                break;
+            case GROUP_SEARCH_RECIPES:
+                hint = resources.getString(R.string.search_recipes);
+                break;
+            case GROUP_SEARCH_WEATHER:
+                hint = resources.getString(R.string.search_weather);
+                break;
+            case GROUP_BLANK:
+                hint = "";
+                break;
+            default:
+                hint = resources.getString(R.string.search_or_type_url);
+        }
+        ChromePreferenceManager.getInstance().setUrlBarHint(hint);
+    }
+
+    private static void setUrlBarHint() {

[mdjones, 2017/05/26 16:14:34]

nit: javadoc

[mdjones, 2017/05/26 16:14:34]

Based on the name of this function, it's unclear to me if the experiment flag is
cached or the hint text itself is.

[huayinz, 2017/05/26 17:21:36]

The hint text itself is cached in both shared preference and the static variable
in this class.

[huayinz, 2017/05/26 17:21:36]

Done.

[mdjones, 2017/05/26 22:08:51]

In most cases it's best practice to cache the flag value rather than the thing
it is enabling. For example, imagine the language changes, you would have the
incorrect text cached. It also helps if you need to make more than one decision
based on the flag. You can store the current text in a static variable in this
class, but the shared preferences should definitely contain the flag value.

[huayinz, 2017/05/30 17:16:47]

Done.

+        // setUrlBarHint is only called once when hint is not cached to static variable.
+        // This is to keep consistency on showing same hint to user in one session.
+        sCachedHint = ChromePreferenceManager.getInstance().getUrlBarHint();
+        if (sCachedHint == null) {
+            Resources resources = ContextUtils.getApplicationContext().getResources();
+            sCachedHint = resources.getString(R.string.search_or_type_url);
+        }
+    }
+
+    /**
+     * @return String of hint text to show in omnibox url bar.
+     */
+    public static String getUrlBarHint() {
+        if (sCachedHint == null) setUrlBarHint();
+        return sCachedHint;
+    }
+}