Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(33)

Issue 1041843002: [Telemetry] Do not close tab connections if the tab is already gone. (Closed)

Created:
5 years, 8 months ago by slamm
Modified:
5 years, 8 months ago
Reviewers:
nednguyen
CC:
chromium-reviews, telemetry-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[Telemetry] Do not close tab connections if the tab is already gone. Update tests so they check for formatted exceptions (so they will catch a bug such as this one). BUG=444240 Committed: https://crrev.com/86133f6e51b06f58afdcf4b00e65ffb3f510f34b Cr-Commit-Position: refs/heads/master@{#322613}

Patch Set 1 #

Patch Set 2 : comment tweak #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+42 lines, -16 lines) Patch
M tools/telemetry/telemetry/page/page_run_end_to_end_unittest.py View 1 12 chunks +39 lines, -15 lines 1 comment Download
M tools/telemetry/telemetry/page/shared_page_state.py View 2 chunks +3 lines, -1 line 0 comments Download

Messages

Total messages: 9 (2 generated)
slamm
5 years, 8 months ago (2015-03-27 17:00:01 UTC) #2
slamm
comment tweak
5 years, 8 months ago (2015-03-27 17:04:26 UTC) #3
nednguyen
lgtm https://codereview.chromium.org/1041843002/diff/20001/tools/telemetry/telemetry/page/page_run_end_to_end_unittest.py File tools/telemetry/telemetry/page/page_run_end_to_end_unittest.py (right): https://codereview.chromium.org/1041843002/diff/20001/tools/telemetry/telemetry/page/page_run_end_to_end_unittest.py#newcode108 tools/telemetry/telemetry/page/page_run_end_to_end_unittest.py:108: return self._formatted_exception_buffer.getvalue() So cool!
5 years, 8 months ago (2015-03-27 17:21:59 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1041843002/20001
5 years, 8 months ago (2015-03-27 17:24:13 UTC) #6
commit-bot: I haz the power
Committed patchset #2 (id:20001)
5 years, 8 months ago (2015-03-27 18:45:26 UTC) #7
commit-bot: I haz the power
Patchset 2 (id:??) landed as https://crrev.com/86133f6e51b06f58afdcf4b00e65ffb3f510f34b Cr-Commit-Position: refs/heads/master@{#322613}
5 years, 8 months ago (2015-03-27 18:46:20 UTC) #8
slamm
5 years, 8 months ago (2015-03-30 04:00:04 UTC) #9
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/1047613002/ by slamm@chromium.org.

The reason for reverting is: https://crbug.com/471512
Flaky telemetry_perf_unittests on all platforms due to ChromeTracingStartedError

Setting current_tab to None should be done for the crash tab/app errors only.

Also, consider guarding the other clean-up methods that use it, or make sure
they can dean with a None value..

Powered by Google App Engine
This is Rietveld 408576698