[Telemetry] Drop check for devtools support for record-as-much-as-possible (chrome >2118)

[Telemetry] Drop check for devtools support for record-as-much-as-possible (chrome >2118)

BUG=396081
Committed: https://crrev.com/1ca83b0ff277b7ec1d10ed3a14dcca823ce72249
Cr-Commit-Position: refs/heads/master@{#309872}

[slamm, 2014-12-22 18:19:08]

slamm@chromium.org changed reviewers:
+ nednguyen@google.com

[nednguyen, 2014-12-22 18:29:48]

https://codereview.chromium.org/808563005/diff/1/tools/telemetry/telemetry/co...
File tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py (right):

https://codereview.chromium.org/808563005/diff/1/tools/telemetry/telemetry/co...
tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py:23: def
__init__(self, devtools_port, chrome_browser_backend):
We should be able to drop chrome_browser_backend from the constructor.

[nednguyen, 2014-12-22 19:20:44]

nednguyen@google.com changed reviewers:
+ ariblue@google.com

[slamm, 2014-12-22 20:38:33]

Drop chrome_browser_backend from TracingBackend. The version check was the only
thing that needed it.

[nednguyen, 2014-12-22 21:48:29]

The CQ bit was checked by nednguyen@google.com

[nednguyen, 2014-12-22 21:48:30]

lgtm

[commit-bot: I haz the power, 2014-12-22 21:49:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/808563005/20001

[nednguyen, 2014-12-22 21:49:39]

The CQ bit was unchecked by nednguyen@google.com

[slamm, 2014-12-22 22:11:26]

Add back the mapping from telemetry tracing record_mode to DevTools string.
Update docstrings.

[slamm, 2014-12-22 22:15:17]

As per our conversation, I added back the dict to map the Telemetry record_mode
to the DevTools API string.

I fixed up a couple doc strings too.

https://codereview.chromium.org/808563005/diff/1/tools/telemetry/telemetry/co...
File tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py (right):

https://codereview.chromium.org/808563005/diff/1/tools/telemetry/telemetry/co...
tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py:23: def
__init__(self, devtools_port, chrome_browser_backend):
On 2014/12/22 18:29:48, nednguyen wrote:
> We should be able to drop chrome_browser_backend from the constructor.

Done. That is much better.

[nednguyen, 2014-12-22 22:19:50]

Feel free to land this after you address the nit.

https://codereview.chromium.org/808563005/diff/40001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py (right):

https://codereview.chromium.org/808563005/diff/40001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py:53:
}[trace_options.record_mode]
Nit: can you declare the map like the left?

[slamm, 2014-12-22 22:42:13]

Restore original map style

[slamm, 2014-12-22 22:44:25]

Restore original map style

[slamm, 2014-12-22 22:45:09]

Patchset #4 (id:60001) has been deleted

[slamm, 2014-12-22 22:45:37]

The CQ bit was checked by slamm@chromium.org

[commit-bot: I haz the power, 2014-12-22 22:46:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/808563005/80001

[chrishenry, 2014-12-22 23:26:35]

chrishenry@google.com changed reviewers:
+ chrishenry@google.com

[chrishenry, 2014-12-22 23:26:36]

Can you make sure that L webview is past this branch number or doesn't require
this for other reason? Thanks!

[commit-bot: I haz the power, 2014-12-22 23:47:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-22 23:47:39]

Failed to apply patch for
tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py:
While running git apply --index -3 -p1;
  error: patch failed:
tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py:30
  Falling back to three-way merge...
  Applied patch to
'tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py' with
conflicts.
  U tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py

Patch:       tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py
Index: tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py
diff --git a/tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py
b/tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py
index
62426419bdc07504e10264d4b74b915761da93a8..f9fb353b6bdc1c5d4e507a7f3e76aa9ea5019619
100644
--- a/tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py
+++ b/tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py
@@ -20,7 +20,7 @@ class TracingHasNotRunException(Exception):
 
 
 class TracingBackend(object):
-  def __init__(self, devtools_port, chrome_browser_backend):
+  def __init__(self, devtools_port):
     self._inspector_websocket = inspector_websocket.InspectorWebsocket(
         self._ErrorHandler)
     self._inspector_websocket.RegisterDomain(
@@ -30,42 +30,33 @@ class TracingBackend(object):
         'ws://127.0.0.1:%i/devtools/browser' % devtools_port)
     self._tracing_data = []
     self._is_tracing_running = False
-    self._chrome_browser_backend = chrome_browser_backend
 
   @property
   def is_tracing_running(self):
     return self._is_tracing_running
 
-  @property
-  def _devtools_client(self):
-    return self._chrome_browser_backend.devtools_client
-
   def StartTracing(self, trace_options, custom_categories=None, timeout=10):
-    """ Starts tracing on the first call and returns True. Returns False
-        and does nothing on subsequent nested calls.
+    """When first called, starts tracing, and returns True.
+
+    Once started, repeated calls do not affect tracing and return False.
     """
     if self.is_tracing_running:
       return False
     # Reset collected tracing data from previous tracing calls.
     self._tracing_data = []
     self._CheckNotificationSupported()
-    #TODO(nednguyen): remove this when the stable branch pass 2118.
-    if (trace_options.record_mode == tracing_options.RECORD_AS_MUCH_AS_POSSIBLE
-        and self._devtools_client.GetChromeBranchNumber()
-        and self._devtools_client.GetChromeBranchNumber() < 2118):
-      logging.warning(
-          'Cannot use %s tracing mode on chrome browser with branch version
%i,'
-          ' (<2118) fallback to use %s tracing mode' % (
-              trace_options.record_mode,
-              self._devtools_client.GetChromeBranchNumber(),
-              tracing_options.RECORD_UNTIL_FULL))
-      trace_options.record_mode = tracing_options.RECORD_UNTIL_FULL
-    req = {'method': 'Tracing.start'}
-    req['params'] = {}
+    # Map telemetry's tracing record_mode to the DevTools API string.
+    # (The keys happen to be the same as the values.)
     m = {tracing_options.RECORD_UNTIL_FULL: 'record-until-full',
          tracing_options.RECORD_AS_MUCH_AS_POSSIBLE:
          'record-as-much-as-possible'}
-    req['params']['options'] = m[trace_options.record_mode]
+    # DevTools started supporting RECORD_AS_MUCH_AS_POSSIBLE in Chrome 2118.
+    # However, we send it for earlier versions as well because Chrome ignores
+    # the unknown value and falls back to RECORD_UNTIL_FULL.
+    req = {
+        'method': 'Tracing.start',
+        'params': {'options': m[trace_options.record_mode]}
+        }
     if custom_categories:
       req['params']['categories'] = custom_categories
     self._inspector_websocket.SyncRequest(req, timeout)
@@ -73,9 +64,10 @@ class TracingBackend(object):
     return True
 
   def StopTracing(self, timeout=30):
-    """ Stops tracing and returns the raw json trace result. It this is called
-    after tracing has been stopped, trace data from the last tracing run is
-    returned.
+    """Stops tracing, and returns the raw json trace result.
+
+    If this is called after tracing has been stopped, this returns
+    trace data from the most recent tracing run.
     """
     if not self.is_tracing_running:
       if not self._tracing_data:

[nednguyen, 2014-12-23 02:06:06]

On 2014/12/22 23:26:36, chrishenry (OOO until Jan 5) wrote:
> Can you make sure that L webview is past this branch number or doesn't require
> this for other reason? Thanks!

Doesn't matter that much. I implemented this after the I landed the
record-as-much-as-possible tracing mode. If the chrome webview doesn't past that
branch number, then devtool implementation also ignore that options field.

[nednguyen, 2014-12-23 16:29:51]

The CQ bit was checked by nednguyen@google.com

[commit-bot: I haz the power, 2014-12-23 16:31:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/808563005/80001

[commit-bot: I haz the power, 2014-12-23 18:20:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-23 18:20:20]

Failed to apply patch for
tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py:
While running git apply --index -3 -p1;
  error: patch failed:
tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py:30
  Falling back to three-way merge...
  Applied patch to
'tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py' with
conflicts.
  U tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py

Patch:       tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py
Index: tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py
diff --git a/tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py
b/tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py
index
62426419bdc07504e10264d4b74b915761da93a8..f9fb353b6bdc1c5d4e507a7f3e76aa9ea5019619
100644
--- a/tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py
+++ b/tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py
@@ -20,7 +20,7 @@ class TracingHasNotRunException(Exception):
 
 
 class TracingBackend(object):
-  def __init__(self, devtools_port, chrome_browser_backend):
+  def __init__(self, devtools_port):
     self._inspector_websocket = inspector_websocket.InspectorWebsocket(
         self._ErrorHandler)
     self._inspector_websocket.RegisterDomain(
@@ -30,42 +30,33 @@ class TracingBackend(object):
         'ws://127.0.0.1:%i/devtools/browser' % devtools_port)
     self._tracing_data = []
     self._is_tracing_running = False
-    self._chrome_browser_backend = chrome_browser_backend
 
   @property
   def is_tracing_running(self):
     return self._is_tracing_running
 
-  @property
-  def _devtools_client(self):
-    return self._chrome_browser_backend.devtools_client
-
   def StartTracing(self, trace_options, custom_categories=None, timeout=10):
-    """ Starts tracing on the first call and returns True. Returns False
-        and does nothing on subsequent nested calls.
+    """When first called, starts tracing, and returns True.
+
+    Once started, repeated calls do not affect tracing and return False.
     """
     if self.is_tracing_running:
       return False
     # Reset collected tracing data from previous tracing calls.
     self._tracing_data = []
     self._CheckNotificationSupported()
-    #TODO(nednguyen): remove this when the stable branch pass 2118.
-    if (trace_options.record_mode == tracing_options.RECORD_AS_MUCH_AS_POSSIBLE
-        and self._devtools_client.GetChromeBranchNumber()
-        and self._devtools_client.GetChromeBranchNumber() < 2118):
-      logging.warning(
-          'Cannot use %s tracing mode on chrome browser with branch version
%i,'
-          ' (<2118) fallback to use %s tracing mode' % (
-              trace_options.record_mode,
-              self._devtools_client.GetChromeBranchNumber(),
-              tracing_options.RECORD_UNTIL_FULL))
-      trace_options.record_mode = tracing_options.RECORD_UNTIL_FULL
-    req = {'method': 'Tracing.start'}
-    req['params'] = {}
+    # Map telemetry's tracing record_mode to the DevTools API string.
+    # (The keys happen to be the same as the values.)
     m = {tracing_options.RECORD_UNTIL_FULL: 'record-until-full',
          tracing_options.RECORD_AS_MUCH_AS_POSSIBLE:
          'record-as-much-as-possible'}
-    req['params']['options'] = m[trace_options.record_mode]
+    # DevTools started supporting RECORD_AS_MUCH_AS_POSSIBLE in Chrome 2118.
+    # However, we send it for earlier versions as well because Chrome ignores
+    # the unknown value and falls back to RECORD_UNTIL_FULL.
+    req = {
+        'method': 'Tracing.start',
+        'params': {'options': m[trace_options.record_mode]}
+        }
     if custom_categories:
       req['params']['categories'] = custom_categories
     self._inspector_websocket.SyncRequest(req, timeout)
@@ -73,9 +64,10 @@ class TracingBackend(object):
     return True
 
   def StopTracing(self, timeout=30):
-    """ Stops tracing and returns the raw json trace result. It this is called
-    after tracing has been stopped, trace data from the last tracing run is
-    returned.
+    """Stops tracing, and returns the raw json trace result.
+
+    If this is called after tracing has been stopped, this returns
+    trace data from the most recent tracing run.
     """
     if not self.is_tracing_running:
       if not self._tracing_data:

[commit-bot: I haz the power, 2014-12-23 18:20:26]

The CQ bit was unchecked by commit-bot@chromium.org

[slamm, 2014-12-23 18:27:21]

Rebase

[slamm, 2014-12-23 18:34:13]

Patchset #5 (id:100001) has been deleted

[slamm, 2014-12-23 18:34:18]

The CQ bit was checked by slamm@chromium.org

[commit-bot: I haz the power, 2014-12-23 18:35:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/808563005/80001

[commit-bot: I haz the power, 2014-12-23 18:35:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-23 18:35:56]

Failed to apply patch for
tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py:
While running git apply --index -3 -p1;
  error: patch failed:
tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py:30
  Falling back to three-way merge...
  Applied patch to
'tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py' with
conflicts.
  U tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py

Patch:       tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py
Index: tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py
diff --git a/tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py
b/tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py
index
62426419bdc07504e10264d4b74b915761da93a8..f9fb353b6bdc1c5d4e507a7f3e76aa9ea5019619
100644
--- a/tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py
+++ b/tools/telemetry/telemetry/core/backends/chrome/tracing_backend.py
@@ -20,7 +20,7 @@ class TracingHasNotRunException(Exception):
 
 
 class TracingBackend(object):
-  def __init__(self, devtools_port, chrome_browser_backend):
+  def __init__(self, devtools_port):
     self._inspector_websocket = inspector_websocket.InspectorWebsocket(
         self._ErrorHandler)
     self._inspector_websocket.RegisterDomain(
@@ -30,42 +30,33 @@ class TracingBackend(object):
         'ws://127.0.0.1:%i/devtools/browser' % devtools_port)
     self._tracing_data = []
     self._is_tracing_running = False
-    self._chrome_browser_backend = chrome_browser_backend
 
   @property
   def is_tracing_running(self):
     return self._is_tracing_running
 
-  @property
-  def _devtools_client(self):
-    return self._chrome_browser_backend.devtools_client
-
   def StartTracing(self, trace_options, custom_categories=None, timeout=10):
-    """ Starts tracing on the first call and returns True. Returns False
-        and does nothing on subsequent nested calls.
+    """When first called, starts tracing, and returns True.
+
+    Once started, repeated calls do not affect tracing and return False.
     """
     if self.is_tracing_running:
       return False
     # Reset collected tracing data from previous tracing calls.
     self._tracing_data = []
     self._CheckNotificationSupported()
-    #TODO(nednguyen): remove this when the stable branch pass 2118.
-    if (trace_options.record_mode == tracing_options.RECORD_AS_MUCH_AS_POSSIBLE
-        and self._devtools_client.GetChromeBranchNumber()
-        and self._devtools_client.GetChromeBranchNumber() < 2118):
-      logging.warning(
-          'Cannot use %s tracing mode on chrome browser with branch version
%i,'
-          ' (<2118) fallback to use %s tracing mode' % (
-              trace_options.record_mode,
-              self._devtools_client.GetChromeBranchNumber(),
-              tracing_options.RECORD_UNTIL_FULL))
-      trace_options.record_mode = tracing_options.RECORD_UNTIL_FULL
-    req = {'method': 'Tracing.start'}
-    req['params'] = {}
+    # Map telemetry's tracing record_mode to the DevTools API string.
+    # (The keys happen to be the same as the values.)
     m = {tracing_options.RECORD_UNTIL_FULL: 'record-until-full',
          tracing_options.RECORD_AS_MUCH_AS_POSSIBLE:
          'record-as-much-as-possible'}
-    req['params']['options'] = m[trace_options.record_mode]
+    # DevTools started supporting RECORD_AS_MUCH_AS_POSSIBLE in Chrome 2118.
+    # However, we send it for earlier versions as well because Chrome ignores
+    # the unknown value and falls back to RECORD_UNTIL_FULL.
+    req = {
+        'method': 'Tracing.start',
+        'params': {'options': m[trace_options.record_mode]}
+        }
     if custom_categories:
       req['params']['categories'] = custom_categories
     self._inspector_websocket.SyncRequest(req, timeout)
@@ -73,9 +64,10 @@ class TracingBackend(object):
     return True
 
   def StopTracing(self, timeout=30):
-    """ Stops tracing and returns the raw json trace result. It this is called
-    after tracing has been stopped, trace data from the last tracing run is
-    returned.
+    """Stops tracing, and returns the raw json trace result.
+
+    If this is called after tracing has been stopped, this returns
+    trace data from the most recent tracing run.
     """
     if not self.is_tracing_running:
       if not self._tracing_data:

[commit-bot: I haz the power, 2014-12-23 18:36:06]

The CQ bit was unchecked by commit-bot@chromium.org

[slamm, 2015-01-02 22:07:36]

Rebase.

[slamm, 2015-01-02 22:08:50]

The CQ bit was checked by slamm@chromium.org

[commit-bot: I haz the power, 2015-01-02 22:09:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/808563005/120001

[commit-bot: I haz the power, 2015-01-03 00:09:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-01-03 00:09:21]

Try jobs failed on following builders:
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)

[slamm, 2015-01-03 03:07:37]

The CQ bit was checked by slamm@chromium.org

[commit-bot: I haz the power, 2015-01-03 03:08:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/808563005/120001

[commit-bot: I haz the power, 2015-01-03 04:10:19]

Committed patchset #5 (id:120001)

[commit-bot: I haz the power, 2015-01-03 04:11:13]

Patchset 5 (id:??) landed as
https://crrev.com/1ca83b0ff277b7ec1d10ed3a14dcca823ce72249
Cr-Commit-Position: refs/heads/master@{#309872}