show_stdout to BrowserOptions

show_stdout to BrowserOptions

BUG=269131
TEST=manual

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=221603

[achuithb, 2013-09-04 01:26:34]

Dave, what do you think of this?

I can move the rest of the browser options over from here:
https://code.google.com/p/chromium/codesearch#chromium/src/tools/telemetry/te...

[achuithb, 2013-09-04 21:44:52]

On 2013/09/04 01:26:34, achuith.bhandarkar wrote:
> Dave, what do you think of this?
> 
> I can move the rest of the browser options over from here:
>
https://code.google.com/p/chromium/codesearch#chromium/src/tools/telemetry/te...

This is what the CL would look like that moved the rest of the Browser options:
https://codereview.chromium.org/23749004/

[dtu, 2013-09-05 20:00:36]

lgtm

[achuithb, 2013-09-05 20:29:32]

On 2013/09/05 20:00:36, Dave Tu wrote:
> lgtm

Thanks.

So this is my plan:

We can have browser_options be a part of finder_options for now. We replace all
*_backend invocations of finder_options with browser_options, so at least the
callsite invocations will be correct - they will be of the form
if self.browser_options.foo:
  do_something


Then it becomes easier to create a global object that scans the command line,
and have finder_options, browser_options, et al  subscribe to it somehow. We can
hopefully make changes to browser_options.py without having to modify the rest
of the world.

Wdyt?

[commit-bot: I haz the power, 2013-09-05 20:31:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/achuith@chromium.org/23536019/14001

[commit-bot: I haz the power, 2013-09-05 20:31:12]

Failed to apply patch for tools/telemetry/telemetry/core/browser_options.py:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file tools/telemetry/telemetry/core/browser_options.py
  Hunk #4 FAILED at 173.
  Hunk #5 FAILED at 216.
  Hunk #6 succeeded at 246 (offset 5 lines).
  2 out of 6 hunks FAILED -- saving rejects to file
tools/telemetry/telemetry/core/browser_options.py.rej

Patch:       tools/telemetry/telemetry/core/browser_options.py
Index: tools/telemetry/telemetry/core/browser_options.py
diff --git a/tools/telemetry/telemetry/core/browser_options.py
b/tools/telemetry/telemetry/core/browser_options.py
index
2b2608151d186b17cc8aff200d35cdef5fcc1e29..6841c806e9e38e19ad5d7dc417608b109aa8e963
100644
--- a/tools/telemetry/telemetry/core/browser_options.py
+++ b/tools/telemetry/telemetry/core/browser_options.py
@@ -35,7 +35,6 @@ class BrowserFinderOptions(optparse.Values):
     self.profile_type = None
     self.extra_browser_args = []
     self.extra_wpr_args = []
-    self.show_stdout = False
     self.extensions_to_load = []
     self.clear_sytem_cache_for_browser_and_profile_on_start = False
 
@@ -56,6 +55,7 @@ class BrowserFinderOptions(optparse.Values):
     self.no_proxy_server = False
 
     self.repeat_options = repeat_options.RepeatOptions()
+    self.browser_options = BrowserOptions()
     self.output_file = None
     self.skip_navigate_on_repeat = False
 
@@ -126,9 +126,6 @@ class BrowserFinderOptions(optparse.Values):
         dest='extra_wpr_args_as_string',
         help=('Additional arguments to pass to Web Page Replay. '
               'See third_party/webpagereplay/replay.py for usage.'))
-    group.add_option('--show-stdout',
-        action='store_true',
-        help='When possible, will display the stdout of the process')
     parser.add_option_group(group)
 
     # Page set options
@@ -176,9 +173,12 @@ class BrowserFinderOptions(optparse.Values):
         'This option prevents Telemetry from tweaking such platform settings.')
     parser.add_option_group(group)
 
-    # Repeat options
+    # Repeat options.
     repeat_options.RepeatOptions.AddCommandLineOptions(parser)
 
+    # Browser options.
+    self.browser_options.AddCommandLineOptions(parser)
+
     real_parse = parser.parse_args
     def ParseArgs(args=None):
       defaults = parser.get_default_values()
@@ -219,9 +219,12 @@ class BrowserFinderOptions(optparse.Values):
       if self.profile_type == 'default':
         self.dont_override_profile = True
 
-      # Parse repeat options
+      # Parse repeat options.
       self.repeat_options.UpdateFromParseResults(self, parser)
 
+      # Parse browser options.
+      self.browser_options.UpdateFromParseResults(self)
+
       if self.profile_dir and self.profile_type != 'clean':
         raise Exception("It's illegal to specify both --profile-type and"
             " --profile-dir.")
@@ -244,4 +247,20 @@ class BrowserFinderOptions(optparse.Values):
 class BrowserOptions():
   """Options to be used for launching a browser."""
   def __init__(self):
-    pass
+    self.show_stdout = False
+
+  def AddCommandLineOptions(self, parser):
+    group = optparse.OptionGroup(parser, 'Browser options')
+    group.add_option('--show-stdout',
+        action='store_true',
+        help='When possible, will display the stdout of the process')
+    parser.add_option_group(group)
+
+  def UpdateFromParseResults(self, finder_options):
+    """Copies our options from finder_options"""
+    browser_options_list = ['show_stdout']
+    for o in browser_options_list:
+      a = getattr(finder_options, o)
+      if a:
+        setattr(self, o, a)
+        delattr(finder_options, o)

[commit-bot: I haz the power, 2013-09-05 22:35:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/achuith@chromium.org/23536019/21001

[commit-bot: I haz the power, 2013-09-06 06:22:45]

Change committed as 221603