New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 877178 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 4
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 0
Type: Bug



Sign in to add a comment

Child status reporting daily data aggregation does not fit Unicorn time limit schedule.

Project Member Reported by agawronska@chromium.org, Aug 23

Issue description

User activity for status reporting is aggregated by day starting at midnight. Unicorn time limits are relying on different schedule (the time limit is refreshed daily at 6am). Because of that user activity cannot be properly calculated.

To solve it we can customize day start in activity reporting based on value passe in TimeLimits policy. 
 
Labels: -M-70 M-69
Cc: cindyb@chromium.org
Labels: OS-Chrome
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 23

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

commit abae2fe888bffc7e37b1a108d9825fa691e4155f
Author: Aga Wronska <agawronska@chromium.org>
Date: Thu Aug 23 23:47:59 2018

Allow to customize day start for activity reporting aggreegation in
DeviceStatusCollector.

DeviceStatusCollector takes day start as an argument. For enterprise
reporting it is set to midnight as it was before. For consumer/child
reporting it is read from UsageTimeLimits policy.

If the day start changes data are aggregated into new day buckets
whithout affecting pre-existing buckets. It means that previously
stored data will not be mapped to the new day start buckets.

Bug:  877178 
Test: Run tests in device_status_collector_browsertests.
      Manually simulate status reports for enterprise and consumer.


Change-Id: I60fe3a9f22149aefbfe81c0a48ddbf6ed29ce1fd
Reviewed-on: https://chromium-review.googlesource.com/1184397
Commit-Queue: Aga Wronska <agawronska@chromium.org>
Reviewed-by: Rahul Chaturvedi <rkc@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585661}
[modify] https://crrev.com/abae2fe888bffc7e37b1a108d9825fa691e4155f/chrome/browser/chromeos/child_accounts/consumer_status_reporting_service.cc
[modify] https://crrev.com/abae2fe888bffc7e37b1a108d9825fa691e4155f/chrome/browser/chromeos/child_accounts/consumer_status_reporting_service.h
[modify] https://crrev.com/abae2fe888bffc7e37b1a108d9825fa691e4155f/chrome/browser/chromeos/child_accounts/usage_time_limit_processor.cc
[modify] https://crrev.com/abae2fe888bffc7e37b1a108d9825fa691e4155f/chrome/browser/chromeos/child_accounts/usage_time_limit_processor.h
[modify] https://crrev.com/abae2fe888bffc7e37b1a108d9825fa691e4155f/chrome/browser/chromeos/child_accounts/usage_time_limit_processor_unittest.cc
[modify] https://crrev.com/abae2fe888bffc7e37b1a108d9825fa691e4155f/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos.cc
[modify] https://crrev.com/abae2fe888bffc7e37b1a108d9825fa691e4155f/chrome/browser/chromeos/policy/device_status_collector.cc
[modify] https://crrev.com/abae2fe888bffc7e37b1a108d9825fa691e4155f/chrome/browser/chromeos/policy/device_status_collector.h
[modify] https://crrev.com/abae2fe888bffc7e37b1a108d9825fa691e4155f/chrome/browser/chromeos/policy/device_status_collector_browsertest.cc
[modify] https://crrev.com/abae2fe888bffc7e37b1a108d9825fa691e4155f/chrome/browser/chromeos/policy/status_uploader_unittest.cc

Labels: Merge-Request-69
Status: Fixed (was: Started)
Project Member

Comment 6 by sheriffbot@chromium.org, Aug 24

Labels: -Merge-Request-69 Merge-Review-69 Hotlist-Merge-Review
This bug requires manual review: We are only 10 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Has this been verified in ToT? Tested on variety of devices?
On my end I tried it on eve and meowth.
Unicorn team is doing server side changes and so far they say that this change should fix the problem for them.    
I have performed some e2e tests and this change solved the issue.
Labels: -Merge-Review-69 Merge-Approved-69
Project Member

Comment 11 by bugdroid1@chromium.org, Aug 29

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/922f06de958dcf28211127747145f1ce9e0413e5

commit 922f06de958dcf28211127747145f1ce9e0413e5
Author: Aga Wronska <agawronska@chromium.org>
Date: Wed Aug 29 20:20:40 2018

Allow to customize day start for activity reporting aggreegation in DeviceStatusCollector.

DeviceStatusCollector takes day start as an argument. For enterprise
reporting it is set to midnight as it was before. For consumer/child
reporting it is read from UsageTimeLimits policy.

If the day start changes data are aggregated into new day buckets
whithout affecting pre-existing buckets. It means that previously
stored data will not be mapped to the new day start buckets.

Bug:  877178 
Test: Run tests in device_status_collector_browsertests.
      Manually simulate status reports for enterprise and consumer.

TBR=agawronska@chromium.org

(cherry picked from commit abae2fe888bffc7e37b1a108d9825fa691e4155f)

Change-Id: I60fe3a9f22149aefbfe81c0a48ddbf6ed29ce1fd
Reviewed-on: https://chromium-review.googlesource.com/1184397
Commit-Queue: Aga Wronska <agawronska@chromium.org>
Reviewed-by: Rahul Chaturvedi <rkc@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#585661}
Reviewed-on: https://chromium-review.googlesource.com/1187715
Reviewed-by: Aga Wronska <agawronska@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#843}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/922f06de958dcf28211127747145f1ce9e0413e5/chrome/browser/chromeos/child_accounts/consumer_status_reporting_service.cc
[modify] https://crrev.com/922f06de958dcf28211127747145f1ce9e0413e5/chrome/browser/chromeos/child_accounts/consumer_status_reporting_service.h
[modify] https://crrev.com/922f06de958dcf28211127747145f1ce9e0413e5/chrome/browser/chromeos/child_accounts/usage_time_limit_processor.cc
[modify] https://crrev.com/922f06de958dcf28211127747145f1ce9e0413e5/chrome/browser/chromeos/child_accounts/usage_time_limit_processor.h
[modify] https://crrev.com/922f06de958dcf28211127747145f1ce9e0413e5/chrome/browser/chromeos/child_accounts/usage_time_limit_processor_unittest.cc
[modify] https://crrev.com/922f06de958dcf28211127747145f1ce9e0413e5/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos.cc
[modify] https://crrev.com/922f06de958dcf28211127747145f1ce9e0413e5/chrome/browser/chromeos/policy/device_status_collector.cc
[modify] https://crrev.com/922f06de958dcf28211127747145f1ce9e0413e5/chrome/browser/chromeos/policy/device_status_collector.h
[modify] https://crrev.com/922f06de958dcf28211127747145f1ce9e0413e5/chrome/browser/chromeos/policy/device_status_collector_browsertest.cc
[modify] https://crrev.com/922f06de958dcf28211127747145f1ce9e0413e5/chrome/browser/chromeos/policy/status_uploader_unittest.cc

Status: Assigned (was: Fixed)
This CL appears to have damaged the R69 branch build. 

...
chromeos-chrome-69.0.3497.74_rc-r1: FAILED: obj/chrome/browser/chromeos/chromeos/usage_time_limit_processor.o 
chromeos-chrome-69.0.3497.74_rc-r1: /home/chrome-bot/goma/gomacc x86_64-cros-linux-gnu-clang++ -B/usr/x86_64-pc-linux-gnu/x86_64-cros-linux-gnu/binutils-bin/2.27.0-gold -MMD -MF obj/chrome/browser/chromeos/chromeos/usage_time_limit_processor.o.d -DUSE_CRAS -DV8_DEPRECATION_WARNINGS -DUSE_UDEV -DUSE_AURA=1 -DUSE_NSS_CERTS=1 -DUSE_OZONE=1 -DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD -DSAFE_BROWSING_DB_LOCAL -DOFFICIAL_BUILD -DGOOGLE_CHROME_BUILD -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -DNO_UNWIND_TABLES -DCR_CLANG_REVISION=\"337439-1\" -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D_FORTIFY_SOURCE=2 -DCR_SYSROOT_HASH=4e7db513b0faeea8fb410f70c9909e8736f5c0ab -DOS_CHROMEOS -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DWEBP_EXTERN=extern -DUSE_EGL -DTOOLKIT_VIEWS=1 -DEXPAT_RELATIVE_PATH -DGOOGLE_PROTOBUF_NO_RTTI -DGOOGLE_PROTOBUF_NO_STATIC_INITIALIZER -DHAVE_PTHREAD -DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0 -DU_STATIC_IMPLEMENTATION -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -DUCHAR_TYPE=uint16_t -DSK_IGNORE_LINEONLY_AA_CONVEX_PATH_OPTS -DSK_HAS_PNG_LIBRARY -DSK_HAS_WEBP_LIBRARY -DSK_HAS_JPEG_LIBRARY -DSK_SUPPORT_GPU=1 -DSK_GPU_WORKAROUNDS_HEADER=\"gpu/config/gpu_driver_bug_workaround_autogen.h\" -DLEVELDB_PLATFORM_CHROMIUM=1 -DMESA_EGL_NO_X11_HEADERS -DRLZ_NETWORK_IMPLEMENTATION_CHROME_NET -DV8_USE_EXTERNAL_STARTUP_DATA -DWEBRTC_NON_STATIC_TRACE_EVENT_HANDLERS=0 -DGTEST_RELATIVE_PATH -DWEBRTC_CHROMIUM_BUILD -DWEBRTC_POSIX -DWEBRTC_LINUX -DCHROMEOS -DABSL_ALLOCATOR_NOTHROW=1 -DNO_MAIN_THREAD_WRAPPING -I../../../../../../../home/chrome-bot/chrome_root/src -Igen -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/libwebp/src -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/khronos -I../../../../../../../home/chrome-bot/chrome_root/src/gpu -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/libyuv/include -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/webrtc_overrides -I../../../../../../../home/chrome-bot/chrome_root/src/testing/gtest/include -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/libyuv/include -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/usrsctp/usrsctplib -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/webrtc -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/protobuf/src -Igen/protoc_out -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/protobuf/src -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/ced/src -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/icu/source/common -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/icu/source/i18n -I../../../../../../../home/chrome-bot/chrome_root/src/skia/config -I../../../../../../../home/chrome-bot/chrome_root/src/skia/ext -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/skia/include/c -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/skia/include/config -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/skia/include/core -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/skia/include/effects -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/skia/include/encode -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/skia/include/gpu -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/skia/include/images -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/skia/include/lazy -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/skia/include/pathops -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/skia/include/pdf -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/skia/include/pipe -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/skia/include/ports -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/skia/include/utils -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/skia/src/gpu -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/skia/src/sksl -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/libwebm/source -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/leveldatabase -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/leveldatabase/src -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/leveldatabase/src/include -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/mesa/src/include -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/boringssl/src/include -Igen/third_party/metrics_proto -Igen -Igen -I../../../../../../../home/chrome-bot/chrome_root/src/v8/include -Igen/v8/include -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/webrtc_overrides -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/webrtc -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/perfetto/include -Igen/third_party/perfetto/protos -Igen/third_party/perfetto/protos -Igen/third_party/perfetto/protos -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/re2/src -Igen -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/cacheinvalidation/overrides -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/cacheinvalidation/src -Igen -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/flatbuffers/src/include -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/webrtc_overrides -I../../../../../../../home/chrome-bot/chrome_root/src/testing/gtest/include -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/webrtc -I../../../../../../../home/chrome-bot/chrome_root/src/third_party/zlib -fno-strict-aliasing -fno-unwind-tables -fno-asynchronous-unwind-tables -fPIC -pthread -fcolor-diagnostics -fmerge-all-constants -Xclang -fdebug-compilation-dir -Xclang . -no-canonical-prefixes -m64 -march=x86-64 -Wall -Wextra -Wimplicit-fallthrough -Wthread-safety -Wno-missing-field-initializers -Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default -Wno-unneeded-internal-declaration -Wno-undefined-var-template -Wno-address-of-packed-member -Wno-nonportable-include-path -Wno-user-defined-warnings -Wno-unused-lambda-capture -Wno-null-pointer-arithmetic -Wno-enum-compare-switch -Wno-ignored-pragma-optimize -O2 -fno-ident -fdata-sections -ffunction-sections -fno-omit-frame-pointer -g2 -ggnu-pubnames -fvisibility=hidden -Wheader-hygiene -Wstring-conversion -Wtautological-overlap-compare -Wexit-time-destructors -Wno-header-guard -isystem../../../../../../../build/samus/usr/include/nss -isystem../../../../../../../build/samus/usr/include/nspr -isystem ../../../../../../../home/chrome-bot/chrome_root/src/third_party/abseil-cpp -isystem../../../../../../../build/samus/usr/include/dbus-1.0 -isystem../../../../../../../build/samus/usr/lib64/dbus-1.0/include -Wno-exit-time-destructors -std=gnu++14 -fno-exceptions -fno-rtti --sysroot=../../../../../../../build/samus -fvisibility-inlines-hidden -pipe -pipe -pipe -march=corei7 -fdebug-info-for-profiling -D__google_stl_debug_vector=1 -Wno-unknown-warning-option -stdlib=libc++  -c ../../../../../../../home/chrome-bot/chrome_root/src/chrome/browser/chromeos/child_accounts/usage_time_limit_processor.cc -o obj/chrome/browser/chromeos/chromeos/usage_time_limit_processor.o
chromeos-chrome-69.0.3497.74_rc-r1: ../../../../../../../home/chrome-bot/chrome_root/src/chrome/browser/chromeos/child_accounts/usage_time_limit_processor.cc:756:33: error: use of undeclared identifier 'time_usage_limit_'; did you mean 'time_usage_limit'?
chromeos-chrome-69.0.3497.74_rc-r1:   return GetUsageLimitResetTime(time_usage_limit_);
chromeos-chrome-69.0.3497.74_rc-r1:                                 ^~~~~~~~~~~~~~~~~
chromeos-chrome-69.0.3497.74_rc-r1:                                 time_usage_limit
chromeos-chrome-69.0.3497.74_rc-r1: ../../../../../../../home/chrome-bot/chrome_root/src/chrome/browser/chromeos/child_accounts/usage_time_limit_processor.cc:173:44: note: 'time_usage_limit' declared here
chromeos-chrome-69.0.3497.74_rc-r1:   base::Optional<internal::TimeUsageLimit> time_usage_limit;
chromeos-chrome-69.0.3497.74_rc-r1:                                            ^
chromeos-chrome-69.0.3497.74_rc-r1: 1 error generated.
chromeos-chrome-69.0.3497.74_rc-r1: 
...

Labels: -Pri-1 Pri-0
I synced the branch. Since the revert is not submitted yet, I am trying a build fix on a branch. I am good to either do revert and reland or fix on a branch. 
Posted on the branch one-liner fix here: https://chromium-review.googlesource.com/c/chromium/src/+/1197424
Project Member

Comment 18 by bugdroid1@chromium.org, Aug 30

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

commit 1517ccc3851dc9e0842cc2c04abc278a97c3f281
Author: Aga Wronska <agawronska@chromium.org>
Date: Thu Aug 30 23:08:18 2018

Fix build breakage.

Caused by:
https://chromium.googlesource.com/chromium/src/+/922f06de958dcf28211127747145f1ce9e0413e5

Bug:  877178 
Test: Build branch manually and run tests.

Change-Id: I9b253a474484391739e49f14a259a95cee543ea0
Reviewed-on: https://chromium-review.googlesource.com/1197424
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#854}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/1517ccc3851dc9e0842cc2c04abc278a97c3f281/chrome/browser/chromeos/child_accounts/usage_time_limit_processor.cc

Status: Fixed (was: Assigned)

Sign in to add a comment