Remove usages of Timeline commands in telemetry

Remove usages of Timeline commands in telemetry

We are going to remove Timeline domain from the remote debugging protocol soon. This CL fixes remaining usages of timeline commands.

BUG=448318
Committed: https://crrev.com/9ca9eb0c7668c7a7e1f595ea0c4f3bca221e9350
Cr-Commit-Position: refs/heads/master@{#314522}

[yurys, 2015-02-03 12:21:37]

yurys@chromium.org changed reviewers:
+ caseq@chromium.org, pfeldman@chromium.org

[yurys, 2015-02-03 12:22:40]

yurys@chromium.org changed reviewers:
+ marja@chromium.org

[yurys, 2015-02-03 12:22:40]

@marja: please take a look

[marja, 2015-02-03 12:24:54]

I'm lacking context here. Is there something Timeline is replaced with? Which
benchmarks used to use it?

[yurys, 2015-02-03 12:36:25]

On 2015/02/03 12:24:54, marja wrote:
> I'm lacking context here. Is there something Timeline is replaced with? Which
> benchmarks used to use it?

We've switched from the old Timeline instrumentation to Tracing in DevTools. Old
Timeline implementation is deprecated and will be removed soon along with
"Timeline" domain in the remote debugging protocol as described in the bug.

There used to be one more usage of timeline but it was fixed recently in [1] so
this CL should be removing the last dependency on Timeline.

The only usage that I found is in
tools/chrome_proxy/integration_tests/network_metrics.py which calls
StartTimelineRecording [2]. Though StartTimelineRecording starts both network
recording and timeline recording [3] it only uses HTTPResponse events later. And
all those events belong to Network domain in the protocol, not timeline so there
is no much point in timeline recording.

[1] https://crrev.com/40c1889471c045e20793f37a95cd70622ea30625
[2]
https://code.google.com/p/chromium/codesearch#chromium/src/tools/chrome_proxy...
[3]
https://code.google.com/p/chromium/codesearch#chromium/src/tools/telemetry/te...
[4]
https://code.google.com/p/chromium/codesearch#chromium/src/tools/chrome_proxy...

[marja, 2015-02-03 12:51:39]

https://codereview.chromium.org/895043002/diff/20001/tools/telemetry/telemetr...
File
tools/telemetry/telemetry/core/backends/chrome_inspector/inspector_backend.py
(right):

https://codereview.chromium.org/895043002/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/backends/chrome_inspector/inspector_backend.py:192:
self._timeline_model = timeline_model_module.TimelineModel(
Does TimelineModel still make sense?

https://codereview.chromium.org/895043002/diff/20001/tools/telemetry/telemetr...
File
tools/telemetry/telemetry/core/backends/chrome_inspector/inspector_timeline.py
(left):

https://codereview.chromium.org/895043002/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/backends/chrome_inspector/inspector_timeline.py:16:
class Recorder(object):
Why is it sensemaking to remove Recorder, which says that it's an alternative
for calling Start and Stop below, and not remove Start and Stop?

[yurys, 2015-02-03 13:02:50]

https://codereview.chromium.org/895043002/diff/20001/tools/telemetry/telemetr...
File
tools/telemetry/telemetry/core/backends/chrome_inspector/inspector_backend.py
(right):

https://codereview.chromium.org/895043002/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/backends/chrome_inspector/inspector_backend.py:192:
self._timeline_model = timeline_model_module.TimelineModel(
On 2015/02/03 12:51:39, marja wrote:
> Does TimelineModel still make sense?

It can be fed with different input data and is used in many other places so it
should stay. At this place it is used just to covert recorded network events to
TimelineEvent interface. Actual conversion is done by
inspector_importer.InspectorTimelineImporter.

https://codereview.chromium.org/895043002/diff/20001/tools/telemetry/telemetr...
File
tools/telemetry/telemetry/core/backends/chrome_inspector/inspector_timeline.py
(left):

https://codereview.chromium.org/895043002/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/backends/chrome_inspector/inspector_timeline.py:16:
class Recorder(object):
On 2015/02/03 12:51:39, marja wrote:
> Why is it sensemaking to remove Recorder, which says that it's an alternative
> for calling Start and Stop below, and not remove Start and Stop?

No, it doesn't. Removed inspector_timeline.py altogether.

[marja, 2015-02-03 13:48:01]

lgtm modulo comments

https://codereview.chromium.org/895043002/diff/40001/tools/telemetry/telemetr...
File
tools/telemetry/telemetry/core/backends/chrome_inspector/inspector_network.py
(right):

https://codereview.chromium.org/895043002/diff/40001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/backends/chrome_inspector/inspector_network.py:214:
super(TimelineRecorder, self).__init__()
The super line is incorrect now

https://codereview.chromium.org/895043002/diff/40001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/timeline/recording_options.py (right):

https://codereview.chromium.org/895043002/diff/40001/tools/telemetry/telemetr...
tools/telemetry/telemetry/timeline/recording_options.py:6: class
TimelineRecordingOptions(object):
This class is not super useful any more, because the only option is to record
network (and not doing that makes no sense, so it's basically always true). Is
this going to change in the near future? If not, I'd just remove this class.

[yurys, 2015-02-03 14:32:52]

https://codereview.chromium.org/895043002/diff/40001/tools/telemetry/telemetr...
File
tools/telemetry/telemetry/core/backends/chrome_inspector/inspector_network.py
(right):

https://codereview.chromium.org/895043002/diff/40001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/backends/chrome_inspector/inspector_network.py:214:
super(TimelineRecorder, self).__init__()
On 2015/02/03 13:48:01, marja wrote:
> The super line is incorrect now

Good catch! Done.

https://codereview.chromium.org/895043002/diff/40001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/timeline/recording_options.py (right):

https://codereview.chromium.org/895043002/diff/40001/tools/telemetry/telemetr...
tools/telemetry/telemetry/timeline/recording_options.py:6: class
TimelineRecordingOptions(object):
On 2015/02/03 13:48:01, marja wrote:
> This class is not super useful any more, because the only option is to record
> network (and not doing that makes no sense, so it's basically always true). Is
> this going to change in the near future? If not, I'd just remove this class.

You are right, the only purpose of StartTimelineRecording now is tracking of
network activity. I removed TimelineRecordingOptions class and all its usages.
Dependent code now works as if record_network was true.

[yurys, 2015-02-03 14:33:49]

The CQ bit was checked by yurys@chromium.org

[commit-bot: I haz the power, 2015-02-03 14:34:05]

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

[commit-bot: I haz the power, 2015-02-03 16:34:15]

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

[commit-bot: I haz the power, 2015-02-03 16:34:16]

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...)

[yurys, 2015-02-04 07:20:07]

The CQ bit was checked by yurys@chromium.org

[commit-bot: I haz the power, 2015-02-04 07:20:35]

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

[commit-bot: I haz the power, 2015-02-04 08:16:51]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-02-04 08:18:02]

Patchset 4 (id:??) landed as
https://crrev.com/9ca9eb0c7668c7a7e1f595ea0c4f3bca221e9350
Cr-Commit-Position: refs/heads/master@{#314522}

[ccameron, 2015-02-04 18:25:07]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/903523002/ by ccameron@chromium.org.

The reason for reverting is: Windows telemetry unit tests are failing
intermittently after this patch.

Sample failing build:
http://build.chromium.org/p/chromium.win/builders/Vista%20Tests%20%281%29/bui...

Output:
[1/1]
telemetry.core.backends.chrome_inspector.inspector_network_unittest.InspectorNetworkTabTest.testHTTPResponseTimelineRecorder
failed unexpectedly:
  Could not find Flash at
E:\b\build\slave\Vista_Tests__1_\build\src\third_party\adobe\flash\binaries\ppapi\win\pepflashplayer.dll.
Continuing without Flash.
  To run with Flash, check it out via http://go/read-src-internal
  Traceback (most recent call last):
    File
"E:\b\build\slave\Vista_Tests__1_\build\src\tools\telemetry\telemetry\decorators.py",
line 55, in wrapper
      func(*args, **kwargs)
    File
"E:\b\build\slave\Vista_Tests__1_\build\src\tools\telemetry\telemetry\core\backends\chrome_inspector\inspector_network_unittest.py",
line 40, in testHTTPResponseTimelineRecorder
      self.assertEqual(test.responses_count, len(events))
  AssertionError: 1 != 2.