New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 854212 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 10
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocked on:
issue 860297

Blocking:
issue 760498



Sign in to add a comment

It should be possible to restart the browser while tracing

Project Member Reported by perezju@chromium.org, Jun 19 2018

Issue description

Something like the following should work, but doesn't:

    platform.tracing_controller.StartTracing(config)
    try:
      browser_to_create.SetUpEnvironment(browser_options)
      for _ in xrange(2):
        browser = browser_to_create.Create()
        browser.tabs[0].Navigate('about:blank')
        browser.tabs[0].WaitForDocumentReadyStateToBeInteractiveOrBetter()
        browser.Close()
      trace_data, errors = tracing_controller.StopTracing()
    finally:
      browser_to_create.CleanUpEnvironment()

Problems are:

1. borwser.Close() should FlushTracing() so we don't lose the trace from that browser.
2. Currently a browser needs to be open when StopTracing is called ("a clock sync" is sent to it, but this breaks if the browser is not there).
3. Even after fixing 1 and 2, restarting the browser in this way causes new tabs to accumulate, and sending DevTools requests to the older tabs causes Telemetry to hang (the request times out).
 
Note the details of the inner loop are not that relevant.

In general the sequence "cause the browser to start, do something with it, close it" x repeat a few times; should work.
Labels: -Pri-3 Pri-1
Status: Started (was: Assigned)
Bumping pri up just to note I'm actively working on this.

Comment 3 by pasko@chromium.org, Jun 21 2018

Blocking: 760498
Project Member

Comment 4 by bugdroid1@chromium.org, Jun 22 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/ce654d76d538a725a4197025dca85f39553e340c

commit ce654d76d538a725a4197025dca85f39553e340c
Author: Juan Antonio Navarro Perez <perezju@chromium.org>
Date: Fri Jun 22 16:04:36 2018

[Telemetry] Do not raise on missing devtools clients for clock sync

Some workflows close the browser before tracing is stopped (and a final
clock sync is recorded). It should not be an error that no DevTools
clients exist at that time.

Bug:  chromium:854212 
Change-Id: I14930725ef3f2b57da56565245b0929219329b69
Reviewed-on: https://chromium-review.googlesource.com/1112006
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Juan Antonio Navarro Pérez <perezju@chromium.org>

[modify] https://crrev.com/ce654d76d538a725a4197025dca85f39553e340c/telemetry/telemetry/core/tracing_controller_unittest.py
[modify] https://crrev.com/ce654d76d538a725a4197025dca85f39553e340c/telemetry/telemetry/internal/platform/tracing_agent/chrome_tracing_agent.py

Project Member

Comment 5 by bugdroid1@chromium.org, Jun 22 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a376b8dce6b0139d14794b42e51f75373730784c

commit a376b8dce6b0139d14794b42e51f75373730784c
Author: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Fri Jun 22 16:39:23 2018

Roll src/third_party/catapult 87eefd4f1143..e3b12b3e7d98 (1 commits)

https://chromium.googlesource.com/catapult.git/+log/87eefd4f1143..e3b12b3e7d98


git log 87eefd4f1143..e3b12b3e7d98 --date=short --no-merges --format='%ad %ae %s'
2018-06-22 perezju@chromium.org [Telemetry] Fix and re-enable StartupTracingTest


Created with:
  gclient setdep -r src/third_party/catapult@e3b12b3e7d98

The AutoRoll server is located here: https://catapult-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG= chromium:854212 
TBR=sullivan@chromium.org

Change-Id: Ibd0c21dd2e8139f038212adaccdf40a5b24fa8e0
Reviewed-on: https://chromium-review.googlesource.com/1111921
Reviewed-by: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#569654}
[modify] https://crrev.com/a376b8dce6b0139d14794b42e51f75373730784c/DEPS

Project Member

Comment 6 by bugdroid1@chromium.org, Jun 22 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f2ac61ff6103f952097e76fcb3cd43461f6614d9

commit f2ac61ff6103f952097e76fcb3cd43461f6614d9
Author: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Fri Jun 22 19:22:39 2018

Roll src/third_party/catapult e3b12b3e7d98..fa74d9840634 (2 commits)

https://chromium.googlesource.com/catapult.git/+log/e3b12b3e7d98..fa74d9840634


git log e3b12b3e7d98..fa74d9840634 --date=short --no-merges --format='%ad %ae %s'
2018-06-22 perezju@chromium.org [Telemetry] Should not "warn" when computing TBM
2018-06-22 perezju@chromium.org [Telemetry] Do not raise on missing devtools clients for clock sync


Created with:
  gclient setdep -r src/third_party/catapult@fa74d9840634

The AutoRoll server is located here: https://catapult-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG= chromium:854212 
TBR=sullivan@chromium.org

Change-Id: I228ac2a4d7ae2ad4ae535d3dd69fe390b0c73c44
Reviewed-on: https://chromium-review.googlesource.com/1112119
Reviewed-by: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#569731}
[modify] https://crrev.com/f2ac61ff6103f952097e76fcb3cd43461f6614d9/DEPS

Project Member

Comment 7 by bugdroid1@chromium.org, Jun 25 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/7d423fc5614bf9265b1c84005c62d64f554d00a5

commit 7d423fc5614bf9265b1c84005c62d64f554d00a5
Author: Juan Antonio Navarro Perez <perezju@chromium.org>
Date: Mon Jun 25 13:13:02 2018

[Telemetry] Remove workaround for closing browser before tracing stops

After the dependency lands:
https://chromium-review.googlesource.com/c/catapult/+/1112006

It's possible to remove one of the workarounds for tracing while
restarting the browser.

Bug:  chromium:854212 
Change-Id: I4445e0633e6522a0162ec05d69f18e5ede5c3693
Reviewed-on: https://chromium-review.googlesource.com/1112011
Reviewed-by: Egor Pasko <pasko@chromium.org>
Commit-Queue: Juan Antonio Navarro Pérez <perezju@chromium.org>

[modify] https://crrev.com/7d423fc5614bf9265b1c84005c62d64f554d00a5/telemetry/examples/benchmarks/android_go_benchmark.py

Project Member

Comment 8 by bugdroid1@chromium.org, Jun 25 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1629896a91fa98a8f9a369cf0f1f23e41d316e05

commit 1629896a91fa98a8f9a369cf0f1f23e41d316e05
Author: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Mon Jun 25 17:17:01 2018

Roll src/third_party/catapult 29f3b8927f88..7d423fc5614b (1 commits)

https://chromium.googlesource.com/catapult.git/+log/29f3b8927f88..7d423fc5614b


git log 29f3b8927f88..7d423fc5614b --date=short --no-merges --format='%ad %ae %s'
2018-06-25 perezju@chromium.org [Telemetry] Remove workaround for closing browser before tracing stops


Created with:
  gclient setdep -r src/third_party/catapult@7d423fc5614b

The AutoRoll server is located here: https://catapult-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG= chromium:854212 
TBR=sullivan@chromium.org

Change-Id: I3c562b6ca2fdc6a3d7c2c758c0f1e4dde5b70e8e
Reviewed-on: https://chromium-review.googlesource.com/1113478
Reviewed-by: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#570078}
[modify] https://crrev.com/1629896a91fa98a8f9a369cf0f1f23e41d316e05/DEPS

I was able to reproduce  issue 3  on telemetry_mini, so this appears to be a DevTools issue (not Telemetry itself).

The repro steps are something like:

1. Write some basic command line file on the device.
2. Clear the browser profile.
3. Launch the browser on about:blank
4. Wait a bit. Close the browser.
5. Launch the browser on about:blank  # Now there are two tabs.
6. Try to connect to each of the two tabs via devtools and send a simple command, e.g. Runtime.evaluate('1 + 1').

On 6 the command succeeds on the most recent tab, but fails on the tab left over from 3.

Writing down a small repro case on python now. Who should be able to help us debug this from Chrome's side?
Cc: caseq@chromium.org pfeldman@chromium.org
+Pavel/Andrey for help on the Chrome's side. Folks: this is a devtool bug

Comment 11 by pasko@chromium.org, Jun 26 2018

On Android we do not create WebContents for background tabs on start. Those are created lazily when a user switches to the tab. So there is by design no DevTools instance to connect to.
Is there a way to query Chrome to get back which tabs have a DevTools instance and which don't?

Or should we try to "guess" this from Telemetry's side?

Comment 13 by pasko@chromium.org, Jun 27 2018

What is the usecase where asking Chrome for the list of devtools instances is needed?

I don't understand the Tab abstraction in Telemetry, does it allow benchmarking things happening in multiple tabs? There is heavy throttling in invisible tabs, so I am not sure we want any data from there.

I always (perhaps naively) assumed an interface to grab devtools from the first foreground tab is all we need for benchmarking. Though I don't know whether the devtools socket exposed from the browser allows doing it.

If we need to close background tabs, we perhaps could expose just that?
# First the general case

In general the Telemetry APIs (exposed all the way up to stories) allow clients to iterate over all tabs and send commands to them. And clients are allowed to do whatever they want with these APIs.

Maybe the current behavior is "fine" in which sending a command to a background tab just times out. And clients need to be aware that this is a possibility.

Alternatively I was thinking if there was a way for a client to know before sending a command, whether the tab was alive or not, so avoid sending the command at all.

# Now on the specific case

Why Telemetry hangs with a story that is not trying to do anything fancy with background tabs?

I think at least one of the issues was the clock sync, which is sent to all tabs. As Ned pointed out, maybe it's fine to send the command just to the foreground tab? (Is that always the first tab you get when iterating over them?)

And I remember there were a few other places where Telemetry tries to iterate over all tabs. I would have to tease out each of them to see why we're doing that.

Comment 15 by pasko@chromium.org, Jun 27 2018

Thank you for the background, Juan!

For the general case we would need to figure out whether the clients really need to iterate over tabs - got it :)

On Android (at least) sending something to background tabs should expect some flakiness - those processes are at the lowest priority on the system, so on any hint of memory pressure they would be killed first, dropping the devtools communication channel.

For this specific case .. the clock sync from just one tab should work (we even drop multiple clock sync events when merging traces). As usual, I am not aware of all intricacies .. the sync from all tabs does not seem necessary in principle, there could be bugs to fix though.
Project Member

Comment 16 by bugdroid1@chromium.org, Jun 29 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/d1692d4ac997001370ef36debef96cd0d992c13e

commit d1692d4ac997001370ef36debef96cd0d992c13e
Author: Juan Antonio Navarro Perez <perezju@chromium.org>
Date: Fri Jun 29 19:24:25 2018

[Telemetry] Test markers are found in trace during StartupTracingTest

To make these tests more robust, check that markes injected into traces
are found when we read them back.

Also switch to use the new trace_runner.ExecuteMappingCodeOnTraceData
and try to factor out some common test code.

Bug:  chromium:854212 
Change-Id: Ib13902a0289dd08e8dff15ecba62d56e901b93bb
Reviewed-on: https://chromium-review.googlesource.com/1114856
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Ned Nguyen <nednguyen@google.com>

[modify] https://crrev.com/d1692d4ac997001370ef36debef96cd0d992c13e/telemetry/telemetry/core/tracing_controller_unittest.py

Project Member

Comment 17 by bugdroid1@chromium.org, Jun 29 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/608c759406e8202907cc324240f1bce6aaa6d58d

commit 608c759406e8202907cc324240f1bce6aaa6d58d
Author: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Fri Jun 29 21:53:07 2018

Roll src/third_party/catapult f76f0b44062c..d1692d4ac997 (2 commits)

https://chromium.googlesource.com/catapult.git/+log/f76f0b44062c..d1692d4ac997


git log f76f0b44062c..d1692d4ac997 --date=short --no-merges --format='%ad %ae %s'
2018-06-29 perezju@chromium.org [Telemetry] Test markers are found in trace during StartupTracingTest
2018-06-29 perezju@chromium.org [dashboard] Make returning bug comments optional


Created with:
  gclient setdep -r src/third_party/catapult@d1692d4ac997

The AutoRoll server is located here: https://catapult-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG= chromium:854212 
TBR=sullivan@chromium.org

Change-Id: I14cafb4de0ff9c4b82b5a6ba9ce25cd4574e7e19
Reviewed-on: https://chromium-review.googlesource.com/1121016
Reviewed-by: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#571653}
[modify] https://crrev.com/608c759406e8202907cc324240f1bce6aaa6d58d/DEPS

Blockedon: 760553
Project Member

Comment 19 by bugdroid1@chromium.org, Jul 4

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/82213060d5ce0d376671dda9ca3928b5ad1c9c69

commit 82213060d5ce0d376671dda9ca3928b5ad1c9c69
Author: Juan Antonio Navarro Perez <perezju@chromium.org>
Date: Wed Jul 04 13:29:51 2018

[Telemetry] Add flush-tracing and sync markers on first tab only

The DevTools client might list contexts which do not yet have a live
DevTools instance to connect to (e.g. background tabs which may have
been discarded or not yet created).

In some situations trying to place markers on all contexts will
cause us to hang when we hit one with a non yet live DevTools
instance. To prevent this, and since this is enough for tracing
purposes, markers will now be added to the firstly created tab only.

Bug:  chromium:854212 
Change-Id: If897b7903102590a58224546eb713380849fd03d
Reviewed-on: https://chromium-review.googlesource.com/1124695
Commit-Queue: Juan Antonio Navarro Pérez <perezju@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>

[modify] https://crrev.com/82213060d5ce0d376671dda9ca3928b5ad1c9c69/telemetry/telemetry/internal/platform/tracing_agent/chrome_tracing_agent_unittest.py
[modify] https://crrev.com/82213060d5ce0d376671dda9ca3928b5ad1c9c69/telemetry/telemetry/internal/platform/tracing_agent/chrome_tracing_agent.py
[modify] https://crrev.com/82213060d5ce0d376671dda9ca3928b5ad1c9c69/telemetry/telemetry/internal/backends/chrome_inspector/devtools_client_backend.py

Blockedon: 860297
Project Member

Comment 21 by bugdroid1@chromium.org, Jul 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/905d3efd2b5b204b0390fcb370226e8d21fa8978

commit 905d3efd2b5b204b0390fcb370226e8d21fa8978
Author: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Wed Jul 04 16:03:03 2018

Roll src/third_party/catapult 5abd99f5f768..82213060d5ce (2 commits)

https://chromium.googlesource.com/catapult.git/+log/5abd99f5f768..82213060d5ce


git log 5abd99f5f768..82213060d5ce --date=short --no-merges --format='%ad %ae %s'
2018-07-04 perezju@chromium.org [Telemetry] Add flush-tracing and sync markers on first tab only
2018-07-04 perezju@chromium.org [dashboard] Skip flaky ListTestSuitesTest.testPartialTestSuites


Created with:
  gclient setdep -r src/third_party/catapult@82213060d5ce

The AutoRoll server is located here: https://catapult-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG= chromium:854212 , chromium:860103 
TBR=sullivan@chromium.org

Change-Id: I964cb29eb623c77634a2b66357acdf4d71f852e2
Reviewed-on: https://chromium-review.googlesource.com/1126281
Reviewed-by: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#572594}
[modify] https://crrev.com/905d3efd2b5b204b0390fcb370226e8d21fa8978/DEPS

Project Member

Comment 22 by bugdroid1@chromium.org, Jul 9

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/45198367e90a402718e4b07d85e8edcdfdbdd3c2

commit 45198367e90a402718e4b07d85e8edcdfdbdd3c2
Author: Juan Antonio Navarro Perez <perezju@chromium.org>
Date: Mon Jul 09 10:05:20 2018

[Telemetry] Add testRestartBrowserWhileTracing

After the work on  crbug.com/860297 , this test can now succeed.

Bug:  chromium:854212 
Change-Id: Ia2e035fcbb5f47a29775b901ff15c5bfea57fb0a
Reviewed-on: https://chromium-review.googlesource.com/1127884
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Juan Antonio Navarro Pérez <perezju@chromium.org>

[modify] https://crrev.com/45198367e90a402718e4b07d85e8edcdfdbdd3c2/telemetry/telemetry/core/tracing_controller_unittest.py

Components: Speed>Telemetry
Project Member

Comment 24 by bugdroid1@chromium.org, Jul 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/93cb388c78f505cd69bf7ae4be7d4ffb1165f606

commit 93cb388c78f505cd69bf7ae4be7d4ffb1165f606
Author: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Mon Jul 09 13:03:22 2018

Roll src/third_party/catapult d8e6a8203fc2..45198367e90a (1 commits)

https://chromium.googlesource.com/catapult.git/+log/d8e6a8203fc2..45198367e90a


git log d8e6a8203fc2..45198367e90a --date=short --no-merges --format='%ad %ae %s'
2018-07-09 perezju@chromium.org [Telemetry] Add testRestartBrowserWhileTracing


Created with:
  gclient setdep -r src/third_party/catapult@45198367e90a

The AutoRoll server is located here: https://catapult-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG= chromium:854212 
TBR=sullivan@chromium.org

Change-Id: Ic02cf56843d98a212e8e237ab3b4d54fce6e68f9
Reviewed-on: https://chromium-review.googlesource.com/1128801
Reviewed-by: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#573281}
[modify] https://crrev.com/93cb388c78f505cd69bf7ae4be7d4ffb1165f606/DEPS

Project Member

Comment 25 by bugdroid1@chromium.org, Jul 9

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/27380b171e3fd0073d475e0fb628fe8850571d06

commit 27380b171e3fd0073d475e0fb628fe8850571d06
Author: Juan Antonio Navarro Perez <perezju@chromium.org>
Date: Mon Jul 09 14:05:50 2018

[Telemetry] Flush tracing on browser close

When closing a browser, and if Chrome tracing is still running, also
flush the trace so we keep a copy of it.

This should not affect most benchmarks in which the browser is closed
after tracing has stopped.

Bug:  chromium:854212 
Change-Id: Ib2082ca2a84a473a7bc6a8b759aa2c6235625c67
Reviewed-on: https://chromium-review.googlesource.com/1128859
Reviewed-by: Egor Pasko <pasko@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Juan Antonio Navarro Pérez <perezju@chromium.org>

[modify] https://crrev.com/27380b171e3fd0073d475e0fb628fe8850571d06/telemetry/telemetry/internal/backends/chrome/chrome_browser_backend.py
[modify] https://crrev.com/27380b171e3fd0073d475e0fb628fe8850571d06/telemetry/examples/benchmarks/android_go_benchmark.py
[modify] https://crrev.com/27380b171e3fd0073d475e0fb628fe8850571d06/telemetry/telemetry/core/tracing_controller.py
[modify] https://crrev.com/27380b171e3fd0073d475e0fb628fe8850571d06/telemetry/telemetry/internal/platform/tracing_controller_backend.py
[modify] https://crrev.com/27380b171e3fd0073d475e0fb628fe8850571d06/telemetry/telemetry/core/tracing_controller_unittest.py

Project Member

Comment 26 by bugdroid1@chromium.org, Jul 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8bcda46d91e7b4a93052cc94f1b2b5498a64c20e

commit 8bcda46d91e7b4a93052cc94f1b2b5498a64c20e
Author: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Mon Jul 09 16:30:07 2018

Roll src/third_party/catapult 45198367e90a..27380b171e3f (1 commits)

https://chromium.googlesource.com/catapult.git/+log/45198367e90a..27380b171e3f


git log 45198367e90a..27380b171e3f --date=short --no-merges --format='%ad %ae %s'
2018-07-09 perezju@chromium.org [Telemetry] Flush tracing on browser close


Created with:
  gclient setdep -r src/third_party/catapult@27380b171e3f

The AutoRoll server is located here: https://catapult-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG= chromium:854212 
TBR=sullivan@chromium.org

Change-Id: I3be00888706587ebe35ec5275b598388cb095acc
Reviewed-on: https://chromium-review.googlesource.com/1129079
Reviewed-by: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#573329}
[modify] https://crrev.com/8bcda46d91e7b4a93052cc94f1b2b5498a64c20e/DEPS

Blockedon: -760553
Project Member

Comment 28 by bugdroid1@chromium.org, Jul 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0caba3d8a33b7aa4e5dc9a1e76eededff4bc7b00

commit 0caba3d8a33b7aa4e5dc9a1e76eededff4bc7b00
Author: Juan Antonio Navarro Perez <perezju@chromium.org>
Date: Tue Jul 10 10:36:29 2018

[tools/perf] Remove hacks to close browser while tracing

These are no longer needed.

Bug:  854212 
Change-Id: I04be67fc6977d1e8162b688e5a0ef7b52dda24bf
Reviewed-on: https://chromium-review.googlesource.com/1130520
Reviewed-by: Egor Pasko <pasko@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Juan Antonio Navarro Pérez <perezju@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573677}
[modify] https://crrev.com/0caba3d8a33b7aa4e5dc9a1e76eededff4bc7b00/tools/perf/benchmarks/startup_mobile.py

Status: Fixed (was: Started)
🎉

Comment 30 by benhenry@google.com, Jan 16 (6 days ago)

Components: Test>Telemetry

Comment 31 by benhenry@google.com, Jan 16 (6 days ago)

Components: -Speed>Telemetry

Sign in to add a comment