Move CastCommandLineHelper.java from internal to upstream and let...

Move CastCommandLineHelper.java from internal to upstream and let CastBrowserHelper use it to initialize command line.

Bug: b/110021629

Test: cast_shell_junit_tests

Change-Id: I9940e95e26be072bc1762e60f6c6ae541277d34c
Reviewed-on: https://chromium-review.googlesource.com/1097548Reviewed-by: Simeon Anfinrud <sanfin@chromium.org>
Commit-Queue: Zhiheng(Vincent) Li <vincentli@google.com>
Cr-Commit-Position: refs/heads/master@{#567041}

chromecast/browser/android/BUILD.gn

@@ -101,6 +101,7 @@ android_library("cast_shell_java") {
     "$java_src_dir/org/chromium/chromecast/shell/CastApplication.java",
     "$java_src_dir/org/chromium/chromecast/shell/CastBrowserHelper.java",
     "$java_src_dir/org/chromium/chromecast/shell/CastContentWindowAndroid.java",
+    "$java_src_dir/org/chromium/chromecast/shell/CastCommandLineHelper.java",
     "$java_src_dir/org/chromium/chromecast/shell/CastCrashHandler.java",
     "$java_src_dir/org/chromium/chromecast/shell/CastCrashUploader.java",
     "$java_src_dir/org/chromium/chromecast/shell/CastCrashUploaderFactory.java",
@@ -163,6 +164,7 @@ junit_binary("cast_shell_junit_tests") {
   java_files = [
     "junit/src/org/chromium/chromecast/shell/AsyncTaskRunnerTest.java",
     "junit/src/org/chromium/chromecast/shell/CastAudioManagerTest.java",
+    "junit/src/org/chromium/chromecast/shell/CastCommandLineHelperTest.java",
     "junit/src/org/chromium/chromecast/shell/CastWebContentsActivityTest.java",
     "junit/src/org/chromium/chromecast/shell/CastWebContentsComponentTest.java",
     "junit/src/org/chromium/chromecast/shell/CastWebContentsIntentUtilsTest.java",

chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastBrowserHelper.java

@@ -4,11 +4,8 @@
 
 package org.chromium.chromecast.shell;
 
-import android.app.Activity;
 import android.content.Context;
-import android.content.Intent;
 
-import org.chromium.base.CommandLine;
 import org.chromium.base.Log;
 import org.chromium.base.library_loader.LibraryLoader;
 import org.chromium.base.library_loader.LibraryProcessType;
@@ -24,8 +21,6 @@ import org.chromium.net.NetworkChangeNotifier;
 public class CastBrowserHelper {
     private static final String TAG = "CastBrowserHelper";
 
-    public static final String COMMAND_LINE_ARGS_KEY = "commandLineArgs";
-
     private static boolean sIsBrowserInitialized = false;
 
     /**
@@ -43,17 +38,8 @@ public class CastBrowserHelper {
         ChromecastConfigAndroid.initializeForBrowser(context);
 
         // Initializing the command line must occur before loading the library.
-        if (!CommandLine.isInitialized()) {
-            ((CastApplication) context.getApplicationContext()).initCommandLine();
-
-            if (context instanceof Activity) {
-                Intent launchingIntent = ((Activity) context).getIntent();
-                String[] commandLineParams = getCommandLineParamsFromIntent(launchingIntent);
-                if (commandLineParams != null) {
-                    CommandLine.getInstance().appendSwitchesAndArguments(commandLineParams);
-                }
-            }
-        }
+        CastCommandLineHelper.initCommandLineWithSavedArgs(
+                () -> { ((CastApplication) context.getApplicationContext()).initCommandLine(); });
 
         DeviceUtils.addDeviceSpecificUserAgentSwitch(context);
 
@@ -73,8 +59,4 @@ public class CastBrowserHelper {
             return false;
         }
     }
-
-    private static String[] getCommandLineParamsFromIntent(Intent intent) {
-        return intent != null ? intent.getStringArrayExtra(COMMAND_LINE_ARGS_KEY) : null;
-    }
 }

chromecast/browser/android/apk/src/org/chromium/chromecast/shell/CastCommandLineHelper.java

+// 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.chromecast.shell;
+
+import android.content.Intent;
+import android.content.SharedPreferences;
+
+import org.json.JSONException;
+import org.json.JSONObject;
+
+import org.chromium.base.CommandLine;
+import org.chromium.base.ContextUtils;
+import org.chromium.base.Log;
+import org.chromium.base.VisibleForTesting;
+
+import java.util.Iterator;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+/**
+ * Helper class for handling command line flags passed to the Cast Service through Intent extras.
+ */
+public class CastCommandLineHelper {
+    private static final String TAG = "CastCmdLineHelper";
+    private static final String COMMAND_LINE_FLAGS_PREF = "cast_command_line_args";
+
+    private static final AtomicBoolean sCommandLineInitialized = new AtomicBoolean(false);
+
+    private CastCommandLineHelper() {}
+
+    /**
+     * Parses whitelisted command line args from the provided Intent's extras and saves them to
+     * persistent storage. No-op if the provided Intent is null.
+     */
+    public static void saveCommandLineArgsFromIntent(
+            Intent intent, Iterable<String> commandLineArgs) {
+        if (intent == null) return;
+
+        // Store the command line args in a JSON object to safely preserve the separation between
+        // switch name and value while fitting SharedPreferences' restricted set of supported types.
+        JSONObject cmdLineArgs = new JSONObject();
+        for (String swtch : commandLineArgs) {
+            // getStringExtra() returning null doesn't necessarily indicate the extra doesn't exist.
+            // It could be a null String extra, e.g. inserted with "--esn" from the command line or
+            // putExtra(name, (String) null)). Null values map to a switch with no value.
+            if (!intent.hasExtra(swtch)) continue;
+
+            String value = intent.getStringExtra(swtch);
+            try {
+                cmdLineArgs.put(swtch, (value == null) ? JSONObject.NULL : value);
+            } catch (JSONException e) {
+                Log.e(TAG, "failed to add arg to JSON object: %s=%s", swtch, value);
+                continue;
+            }
+        }
+
+        // If no matching extras were found, we still overwrite the preference to ensure that old
+        // args aren't applied.
+        SharedPreferences prefs = ContextUtils.getAppSharedPreferences();
+        prefs.edit().putString(COMMAND_LINE_FLAGS_PREF, cmdLineArgs.toString()).apply();
+    }
+
+    /**
+     * Reads command line args from persistent storage and initializes the CommandLine with those
+     * args. Does not initialize CommandLine if no args were saved in storage, or if it has been
+     * already been done.
+     */
+    public static void initCommandLineWithSavedArgs(CommandLineInitializer commandLineInitializer) {
+        // CommandLine is a singleton, so check whether CastCommandLineHelper has initialized it
+        // already and do nothing if so. We keep track of this in a static variable so we can still
+        // assert that something else doesn't generally initialize the CommandLine before us.
+        if (!sCommandLineInitialized.compareAndSet(false, true)) {
+            Log.i(TAG, "command line already initialized by us, skipping");
+            return;
+        }
+        assert !CommandLine.isInitialized();
+
+        final SharedPreferences prefs = ContextUtils.getAppSharedPreferences();
+        final String argsJson = prefs.getString(COMMAND_LINE_FLAGS_PREF, null);
+        if (argsJson == null) {
+            return;
+        }
+
+        final JSONObject args;
+        try {
+            args = new JSONObject(argsJson);
+        } catch (JSONException e) {
+            Log.e(TAG, "failed to parse cmd line args stored in shared prefs: %s", e);
+            return;
+        }
+        if (args.length() == 0) {
+            return;
+        }
+
+        // Let the injected delegate do the standard command line initialization.
+        commandLineInitializer.initCommandLine();
+
+        // If SharedPreferences contains any command line args previously saved from an Intent,
+        // apply them as defaults, i.e. if a value isn't already present in the CommandLine
+        // singleton. Values may already be present if loaded by the Application.
+        final CommandLine cmdline = CommandLine.getInstance();
+        final Iterator<String> switches = args.keys();
+        while (switches.hasNext()) {
+            String swtch = switches.next();
+            String value;
+            try {
+                value = args.isNull(swtch) ? null : args.getString(swtch);
+            } catch (JSONException e) {
+                Log.e(TAG, "failed to get string value for switch '%s'", swtch);
+                continue;
+            }
+
+            if (!cmdline.hasSwitch(swtch)) {
+                Log.d(TAG, "appending command line arg: %s=%s", swtch, value);
+                cmdline.appendSwitchWithValue(swtch, value);
+            } else {
+                Log.w(TAG, "skipped command line arg from intent, value already present: %s=%s",
+                        swtch, value);
+            }
+        }
+    }
+
+    @VisibleForTesting
+    static void resetSavedArgsForTest() {
+        SharedPreferences prefs = ContextUtils.getAppSharedPreferences();
+        prefs.edit().remove(COMMAND_LINE_FLAGS_PREF).apply();
+    }
+
+    @VisibleForTesting
+    static void resetCommandLineForTest() {
+        CommandLine.reset();
+        sCommandLineInitialized.set(false);
+    }
+
+    /**
+     * Delegate interface for initCommandLineWithSavedArgs().
+     *
+     * The CommandLineInitializer is responsible for initializing the CommandLine singleton.
+     * CommandLine.isInitialized() must be true after this interface's method is called.
+     *
+     * TODO(sanfin): This is a workaround for an upstream refactor at go/chromium-cl/794031.
+     * Consider refactoring CastCommandLineHelper and other scattered CommandLine logic to depend
+     * less on singletons and other code smells.
+     */
+    public interface CommandLineInitializer { public void initCommandLine(); }
+}

chromecast/browser/android/junit/src/org/chromium/chromecast/shell/CastCommandLineHelperTest.java

+// 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.chromecast.shell;
+
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.nullValue;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import android.app.Application;
+import android.content.Context;
+import android.content.Intent;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ErrorCollector;
+import org.junit.runner.RunWith;
+import org.robolectric.RuntimeEnvironment;
+import org.robolectric.annotation.Config;
+
+import org.chromium.base.CommandLine;
+import org.chromium.base.ContextUtils;
+import org.chromium.testing.local.LocalRobolectricTestRunner;
+
+import java.util.Arrays;
+import java.util.List;
+
+/** Unit tests for {@link CastCommandLineHelper}. */
+@RunWith(LocalRobolectricTestRunner.class)
+@Config(manifest = Config.NONE, application = CastCommandLineHelperTest.FakeApplication.class)
+public class CastCommandLineHelperTest {
+    private static final String NON_WHITELISTED_ARG = "non-whitelisted-arg";
+    private static final String WHITELISTED_ARG = "audio-input-disable-eraser";
+    private static final String WHITELISTED_ARG_WITH_VALUE = "setup-ssid-suffix";
+    private static final String WHITELISTED_ARG_WITH_VALUE2 = "default-eureka-name-prefix";
+    private static final List<String> COMMAND_LINE_ARG_WHITELIST =
+            Arrays.asList(WHITELISTED_ARG, WHITELISTED_ARG_WITH_VALUE, WHITELISTED_ARG_WITH_VALUE2);
+
+    // CastApplication inherits from ContentApplication, which starts a bunch of unnecessary stuff,
+    // and it also calls initApplicationContext() which causes problems because the Application is
+    // recreated for every test. Use a minimal fake instead.
+    public static class FakeApplication extends Application {
+        @Override
+        protected void attachBaseContext(Context base) {
+            super.attachBaseContext(base);
+            ContextUtils.initApplicationContextForTests(this);
+        }
+    }
+
+    private static Context getApplicationContext() {
+        return RuntimeEnvironment.application;
+    }
+
+    private static Intent getTestIntentWithArgs() {
+        Intent intent = new Intent();
+        intent.putExtra(NON_WHITELISTED_ARG, "value");
+        intent.putExtra(WHITELISTED_ARG, (String) null);
+        intent.putExtra(WHITELISTED_ARG_WITH_VALUE, "a");
+        return intent;
+    }
+
+    // Verifies that the provided CommandLine has the expected switches to match the Intent returned
+    // from getTestIntentWithArgs().
+    private void verifyTestIntent(CommandLine cmdLine) {
+        collector.checkThat(cmdLine.hasSwitch(NON_WHITELISTED_ARG), is(false));
+        collector.checkThat(cmdLine.hasSwitch(WHITELISTED_ARG), is(true));
+        collector.checkThat(cmdLine.getSwitchValue(WHITELISTED_ARG), is(nullValue()));
+        collector.checkThat(cmdLine.hasSwitch(WHITELISTED_ARG_WITH_VALUE), is(true));
+        collector.checkThat(cmdLine.getSwitchValue(WHITELISTED_ARG_WITH_VALUE), is("a"));
+    }
+
+    private void initCommandLineWithSavedArgs(String[] initialArgs) {
+        CastCommandLineHelper.initCommandLineWithSavedArgs(() -> CommandLine.init(initialArgs));
+    }
+
+    @Rule
+    public ErrorCollector collector = new ErrorCollector();
+
+    @Before
+    public void setUp() {
+        // Reset the CommandLine to uninitialized & delete any saved args before starting each test.
+        CastCommandLineHelper.resetCommandLineForTest();
+        CastCommandLineHelper.resetSavedArgsForTest();
+    }
+
+    @Test
+    public void testNullIntent() {
+        CastCommandLineHelper.saveCommandLineArgsFromIntent(null, COMMAND_LINE_ARG_WHITELIST);
+        initCommandLineWithSavedArgs(null);
+        assertFalse(CommandLine.isInitialized());
+    }
+
+    @Test
+    public void testEmptyIntent() {
+        CastCommandLineHelper.saveCommandLineArgsFromIntent(
+                new Intent(), COMMAND_LINE_ARG_WHITELIST);
+        initCommandLineWithSavedArgs(null);
+        assertFalse(CommandLine.isInitialized());
+    }
+
+    @Test
+    public void testIntentWIthOnlyNonWhitelistedExtra() {
+        Intent intent = new Intent();
+        intent.putExtra(NON_WHITELISTED_ARG, "value");
+
+        CastCommandLineHelper.saveCommandLineArgsFromIntent(intent, COMMAND_LINE_ARG_WHITELIST);
+        initCommandLineWithSavedArgs(null);
+        assertFalse(CommandLine.isInitialized());
+    }
+
+    @Test
+    public void testIntentWithArgs() {
+        CastCommandLineHelper.saveCommandLineArgsFromIntent(
+                getTestIntentWithArgs(), COMMAND_LINE_ARG_WHITELIST);
+        initCommandLineWithSavedArgs(null);
+        assertTrue(CommandLine.isInitialized());
+        verifyTestIntent(CommandLine.getInstance());
+    }
+
+    @Test
+    public void testArgsArePersisted() {
+        CastCommandLineHelper.saveCommandLineArgsFromIntent(
+                getTestIntentWithArgs(), COMMAND_LINE_ARG_WHITELIST);
+        initCommandLineWithSavedArgs(null);
+        assertTrue(CommandLine.isInitialized());
+        verifyTestIntent(CommandLine.getInstance());
+
+        // Mimic the process being killed, such that the CommandLine singleton is reset.
+        CastCommandLineHelper.resetCommandLineForTest();
+        assertFalse(CommandLine.isInitialized());
+
+        // CommandLine should be initialized with the same args as above, even if a null Intent is
+        // passed in (which mimics the fact that onStartCommand receives a null Intent when a
+        // START_STICKY Service is restarted).
+        CastCommandLineHelper.saveCommandLineArgsFromIntent(null, COMMAND_LINE_ARG_WHITELIST);
+        initCommandLineWithSavedArgs(null);
+        assertTrue(CommandLine.isInitialized());
+        verifyTestIntent(CommandLine.getInstance());
+    }
+
+    @Test
+    public void testAlreadyInitialized() {
+        CastCommandLineHelper.saveCommandLineArgsFromIntent(
+                getTestIntentWithArgs(), COMMAND_LINE_ARG_WHITELIST);
+        initCommandLineWithSavedArgs(null);
+        assertTrue(CommandLine.isInitialized());
+        verifyTestIntent(CommandLine.getInstance());
+
+        // If Service is killed w/o process being killed, then the CommandLine singleton will still
+        // be initialized. Make sure it's safe for the Service to use CastCommandLineHelper again
+        // when it starts.
+        CastCommandLineHelper.saveCommandLineArgsFromIntent(
+                getTestIntentWithArgs(), COMMAND_LINE_ARG_WHITELIST);
+        initCommandLineWithSavedArgs(null);
+        assertTrue(CommandLine.isInitialized());
+        verifyTestIntent(CommandLine.getInstance());
+    }
+
+    // Test that arguments passed in through the intent don't override values already in the command
+    // line, i.e. loaded from the castshell-command-line file, but that new arguments do get added.
+    @Test
+    public void testDontOverrideExistingValues() {
+        final String[] initialArgs = new String[] {"dummyapp", "--" + WHITELISTED_ARG,
+                "--" + WHITELISTED_ARG_WITH_VALUE + "=initValue"};
+
+        Intent intent = new Intent();
+        intent.putExtra(WHITELISTED_ARG, "intentValue");
+        intent.putExtra(WHITELISTED_ARG_WITH_VALUE, "intentValue");
+        intent.putExtra(WHITELISTED_ARG_WITH_VALUE2, "intentValue");
+
+        CastCommandLineHelper.saveCommandLineArgsFromIntent(intent, COMMAND_LINE_ARG_WHITELIST);
+        initCommandLineWithSavedArgs(initialArgs);
+
+        assertTrue(CommandLine.isInitialized());
+        final CommandLine cmdLine = CommandLine.getInstance();
+        collector.checkThat(cmdLine.hasSwitch(WHITELISTED_ARG), is(true));
+        collector.checkThat(cmdLine.getSwitchValue(WHITELISTED_ARG), is(nullValue()));
+        collector.checkThat(cmdLine.hasSwitch(WHITELISTED_ARG_WITH_VALUE), is(true));
+        collector.checkThat(cmdLine.getSwitchValue(WHITELISTED_ARG_WITH_VALUE), is("initValue"));
+        collector.checkThat(cmdLine.hasSwitch(WHITELISTED_ARG_WITH_VALUE2), is(true));
+        collector.checkThat(cmdLine.getSwitchValue(WHITELISTED_ARG_WITH_VALUE2), is("intentValue"));
+    }
+}