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

Issue 3018503002: Fix telemetry's IsBrowserRunning check (Closed)

Created:
3 years, 3 months ago by bokan
Modified:
3 years, 2 months ago
Reviewers:
perezju
CC:
catapult-reviews_chromium.org, telemetry-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
catapult
Visibility:
Public.

Description

Fix telemetry's IsBrowserRunning check This method currently checks to see if there's a process that matches the _prefix_ 'org.chromium.chrome' and returns true if there is. However, even if the browser itself is shutdown, there may be other processes running that match that prefix which can confuse telemetry. This patch fixes the issue by using GetApplicationPids which uses an exact match rather than a prefix. BUG=catapult:#3912 Review-Url: https://chromiumcodereview.appspot.com/3018503002 Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/ae0c06b444b18e43a058e8e833e50c8c548d81d9

Patch Set 1 #

Total comments: 2

Patch Set 2 : Use GetApplicationPids #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -1 line) Patch
M telemetry/telemetry/internal/platform/android_platform_backend.py View 1 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 23 (16 generated)
bokan
Hi perezju@, I'm wondering if this is the right fix and you look like you've ...
3 years, 3 months ago (2017-09-15 06:48:02 UTC) #6
bokan
ping, also, do I have to create a catapult bug? Where should that go if ...
3 years, 3 months ago (2017-09-19 04:20:39 UTC) #8
perezju
Thanks for looking into this! You can file a bug at: https://github.com/catapult-project/catapult/issues/ (or crbug.com, that ...
3 years, 3 months ago (2017-09-19 12:57:02 UTC) #9
bokan
Sorry for the delay (was traveling). New patch using your suggestion is up. https://codereview.chromium.org/3018503002/diff/1/telemetry/telemetry/internal/backends/chrome/android_browser_backend.py File ...
3 years, 2 months ago (2017-09-25 19:53:32 UTC) #15
perezju
lgtm thanks!
3 years, 2 months ago (2017-09-26 09:07:20 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/3018503002/20001
3 years, 2 months ago (2017-09-26 13:38:18 UTC) #20
commit-bot: I haz the power
3 years, 2 months ago (2017-09-26 13:40:02 UTC) #23
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...

Powered by Google App Engine
This is Rietveld 408576698