Changes threshold frequency for investigation (based on video in the bug)

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}

[varkha, 2016-12-12 21:48:05]

The CQ bit was checked by varkha@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-12 21:49:18]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[varkha, 2016-12-12 21:50:47]

Patchset #1 (id:1) has been deleted

[varkha, 2016-12-12 21:50:51]

The CQ bit was checked by varkha@chromium.org to run a CQ dry run

[varkha, 2016-12-12 21:51:08]

Description was changed from

==========
Changes treshold 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.
Expecting 30 invocations in 30 seconds is more realistic while still
very much impossible to do by hand I think.

BUG=665093
TEST=NONE (but will visit crash stats to check on this)
==========

to

==========
Changes treshold 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.
Expecting 15 invocations in 15 seconds is more realistic while still
very much impossible to do by hand I think.

BUG=665093
TEST=NONE (but will visit crash stats to check on this)
==========

[varkha, 2016-12-12 21:51:27]

oshima@, can you please take a look? I thought the previous change would not
actually trigger so posting this before merge.

[varkha, 2016-12-12 21:51:41]

varkha@chromium.org changed reviewers:
+ oshima@chromium.org

[varkha, 2016-12-12 21:51:41]

oshima@, can you please take a look? I thought the previous change would not
actually trigger so posting this.

[commit-bot: I haz the power, 2016-12-12 21:52:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-13 01:31:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-13 01:31:27]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[varkha, 2016-12-13 03:37:08]

The CQ bit was checked by varkha@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-13 03:37:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-13 05:16:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-13 05:16:33]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[oshima, 2016-12-13 16:47:42]

Looks like test is failing because it does change the state often and quickly.
Can you disable this in the test?

[varkha, 2016-12-13 16:56:00]

On 2016/12/13 16:47:42, oshima wrote:
> Looks like test is failing because it does change the state often and quickly.
> Can you disable this in the test?

Yes, saw this and indeed will need to disable this in tests.

[varkha, 2016-12-14 16:50:54]

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_she...
File ash/common/shelf/wm_shelf.cc (right):

https://codereview.chromium.org/2562413003/diff/20001/ash/common/shelf/wm_she...
ash/common/shelf/wm_shelf.cc:174: kAutoHideRepeatInterval) {
This actually feels wrong. My intention was to CHECK when number of events in
interval is > threshold. Both current and suggested code is doing something very
different so both are prone to being triggered manually. I will need to rework
this and also make the test that repeatedly swipes up and down on the shelf take
this into account.

[varkha, 2016-12-19 21:46:49]

The CQ bit was checked by varkha@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-19 21:47:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[varkha, 2016-12-19 21:48:50]

I think I've fixed the troubles. PTAL.

https://codereview.chromium.org/2562413003/diff/20001/ash/common/shelf/wm_she...
File ash/common/shelf/wm_shelf.cc (right):

https://codereview.chromium.org/2562413003/diff/20001/ash/common/shelf/wm_she...
ash/common/shelf/wm_shelf.cc:174: kAutoHideRepeatInterval) {
On 2016/12/14 16:50:54, varkha wrote:
> This actually feels wrong. My intention was to CHECK when number of events in
> interval is > threshold. Both current and suggested code is doing something
very
> different so both are prone to being triggered manually. I will need to rework
> this and also make the test that repeatedly swipes up and down on the shelf
take
> this into account.

Done.

[commit-bot: I haz the power, 2016-12-19 22:47:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-19 22:47:46]

Dry run: This issue passed the CQ dry run.

[varkha, 2016-12-20 02:52:30]

Description was changed from

==========
Changes treshold 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.
Expecting 15 invocations in 15 seconds is more realistic while still
very much impossible to do by hand I think.

BUG=665093
TEST=NONE (but will visit crash stats to check on this)
==========

to

==========
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)
==========

[oshima, 2016-12-21 01:32:05]

lgtm

[varkha, 2016-12-21 04:11:41]

The CQ bit was checked by varkha@chromium.org

[commit-bot: I haz the power, 2016-12-21 04:12:10]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-21 04:40:07]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1482293501308870,
"parent_rev": "441a0ef3ebcd230a099ade2fb86bf75c9aa40deb", "commit_rev":
"06cef99b321a0f0ee42af02499f3f9c5ad3ca217"}

[commit-bot: I haz the power, 2016-12-21 04:40:34]

Description was changed from

==========
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)
==========

to

==========
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)

Review-Url: https://codereview.chromium.org/2562413003
==========

[commit-bot: I haz the power, 2016-12-21 04:40:35]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-12-21 04:42:10]

Description was changed from

==========
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)

Review-Url: https://codereview.chromium.org/2562413003
==========

to

==========
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}
==========

[commit-bot: I haz the power, 2016-12-21 04:42:11]

Patchset 2 (id:??) landed as
https://crrev.com/3189d914bae27b9194aa9de802f2c55ae83632ea
Cr-Commit-Position: refs/heads/master@{#440017}