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

Issue 2562413003: Changes threshold frequency for investigation (based on video in the bug) (Closed)

Created:
4 years ago by varkha
Modified:
4 years ago
Reviewers:
oshima
CC:
chromium-reviews, kalyank, sadrul
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Changes threshold frequency for investigation (based on video in the bug) Looking some more at the video the frequency is not that high, so expecting 100 repetitions in 10 seconds was too optimistic and the logic in the implementation was wrong in any case. Expecting 20 invocations with at most 1 second in between subsequent events is more realistic while still very much improbable to do by hand, I think. BUG=665093 TEST=NONE (but will visit crash stats to check on this) Committed: https://crrev.com/3189d914bae27b9194aa9de802f2c55ae83632ea Cr-Commit-Position: refs/heads/master@{#440017}

Patch Set 1 : Changes threshold frequency for investigation (based on video in the bug) #

Total comments: 2

Patch Set 2 : Changes threshold frequency for investigation (fxed a test and reworked math) #

Unified diffs Side-by-side diffs Delta from patch set Stats (+14 lines, -7 lines) Patch
M ash/common/shelf/wm_shelf.h View 1 2 chunks +3 lines, -0 lines 0 comments Download
M ash/common/shelf/wm_shelf.cc View 1 2 chunks +7 lines, -7 lines 0 comments Download
M ash/shelf/shelf_layout_manager_unittest.cc View 1 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 32 (22 generated)
varkha
oshima@, can you please take a look? I thought the previous change would not actually ...
4 years ago (2016-12-12 21:51:27 UTC) #6
varkha
oshima@, can you please take a look? I thought the previous change would not actually ...
4 years ago (2016-12-12 21:51:41 UTC) #8
oshima
Looks like test is failing because it does change the state often and quickly. Can ...
4 years ago (2016-12-13 16:47:42 UTC) #16
varkha
On 2016/12/13 16:47:42, oshima wrote: > Looks like test is failing because it does change ...
4 years ago (2016-12-13 16:56:00 UTC) #17
varkha
Need to rework this, I will ping again when it is ready for review. https://codereview.chromium.org/2562413003/diff/20001/ash/common/shelf/wm_shelf.cc ...
4 years ago (2016-12-14 16:50:54 UTC) #18
varkha
I think I've fixed the troubles. PTAL. https://codereview.chromium.org/2562413003/diff/20001/ash/common/shelf/wm_shelf.cc File ash/common/shelf/wm_shelf.cc (right): https://codereview.chromium.org/2562413003/diff/20001/ash/common/shelf/wm_shelf.cc#newcode174 ash/common/shelf/wm_shelf.cc:174: kAutoHideRepeatInterval) { ...
4 years ago (2016-12-19 21:48:50 UTC) #21
oshima
lgtm
4 years ago (2016-12-21 01:32:05 UTC) #25
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/2562413003/40001
4 years ago (2016-12-21 04:12:10 UTC) #27
commit-bot: I haz the power
Committed patchset #2 (id:40001)
4 years ago (2016-12-21 04:40:35 UTC) #30
commit-bot: I haz the power
4 years ago (2016-12-21 04:42:11 UTC) #32
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/3189d914bae27b9194aa9de802f2c55ae83632ea
Cr-Commit-Position: refs/heads/master@{#440017}

Powered by Google App Engine
This is Rietveld 408576698