Fix cmd line args override in common.py

tools/chrome_proxy/webdriver/common.py

Index: tools/chrome_proxy/webdriver/common.py
diff --git a/tools/chrome_proxy/webdriver/common.py b/tools/chrome_proxy/webdriver/common.py
index 794918fcddcedfbfa3a24e996a32d86efd568b8e..0690de24e640348de6e1101e2001177c38e06717 100644
--- a/tools/chrome_proxy/webdriver/common.py
+++ b/tools/chrome_proxy/webdriver/common.py
@@ -77,11 +77,24 @@ class TestDriver:
 
   def _OverrideChromeArgs(self):
     """
-    Overrides any given flags in the code with those given on the command line.
+    Overrides any given arguments in the code with those given on the command

[RyanSturm, 2016/11/30 20:46:36]

I'm a little confused about how this is used. Can you add a better CL
description?

The way I understand this:
Tests will be written with various flags using AddChromeArg within the code, but
if the test is run with a command line override, the value of the flag will that
is used is the one from the command line.

Let me know if that's wrong.

Assuming that is correct, it might be a little more clear to parse
self._flags.browser_args into a seperate member dict during init, and in the
Add/Remove calls only add the new arg if it's key is not in the dict.

The reason I suggest this, is I don't understand when _OverrdieChromeArgs would
be called and when AddChromeArgs would be called.

[Robert Ogden, 2016/12/01 01:23:04]

CL description improved, but yes you were correct.

Do you mean turn _chrome_args into a dict? Filling that dict on init and then
not letting calls to AddChromeArg would prevent client code from modifying
existing flags, so flags couldn't be removed for example. Maybe that's ok, but
it gives some weird behavior that I don't think would be expected.
If not, then as it is _OverrideChromeArgs is called on _StartDriver so anything
the client code did gets wiped away at the very last second before Chrome
starts.

[RyanSturm, 2016/12/01 01:32:11]

SGTM.
That sounds fine then.

+    line. Arguments that need to be overridden may contain different values for
+    a flag given in the code. In that case, check by the flag whether to
+    override the argument.
     """
+    def GetDictKey(argument):
+      return argument.split('=', 1)[0]
     if self._flags.browser_args and len(self._flags.browser_args) > 0:
-      for arg in shlex.split(self._flags.browser_args):
-        self._chrome_args.add(arg)
+      # Build a dict of flags mapped to the whole argument.
+      originalArgs = {}

[RyanSturm, 2016/11/30 20:46:36]

s/originalArgs/original_args/

AFAIK naming for python in chromium uses
https://google.github.io/styleguide/pyguide.html?showone=Naming#Naming

except for a couple cases (specifically your method names are correct for
chromium with CamelCase):

https://chromium.googlesource.com/chromium/src/+/master/styleguide/styleguide.md

[Robert Ogden, 2016/12/01 01:23:04]

Done.

+      for arg in self._chrome_args:
+          originalArgs[GetDictKey(arg)] = arg
+      # Override by flag.
+      for overrideArg in shlex.split(self._flags.browser_args):

[RyanSturm, 2016/11/30 20:46:36]

s/overrideArg/override_arg/

[Robert Ogden, 2016/12/01 01:23:04]

Done.

+        argKey = GetDictKey(overrideArg)

[RyanSturm, 2016/11/30 20:46:36]

s/argKey/arg_key/

[Robert Ogden, 2016/12/01 01:23:04]

Done.

+        if argKey in originalArgs:
+          self._chrome_args.remove(originalArgs[argKey])
+        self._chrome_args.add(overrideArg)
 
   def _StartDriver(self):
     """