New issue
Advanced search Search tips

Issue 822577 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

UserActivityManager::OnIdleEventObserved called every 30 seconds

Project Member Reported by npm@chromium.org, Mar 16 2018

Issue description

The method should be fired after 30 seconds of idleness. But now it's firing every 30 seconds, regardless. This means that the timer from IdleEventNotifier is not being reset properly.

This is a recent regression, and I've bisected to d13c288b1f348f2eab7d353064d05fae835b6f8f. That is, the method is fired every 30 seconds under that commit but not under the next, 3135d7d1cb10ef7d7f94da864643da26ed5a9c84.

The fact that this is broken right now is a problem because UKM will get logged in every roughly 30 seconds: MaybeLogEvent will log after idleness is detected and some other event happens (any user activity).
 

Comment 1 by npm@chromium.org, Mar 16 2018

Labels: OS-Chrome
Thanks for doing this analysis Nicolas! 

Hi Scott,

Would appreciate it if you could take a look urgently as it's not only causing flooding to UKM logs but also logs incorrect data. We're using these logged data to train ML models for improved power management on chrome OS.

Thanks.
Jia

Comment 3 by sky@chromium.org, Mar 16 2018

Status: Started (was: Assigned)
Do I need to do anything special to see this? Say enabled some flags?
Thanks Scott! You could either run it on linux by commenting out

https://cs.chromium.org/chromium/src/chrome/browser/chromeos/power/ml/user_activity_controller.cc?sq=package:chromium&dr=C&l=24-26

Or run it on a chromebook directly. Then you could see OnIdleEventNotifier called every 30sec, even with continuous user activity. 

Thanks,
Jia

Comment 5 by sky@chromium.org, Mar 16 2018

I'm reverting my patch that broke this here: https://chromium-review.googlesource.com/c/chromium/src/+/966694 . I will ensure when I reland that UserActivityDetector still works.
Thanks a lot! :)
Project Member

Comment 7 by bugdroid1@chromium.org, Mar 19 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f1f2c1882604d36eaa85e5a169fef069ad1a062b

commit f1f2c1882604d36eaa85e5a169fef069ad1a062b
Author: Scott Violet <sky@chromium.org>
Date: Mon Mar 19 16:41:04 2018

chromeos: Makes WindowTreeClient install PlatformEventSource

This only works for Ozone and is necessary for both
UserActivityDetector and remoting. Long term we need to rewrite both
of these to use something else, but for now this accomplishes what
they need.

BUG= 822577 
TEST=covered by tests

Change-Id: I432cbdcce17bb199bf8bb0f00ec51de81d3c71f0
Reviewed-on: https://chromium-review.googlesource.com/967447
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544063}
[modify] https://crrev.com/f1f2c1882604d36eaa85e5a169fef069ad1a062b/ui/aura/BUILD.gn
[add] https://crrev.com/f1f2c1882604d36eaa85e5a169fef069ad1a062b/ui/aura/mus/platform_event_source_mus_ozone.cc
[add] https://crrev.com/f1f2c1882604d36eaa85e5a169fef069ad1a062b/ui/aura/mus/platform_event_source_mus_ozone.h
[modify] https://crrev.com/f1f2c1882604d36eaa85e5a169fef069ad1a062b/ui/aura/mus/window_tree_client.cc
[modify] https://crrev.com/f1f2c1882604d36eaa85e5a169fef069ad1a062b/ui/aura/mus/window_tree_client.h
[modify] https://crrev.com/f1f2c1882604d36eaa85e5a169fef069ad1a062b/ui/aura/mus/window_tree_client_unittest.cc
[add] https://crrev.com/f1f2c1882604d36eaa85e5a169fef069ad1a062b/ui/aura/test/aura_test_suite.h
[modify] https://crrev.com/f1f2c1882604d36eaa85e5a169fef069ad1a062b/ui/aura/test/mus/window_tree_client_private.cc
[modify] https://crrev.com/f1f2c1882604d36eaa85e5a169fef069ad1a062b/ui/aura/test/run_all_unittests.cc
[modify] https://crrev.com/f1f2c1882604d36eaa85e5a169fef069ad1a062b/ui/events/platform/platform_event_source.h

Comment 8 by sky@chromium.org, Mar 20 2018

Status: Fixed (was: Started)

Sign in to add a comment