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

Issue 2657393003: Add unit-test for SharedSampler handling of zero-thread processes. (Closed)

Created:
3 years, 10 months ago by Wez
Modified:
3 years, 10 months ago
CC:
chromium-reviews
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add unit-test for SharedSampler handling of zero-thread processes. BUG=683849, 673175 Review-Url: https://codereview.chromium.org/2657393003 Cr-Commit-Position: refs/heads/master@{#448166} Committed: https://chromium.googlesource.com/chromium/src/+/a9abedb1e941cc2512f65bb3f66d5446033682d3

Patch Set 1 #

Patch Set 2 : Working #

Total comments: 13

Patch Set 3 : Address review comments #

Total comments: 1

Patch Set 4 : Remove anonymous namespace #

Unified diffs Side-by-side diffs Delta from patch set Stats (+203 lines, -314 lines) Patch
M chrome/browser/BUILD.gn View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/task_manager/sampling/shared_sampler.h View 1 1 chunk +8 lines, -0 lines 0 comments Download
D chrome/browser/task_manager/sampling/shared_sampler_unittest_win.cc View 1 chunk +0 lines, -204 lines 0 comments Download
M chrome/browser/task_manager/sampling/shared_sampler_win.cc View 1 2 4 chunks +24 lines, -106 lines 0 comments Download
A chrome/browser/task_manager/sampling/shared_sampler_win_defines.h View 1 2 3 1 chunk +113 lines, -0 lines 0 comments Download
A + chrome/browser/task_manager/sampling/shared_sampler_win_unittest.cc View 1 4 chunks +56 lines, -3 lines 0 comments Download
M chrome/test/BUILD.gn View 1 2 3 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 25 (14 generated)
Wez
stanisc: PTAL; hacking in the seam seems pretty ugly but I can't immediately see a ...
3 years, 10 months ago (2017-01-30 18:51:43 UTC) #3
stanisc
lgtm https://codereview.chromium.org/2657393003/diff/20001/chrome/browser/task_manager/sampling/shared_sampler_win.cc File chrome/browser/task_manager/sampling/shared_sampler_win.cc (right): https://codereview.chromium.org/2657393003/diff/20001/chrome/browser/task_manager/sampling/shared_sampler_win.cc#newcode96 chrome/browser/task_manager/sampling/shared_sampler_win.cc:96: if (g_query_system_information_for_test) { I wonder if it could ...
3 years, 10 months ago (2017-01-30 21:51:15 UTC) #8
Wez
https://codereview.chromium.org/2657393003/diff/20001/chrome/browser/task_manager/sampling/shared_sampler_win.cc File chrome/browser/task_manager/sampling/shared_sampler_win.cc (right): https://codereview.chromium.org/2657393003/diff/20001/chrome/browser/task_manager/sampling/shared_sampler_win.cc#newcode96 chrome/browser/task_manager/sampling/shared_sampler_win.cc:96: if (g_query_system_information_for_test) { On 2017/01/30 21:51:14, stanisc wrote: > ...
3 years, 10 months ago (2017-01-31 03:36:25 UTC) #9
afakhry
I think for Windows-specific changes, Nick is the right person for the review. +nick@ Can ...
3 years, 10 months ago (2017-01-31 03:51:13 UTC) #11
ncarter (slow)
https://codereview.chromium.org/2657393003/diff/20001/chrome/browser/task_manager/sampling/shared_sampler_win_defines.h File chrome/browser/task_manager/sampling/shared_sampler_win_defines.h (right): https://codereview.chromium.org/2657393003/diff/20001/chrome/browser/task_manager/sampling/shared_sampler_win_defines.h#newcode5 chrome/browser/task_manager/sampling/shared_sampler_win_defines.h:5: #ifndef CHROME_BROWSER_TASK_MANAGER_SAMPLING_SHARED_SAMPLER_WIN_DEFINES_H_ win_defines.h or defines_win.h ? https://codereview.chromium.org/2657393003/diff/20001/chrome/browser/task_manager/sampling/shared_sampler_win_defines.h#newcode6 chrome/browser/task_manager/sampling/shared_sampler_win_defines.h:6: #define ...
3 years, 10 months ago (2017-01-31 19:35:04 UTC) #12
Wez
https://codereview.chromium.org/2657393003/diff/20001/chrome/browser/task_manager/sampling/shared_sampler_win_defines.h File chrome/browser/task_manager/sampling/shared_sampler_win_defines.h (right): https://codereview.chromium.org/2657393003/diff/20001/chrome/browser/task_manager/sampling/shared_sampler_win_defines.h#newcode5 chrome/browser/task_manager/sampling/shared_sampler_win_defines.h:5: #ifndef CHROME_BROWSER_TASK_MANAGER_SAMPLING_SHARED_SAMPLER_WIN_DEFINES_H_ On 2017/01/31 19:35:03, ncarter wrote: > win_defines.h ...
3 years, 10 months ago (2017-02-01 01:24:46 UTC) #13
ncarter (slow)
https://codereview.chromium.org/2657393003/diff/20001/chrome/browser/task_manager/sampling/shared_sampler_win_defines.h File chrome/browser/task_manager/sampling/shared_sampler_win_defines.h (right): https://codereview.chromium.org/2657393003/diff/20001/chrome/browser/task_manager/sampling/shared_sampler_win_defines.h#newcode13 chrome/browser/task_manager/sampling/shared_sampler_win_defines.h:13: namespace { On 2017/02/01 01:24:45, Wez wrote: > On ...
3 years, 10 months ago (2017-02-01 21:55:29 UTC) #14
Wez
OK, fair point; will follow-up w/ a patch to fix that. On 1 February 2017 ...
3 years, 10 months ago (2017-02-01 22:05:15 UTC) #15
ncarter (slow)
https://codereview.chromium.org/2657393003/diff/40001/chrome/browser/task_manager/sampling/shared_sampler_win_defines.h File chrome/browser/task_manager/sampling/shared_sampler_win_defines.h (right): https://codereview.chromium.org/2657393003/diff/40001/chrome/browser/task_manager/sampling/shared_sampler_win_defines.h#newcode13 chrome/browser/task_manager/sampling/shared_sampler_win_defines.h:13: namespace { lgtm assuming this "namespace {" is dropped ...
3 years, 10 months ago (2017-02-02 20:17:14 UTC) #16
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/2657393003/60001
3 years, 10 months ago (2017-02-05 00:30:07 UTC) #22
commit-bot: I haz the power
3 years, 10 months ago (2017-02-05 02:22:55 UTC) #25
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/a9abedb1e941cc2512f65bb3f66d...

Powered by Google App Engine
This is Rietveld 408576698