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

Issue 852860 link

Starred by 4 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Add new policy: Update Time Restrictions

Project Member Reported by adokar@google.com, Jun 14 2018

Issue description

Implement update time restrictions device policy to give admins the ability to set times of the day where devices are not allowed to update.
Design Doc: go/cros-update-time-restrictions-dd


 
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/update_engine/+/9614350fdecd73ded555d6ea06097c70416575af

commit 9614350fdecd73ded555d6ea06097c70416575af
Author: Adolfo Victoria <adokar@google.com>
Date: Sat Jun 16 05:17:14 2018

update_engine: Add minutes variable to real time provider.

Make it so that the user can also get the current
minutes in local time from the real time provider.

BUG=chromium:852860
TEST=Real time provider unittests
Change-Id: I8118e28c5ab7f000434947fdee9aeae441535ba6
Reviewed-on: https://chromium-review.googlesource.com/1101468
Commit-Ready: Adolfo Higueros <adokar@google.com>
Tested-by: Adolfo Higueros <adokar@google.com>
Reviewed-by: Amin Hassani <ahassani@chromium.org>

[modify] https://crrev.com/9614350fdecd73ded555d6ea06097c70416575af/update_manager/real_time_provider_unittest.cc
[modify] https://crrev.com/9614350fdecd73ded555d6ea06097c70416575af/update_manager/real_time_provider.cc
[modify] https://crrev.com/9614350fdecd73ded555d6ea06097c70416575af/update_manager/time_provider.h
[modify] https://crrev.com/9614350fdecd73ded555d6ea06097c70416575af/update_manager/fake_time_provider.h
[modify] https://crrev.com/9614350fdecd73ded555d6ea06097c70416575af/update_manager/real_time_provider.h

Project Member

Comment 2 by bugdroid1@chromium.org, Jun 19 2018

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

commit d2f88d6fa94ba01da58ffa4aecb0f6bc256f6fe9
Author: Adolfo Victoria <adokar@google.com>
Date: Tue Jun 19 02:30:18 2018

Add new policy: Update Time Restrictions

Allows administrators to set time intervals in which the managed
devices should not check for updates. Unless the updates are forced.

BUG=chromium:852860
TEST=Tested locally and used unit tests
CQ-DEPEND=CL:1097698

Change-Id: Ibd9ff26fb537a966f86d9a824845fa4cf7f92d8e
Reviewed-on: https://chromium-review.googlesource.com/1097990
Commit-Queue: Adolfo Higueros <adokar@google.com>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568308}
[modify] https://crrev.com/d2f88d6fa94ba01da58ffa4aecb0f6bc256f6fe9/chrome/browser/chromeos/BUILD.gn
[add] https://crrev.com/d2f88d6fa94ba01da58ffa4aecb0f6bc256f6fe9/chrome/browser/chromeos/policy/device_autoupdate_time_restrictions_proto_parser.cc
[add] https://crrev.com/d2f88d6fa94ba01da58ffa4aecb0f6bc256f6fe9/chrome/browser/chromeos/policy/device_autoupdate_time_restrictions_proto_parser.h
[add] https://crrev.com/d2f88d6fa94ba01da58ffa4aecb0f6bc256f6fe9/chrome/browser/chromeos/policy/device_autoupdate_time_restrictions_proto_parser_unittest.cc
[modify] https://crrev.com/d2f88d6fa94ba01da58ffa4aecb0f6bc256f6fe9/chrome/browser/chromeos/policy/device_policy_decoder_chromeos.cc
[modify] https://crrev.com/d2f88d6fa94ba01da58ffa4aecb0f6bc256f6fe9/chrome/test/data/policy/policy_test_cases.json
[modify] https://crrev.com/d2f88d6fa94ba01da58ffa4aecb0f6bc256f6fe9/components/policy/proto/chrome_device_policy.proto
[modify] https://crrev.com/d2f88d6fa94ba01da58ffa4aecb0f6bc256f6fe9/components/policy/resources/policy_templates.json
[modify] https://crrev.com/d2f88d6fa94ba01da58ffa4aecb0f6bc256f6fe9/tools/metrics/histograms/enums.xml

Project Member

Comment 3 by bugdroid1@chromium.org, Jun 27 2018

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

commit 6ca6c93c28c1cebb7bb68e81542b290a853e0eb7
Author: Adolfo Victoria <adokar@google.com>
Date: Wed Jun 27 23:17:00 2018

Change DeviceAutoUpdateTimeRestrictions to JSON

Refactor DeviceAutoUpdateTimeRestrictions to use a JSON string instead
of a proto.

BUG=chromium:852860
TEST=None

Change-Id: Ic5174ad705b3cf59dfbbfc5713c231bf7f057a0f
Reviewed-on: https://chromium-review.googlesource.com/1109264
Reviewed-by: Lutz Justen <ljusten@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Commit-Queue: Maksim Ivanov <emaxx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570928}
[modify] https://crrev.com/6ca6c93c28c1cebb7bb68e81542b290a853e0eb7/chrome/browser/chromeos/BUILD.gn
[delete] https://crrev.com/d42e00a2b908b813aa98b35a0ceef685f3b316cf/chrome/browser/chromeos/policy/device_autoupdate_time_restrictions_proto_parser.cc
[delete] https://crrev.com/d42e00a2b908b813aa98b35a0ceef685f3b316cf/chrome/browser/chromeos/policy/device_autoupdate_time_restrictions_proto_parser.h
[delete] https://crrev.com/d42e00a2b908b813aa98b35a0ceef685f3b316cf/chrome/browser/chromeos/policy/device_autoupdate_time_restrictions_proto_parser_unittest.cc
[modify] https://crrev.com/6ca6c93c28c1cebb7bb68e81542b290a853e0eb7/chrome/browser/chromeos/policy/device_policy_decoder_chromeos.cc
[modify] https://crrev.com/6ca6c93c28c1cebb7bb68e81542b290a853e0eb7/chrome/test/data/policy/policy_test_cases.json
[modify] https://crrev.com/6ca6c93c28c1cebb7bb68e81542b290a853e0eb7/components/policy/proto/chrome_device_policy.proto
[modify] https://crrev.com/6ca6c93c28c1cebb7bb68e81542b290a853e0eb7/components/policy/resources/policy_templates.json

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 6

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

commit d4b210aab084622bd4bf0f6e6814b3577b5e2aea
Author: Adolfo Victoria <adokar@google.com>
Date: Fri Jul 06 18:22:06 2018

Add conversions for disallowed intervals from ListValue

Add parsers to convert Value types into WeeklyTimeIntervals. Will be
used to decode DeviceAutoUpdateTimeRestrictions from its JSON string.

BUG=chromium:852860
TEST=unit tests

Change-Id: I2d1151c0e36d52b070e05a20852b8ec3f9603505
Reviewed-on: https://chromium-review.googlesource.com/1121626
Commit-Queue: Adolfo Higueros <adokar@google.com>
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573018}
[modify] https://crrev.com/d4b210aab084622bd4bf0f6e6814b3577b5e2aea/chrome/browser/chromeos/BUILD.gn
[add] https://crrev.com/d4b210aab084622bd4bf0f6e6814b3577b5e2aea/chrome/browser/chromeos/policy/device_auto_update_time_restrictions_decoder.cc
[add] https://crrev.com/d4b210aab084622bd4bf0f6e6814b3577b5e2aea/chrome/browser/chromeos/policy/device_auto_update_time_restrictions_decoder.h
[add] https://crrev.com/d4b210aab084622bd4bf0f6e6814b3577b5e2aea/chrome/browser/chromeos/policy/device_auto_update_time_restrictions_decoder_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Jul 10

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

commit bf19c5f1afefe650b93c7173aba5bc7677f0a28f
Author: Adolfo Victoria <adokar@google.com>
Date: Tue Jul 10 18:11:43 2018

Add DeviceAutoUpdateTimeRestrictions to device_settings_provider

Add DeviceAutoUpdateTimeRestrictions policy to the device settings
provider so that the policy can be accessed by about handler to
notify users when autoupdates are disabled (CL:1130503).

BUG=chromium:852860
TEST=unit_tests

Change-Id: I7d72973220041e98b5a2c3af978cbcd07728ba88
Reviewed-on: https://chromium-review.googlesource.com/1128440
Commit-Queue: Adolfo Higueros <adokar@google.com>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Lutz Justen <ljusten@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573811}
[modify] https://crrev.com/bf19c5f1afefe650b93c7173aba5bc7677f0a28f/chrome/browser/chromeos/settings/device_settings_provider.cc
[modify] https://crrev.com/bf19c5f1afefe650b93c7173aba5bc7677f0a28f/chrome/browser/chromeos/settings/device_settings_provider_unittest.cc
[modify] https://crrev.com/bf19c5f1afefe650b93c7173aba5bc7677f0a28f/chromeos/settings/cros_settings_names.cc
[modify] https://crrev.com/bf19c5f1afefe650b93c7173aba5bc7677f0a28f/chromeos/settings/cros_settings_names.h

Project Member

Comment 6 by bugdroid1@chromium.org, Jul 11

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

commit 38395431788594260f54df2a30634027050914b7
Author: Adolfo Victoria <adokar@google.com>
Date: Wed Jul 11 17:14:19 2018

Extend Weekly Time with ToLocalizedString function

Add ToLocalizedString() function to WeeklyTime for later
use in the UI for DeviceAutoUpdateTimeRestrictions.

BUG=chromium:852860
TEST=unit tests

Change-Id: I85db9b19a3f1a1b589c26c064c221fe8e2137123
Reviewed-on: https://chromium-review.googlesource.com/1119392
Commit-Queue: Adolfo Higueros <adokar@google.com>
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Reviewed-by: Sergey Poromov <poromov@chromium.org>
Reviewed-by: May Lippert <maybelle@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574207}
[modify] https://crrev.com/38395431788594260f54df2a30634027050914b7/chrome/browser/chromeos/policy/weekly_time/weekly_time.cc
[modify] https://crrev.com/38395431788594260f54df2a30634027050914b7/chrome/browser/chromeos/policy/weekly_time/weekly_time.h
[modify] https://crrev.com/38395431788594260f54df2a30634027050914b7/chrome/browser/chromeos/policy/weekly_time/weekly_time_unittest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Jul 18

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/9327dc12559221e3d1c76305f90ceec7dbaca39e

commit 9327dc12559221e3d1c76305f90ceec7dbaca39e
Author: Adolfo Victoria <adokar@google.com>
Date: Wed Jul 18 17:26:30 2018

authpolicy: Update encoding for DeviceAutoUpdateTimeRestrictions

Update device_policy_encoder so that it accounts for the change from
proto to string of the DeviceAutoUpdateTimeRestrictions policy.

Ignores DeviceUpdateStagingPercentOfFleetPerWeek for now since it's a
repeated int field, which we don't support so far, and it's going to be
changed to a string field, anyway. It needs to be handled in the next
uprev, though.

CQ-DEPEND=CL:1136538

BUG=chromium:852860
TEST=cros_workon_make authpolicy --test

Change-Id: Ie45b76e57212f86eb5f79b88c538e44628d2ce3b
Reviewed-on: https://chromium-review.googlesource.com/1135897
Commit-Ready: Lutz Justen <ljusten@chromium.org>
Tested-by: Lutz Justen <ljusten@chromium.org>
Reviewed-by: Lutz Justen <ljusten@chromium.org>

[modify] https://crrev.com/9327dc12559221e3d1c76305f90ceec7dbaca39e/authpolicy/policy/device_policy_encoder.cc
[modify] https://crrev.com/9327dc12559221e3d1c76305f90ceec7dbaca39e/authpolicy/policy/device_policy_encoder_unittest.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Jul 19

The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/external/libbrillo/+/2d3967d969318f3041c766f377eed2ef88a613af

commit 2d3967d969318f3041c766f377eed2ef88a613af
Author: Adolfo Victoria <adokar@google.com>
Date: Thu Jul 19 01:15:27 2018

libbrillo: policy: Add new update time restrictions policy

Add a new device policy from chrome_device_policy.proto:
DeviceAutoUpdateTimeRestrictions

BUG=chromium:852860
TEST=libpolicy unit tests
CQ-DEPEND=CL:1136538
Change-Id: Iaef8791f683af00668dcd4041282758cc26b8fbf
Reviewed-on: https://chromium-review.googlesource.com/1101707
Commit-Ready: Adolfo Higueros <adokar@google.com>
Tested-by: Adolfo Higueros <adokar@google.com>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/2d3967d969318f3041c766f377eed2ef88a613af/policy/tests/whitelist/policy_all
[modify] https://crrev.com/2d3967d969318f3041c766f377eed2ef88a613af/policy/tests/libpolicy_unittest.cc
[modify] https://crrev.com/2d3967d969318f3041c766f377eed2ef88a613af/policy/mock_device_policy.h
[modify] https://crrev.com/2d3967d969318f3041c766f377eed2ef88a613af/policy/device_policy_impl.h
[modify] https://crrev.com/2d3967d969318f3041c766f377eed2ef88a613af/policy/device_policy.h
[modify] https://crrev.com/2d3967d969318f3041c766f377eed2ef88a613af/policy/device_policy_impl.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Jul 25

The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/update_engine/+/94ffe135dd1956d5abf3bdf41503d3d32c15be3b

commit 94ffe135dd1956d5abf3bdf41503d3d32c15be3b
Author: Adolfo Victoria <adokar@google.com>
Date: Wed Jul 25 07:14:53 2018

update_engine: Add Update Time Restrictions

Implementation for the update time restrictions policy where the admin
is allowed to choose time interval in which updates will not be checked.

- Adds WeeklyTime and WeeklyTimeInterval classes to update_engine to be
able to easily do interval range checking and time operations easily in
the policy. Added the wiring so the classes can be used as Values and
BoxedValues.
- Adds disallowed_intervals member to device_policy_provider, since this
contains the policy dictated disallowed intervals. The intervals are
obtained from libpolicy, a function was added to convert them to the
new WeeklyTime classes. Added the corresponding changes to the device
policy provider header and interface.
- Added the policy to chromeos_policy.cc so that it's taken into account
in UpdateCheckAllowed.

BUG=chromium:852860
TEST=cros_workon_make update_engine --test

Change-Id: If62a2b3650adf149ba87790345e1eb62f0e1bb1f
Reviewed-on: https://chromium-review.googlesource.com/1119397
Commit-Ready: Adolfo Higueros <adokar@google.com>
Tested-by: Adolfo Higueros <adokar@google.com>
Reviewed-by: Amin Hassani <ahassani@chromium.org>

[modify] https://crrev.com/94ffe135dd1956d5abf3bdf41503d3d32c15be3b/update_manager/policy_test_utils.h
[modify] https://crrev.com/94ffe135dd1956d5abf3bdf41503d3d32c15be3b/update_manager/device_policy_provider.h
[modify] https://crrev.com/94ffe135dd1956d5abf3bdf41503d3d32c15be3b/update_manager/chromeos_policy.h
[add] https://crrev.com/94ffe135dd1956d5abf3bdf41503d3d32c15be3b/update_manager/update_time_restrictions_policy_impl_unittest.cc
[modify] https://crrev.com/94ffe135dd1956d5abf3bdf41503d3d32c15be3b/update_manager/chromeos_policy.cc
[add] https://crrev.com/94ffe135dd1956d5abf3bdf41503d3d32c15be3b/update_manager/update_time_restrictions_policy_impl.cc
[modify] https://crrev.com/94ffe135dd1956d5abf3bdf41503d3d32c15be3b/update_manager/real_device_policy_provider.h
[add] https://crrev.com/94ffe135dd1956d5abf3bdf41503d3d32c15be3b/update_manager/update_time_restrictions_policy_impl.h
[add] https://crrev.com/94ffe135dd1956d5abf3bdf41503d3d32c15be3b/update_manager/weekly_time_unittest.cc
[add] https://crrev.com/94ffe135dd1956d5abf3bdf41503d3d32c15be3b/update_manager/weekly_time.h
[modify] https://crrev.com/94ffe135dd1956d5abf3bdf41503d3d32c15be3b/update_manager/policy_test_utils.cc
[modify] https://crrev.com/94ffe135dd1956d5abf3bdf41503d3d32c15be3b/Android.mk
[modify] https://crrev.com/94ffe135dd1956d5abf3bdf41503d3d32c15be3b/update_manager/real_device_policy_provider_unittest.cc
[add] https://crrev.com/94ffe135dd1956d5abf3bdf41503d3d32c15be3b/update_manager/weekly_time.cc
[modify] https://crrev.com/94ffe135dd1956d5abf3bdf41503d3d32c15be3b/update_manager/real_device_policy_provider.cc
[modify] https://crrev.com/94ffe135dd1956d5abf3bdf41503d3d32c15be3b/update_manager/fake_device_policy_provider.h
[modify] https://crrev.com/94ffe135dd1956d5abf3bdf41503d3d32c15be3b/update_manager/boxed_value_unittest.cc
[modify] https://crrev.com/94ffe135dd1956d5abf3bdf41503d3d32c15be3b/update_manager/chromeos_policy_unittest.cc
[modify] https://crrev.com/94ffe135dd1956d5abf3bdf41503d3d32c15be3b/update_engine.gyp
[modify] https://crrev.com/94ffe135dd1956d5abf3bdf41503d3d32c15be3b/update_manager/boxed_value.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Jul 26

The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/update_engine/+/0dbf1f9b946a11a76116322d5603d0f11a206dc5

commit 0dbf1f9b946a11a76116322d5603d0f11a206dc5
Author: Adolfo Victoria <adokar@google.com>
Date: Thu Jul 26 02:45:34 2018

update_engine: Modify Update Time Restrictions to block downloads

This makes Update Time Restrictions policy block downloads rather than
checks. This will allow for better capturing of metrics. Furthermore,
this change makes Update Time Restrictions only apply for kiosks, per
the discussion with security and PM.
The following changes are made:
* Use UpdateCanBeApplied rather than UpdateCheckAllowed in
UpdateTimeRestrictionsPolicyImpl.
* ChromeOSPolicy::UpdateCanBeApplied now checks for forced updates.
Modify the corresponding classes to implement UpdateCanBeApplied.
* Add the auto_launched_kiosk_app_id variable from libbrillo to the
policy provider, this variable lets us know if the device is in
kiosk-mode.
* If the device is not in kiosk mode, this policy won't be used.
* Change chromeos_policy to check for policies in UpdateCanBeApplied.
* Add unit tests accounting for the new changes.

BUG=chromium:852860
TEST=cros_workon_make update_engine --test

Change-Id: Iba52fa0fd1f89cc55e5009776036f8949e01e3a0
Reviewed-on: https://chromium-review.googlesource.com/1144435
Commit-Ready: Adolfo Higueros <adokar@google.com>
Tested-by: Adolfo Higueros <adokar@google.com>
Reviewed-by: Amin Hassani <ahassani@chromium.org>

[modify] https://crrev.com/0dbf1f9b946a11a76116322d5603d0f11a206dc5/update_manager/update_time_restrictions_policy_impl_unittest.cc
[modify] https://crrev.com/0dbf1f9b946a11a76116322d5603d0f11a206dc5/update_manager/device_policy_provider.h
[modify] https://crrev.com/0dbf1f9b946a11a76116322d5603d0f11a206dc5/update_manager/chromeos_policy.cc
[modify] https://crrev.com/0dbf1f9b946a11a76116322d5603d0f11a206dc5/update_manager/update_time_restrictions_policy_impl.cc
[modify] https://crrev.com/0dbf1f9b946a11a76116322d5603d0f11a206dc5/update_manager/update_time_restrictions_policy_impl.h
[modify] https://crrev.com/0dbf1f9b946a11a76116322d5603d0f11a206dc5/update_manager/interactive_update_policy_impl.cc
[modify] https://crrev.com/0dbf1f9b946a11a76116322d5603d0f11a206dc5/update_manager/interactive_update_policy_impl.h
[modify] https://crrev.com/0dbf1f9b946a11a76116322d5603d0f11a206dc5/update_manager/fake_device_policy_provider.h
[modify] https://crrev.com/0dbf1f9b946a11a76116322d5603d0f11a206dc5/update_manager/real_device_policy_provider_unittest.cc
[modify] https://crrev.com/0dbf1f9b946a11a76116322d5603d0f11a206dc5/update_manager/real_device_policy_provider.cc
[modify] https://crrev.com/0dbf1f9b946a11a76116322d5603d0f11a206dc5/update_manager/chromeos_policy_unittest.cc
[modify] https://crrev.com/0dbf1f9b946a11a76116322d5603d0f11a206dc5/update_manager/real_device_policy_provider.h

Cc: shubhar@chromium.org
 Issue 832750  has been merged into this issue.
Project Member

Comment 12 by bugdroid1@chromium.org, Jul 31

The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/update_engine/+/7a7a08251e2e177d17b899a99e7a3ae2592be7e6

commit 7a7a08251e2e177d17b899a99e7a3ae2592be7e6
Author: Adolfo Victoria <adokar@google.com>
Date: Tue Jul 31 06:08:20 2018

update_engine: Change time restrictions to return kSucceeded

Change UpdateTimeRestrictionsPolicyImpl to return kSucceeded
in place of kAskMeAgainLater. Since the former is not an expected
return value.

Background:
Policies in UpdateCanBeApplied should return kSucceeded when
they want to force a decision, since UpdateCanBeApplied is
supposed to only contain sync policies. kAskMeAgainLater is
reserved for async policies.

BUG=chromium:852860
TEST=cros_workon_make update_engine --test and local tests

Change-Id: Ie81a3d09f98ad7aba3c36183873bc153a6b530cf
Reviewed-on: https://chromium-review.googlesource.com/1153624
Commit-Ready: Adolfo Higueros <adokar@google.com>
Tested-by: Adolfo Higueros <adokar@google.com>
Reviewed-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: May Lippert <maybelle@chromium.org>

[modify] https://crrev.com/7a7a08251e2e177d17b899a99e7a3ae2592be7e6/update_manager/update_time_restrictions_policy_impl.cc
[modify] https://crrev.com/7a7a08251e2e177d17b899a99e7a3ae2592be7e6/update_manager/chromeos_policy_unittest.cc
[modify] https://crrev.com/7a7a08251e2e177d17b899a99e7a3ae2592be7e6/update_manager/update_time_restrictions_policy_impl.h
[modify] https://crrev.com/7a7a08251e2e177d17b899a99e7a3ae2592be7e6/update_manager/update_time_restrictions_policy_impl_unittest.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Aug 8

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

commit 164c3c53e87548981aa603bf007c0d6c50f0d77a
Author: Adolfo Victoria <adokar@google.com>
Date: Wed Aug 08 00:55:53 2018

Refactor WeeklyTime to be explicit with timezones

* Add timezone_offset_ member variable to WeeklyTime class, this
will help the class users know which timezone the class is to
be interpreted in.
* Add ConvertToTimezone and ConvertToCustomTimezone functions to change
between well defined timezones and also to set the timezone of a time
agnostic WeklyTime.
* Move ToLocalizedString to time_utils since its a more specialized
use case.
* Add unit tests for timezone related functions to time utils.

BUG=chromium:852860
TEST=unit_tests

Change-Id: Ifef1b17471c30fb7443727323d00523193327ef1
Reviewed-on: https://chromium-review.googlesource.com/1155786
Commit-Queue: Adolfo Higueros <adokar@google.com>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581420}
[modify] https://crrev.com/164c3c53e87548981aa603bf007c0d6c50f0d77a/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/164c3c53e87548981aa603bf007c0d6c50f0d77a/chrome/browser/chromeos/policy/device_auto_update_time_restrictions_decoder.cc
[modify] https://crrev.com/164c3c53e87548981aa603bf007c0d6c50f0d77a/chrome/browser/chromeos/policy/off_hours/device_off_hours_controller.cc
[modify] https://crrev.com/164c3c53e87548981aa603bf007c0d6c50f0d77a/chrome/browser/chromeos/policy/off_hours/device_off_hours_controller_unittest.cc
[modify] https://crrev.com/164c3c53e87548981aa603bf007c0d6c50f0d77a/chrome/browser/chromeos/policy/off_hours/off_hours_proto_parser.cc
[modify] https://crrev.com/164c3c53e87548981aa603bf007c0d6c50f0d77a/chrome/browser/chromeos/policy/off_hours/off_hours_proto_parser.h
[modify] https://crrev.com/164c3c53e87548981aa603bf007c0d6c50f0d77a/chrome/browser/chromeos/policy/off_hours/off_hours_proto_parser_unittest.cc
[modify] https://crrev.com/164c3c53e87548981aa603bf007c0d6c50f0d77a/chrome/browser/chromeos/policy/weekly_time/time_utils.cc
[modify] https://crrev.com/164c3c53e87548981aa603bf007c0d6c50f0d77a/chrome/browser/chromeos/policy/weekly_time/time_utils.h
[add] https://crrev.com/164c3c53e87548981aa603bf007c0d6c50f0d77a/chrome/browser/chromeos/policy/weekly_time/time_utils_unittest.cc
[modify] https://crrev.com/164c3c53e87548981aa603bf007c0d6c50f0d77a/chrome/browser/chromeos/policy/weekly_time/weekly_time.cc
[modify] https://crrev.com/164c3c53e87548981aa603bf007c0d6c50f0d77a/chrome/browser/chromeos/policy/weekly_time/weekly_time.h
[modify] https://crrev.com/164c3c53e87548981aa603bf007c0d6c50f0d77a/chrome/browser/chromeos/policy/weekly_time/weekly_time_interval.cc
[modify] https://crrev.com/164c3c53e87548981aa603bf007c0d6c50f0d77a/chrome/browser/chromeos/policy/weekly_time/weekly_time_interval.h
[modify] https://crrev.com/164c3c53e87548981aa603bf007c0d6c50f0d77a/chrome/browser/chromeos/policy/weekly_time/weekly_time_interval_unittest.cc
[modify] https://crrev.com/164c3c53e87548981aa603bf007c0d6c50f0d77a/chrome/browser/chromeos/policy/weekly_time/weekly_time_unittest.cc

Owner: maybelle@chromium.org
Project Member

Comment 15 by bugdroid1@chromium.org, Aug 11

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

commit b3392d988ae23dc52d9b386af47dc6a53a2d22c8
Author: Adolfo Victoria <adokar@google.com>
Date: Sat Aug 11 00:42:00 2018

Add vector of intervals range checking to Time Restrictions policy

* Add function GetEndOfCurrent interval to obtain the last end of the
interval which contains the current time.
* Add function MaterializeIntervalsToLocalTimezone, which materializes
the given intervals to the current local timezone.
* Add function TimeIsInsideCrosSettingsIntervals, which checks if the
current time is inside the intervals set in cros settings. It then
returns the localized string of the end of the interval that contains
the current time.

BUG=chromium:852860
TEST=added unittests

Change-Id: Ife32e31c8a929827fd14ab9f108c49eacb9c32c3
Reviewed-on: https://chromium-review.googlesource.com/1132676
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Commit-Queue: Maksim Ivanov <emaxx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582403}
[modify] https://crrev.com/b3392d988ae23dc52d9b386af47dc6a53a2d22c8/chrome/browser/chromeos/BUILD.gn
[add] https://crrev.com/b3392d988ae23dc52d9b386af47dc6a53a2d22c8/chrome/browser/chromeos/policy/device_auto_update_time_restrictions_utils.cc
[add] https://crrev.com/b3392d988ae23dc52d9b386af47dc6a53a2d22c8/chrome/browser/chromeos/policy/device_auto_update_time_restrictions_utils.h
[add] https://crrev.com/b3392d988ae23dc52d9b386af47dc6a53a2d22c8/chrome/browser/chromeos/policy/device_auto_update_time_restrictions_utils_unittest.cc
[modify] https://crrev.com/b3392d988ae23dc52d9b386af47dc6a53a2d22c8/chrome/browser/chromeos/policy/weekly_time/time_utils.cc
[modify] https://crrev.com/b3392d988ae23dc52d9b386af47dc6a53a2d22c8/chrome/browser/chromeos/policy/weekly_time/time_utils.h
[modify] https://crrev.com/b3392d988ae23dc52d9b386af47dc6a53a2d22c8/chrome/browser/chromeos/policy/weekly_time/time_utils_unittest.cc
[modify] https://crrev.com/b3392d988ae23dc52d9b386af47dc6a53a2d22c8/chrome/browser/chromeos/policy/weekly_time/weekly_time.h
[modify] https://crrev.com/b3392d988ae23dc52d9b386af47dc6a53a2d22c8/chrome/browser/chromeos/policy/weekly_time/weekly_time_interval.h

Project Member

Comment 16 by bugdroid1@chromium.org, Aug 11

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

commit 3a30357690bfd391e2ddab461c6bd48e2c8a5580
Author: Findit <findit-for-me@appspot.gserviceaccount.com>
Date: Sat Aug 11 05:19:12 2018

Revert "Add vector of intervals range checking to Time Restrictions policy"

This reverts commit b3392d988ae23dc52d9b386af47dc6a53a2d22c8.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 582403 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtL2IzMzkyZDk4OGFlMjNkYzUyZDliMzg2YWY0N2RjNmE1M2EyZDIyYzgM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.memory/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/28575

Sample Failed Step: unit_tests

Original change's description:
> Add vector of intervals range checking to Time Restrictions policy
> 
> * Add function GetEndOfCurrent interval to obtain the last end of the
> interval which contains the current time.
> * Add function MaterializeIntervalsToLocalTimezone, which materializes
> the given intervals to the current local timezone.
> * Add function TimeIsInsideCrosSettingsIntervals, which checks if the
> current time is inside the intervals set in cros settings. It then
> returns the localized string of the end of the interval that contains
> the current time.
> 
> BUG=chromium:852860
> TEST=added unittests
> 
> Change-Id: Ife32e31c8a929827fd14ab9f108c49eacb9c32c3
> Reviewed-on: https://chromium-review.googlesource.com/1132676
> Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
> Commit-Queue: Maksim Ivanov <emaxx@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#582403}

No-Presubmit: true
No-Tree-Checks: true
No-Try: true
BUG=chromium:852860

Change-Id: I1663fc559003d938270da4b8fd7e85719b8e9e01
Reviewed-on: https://chromium-review.googlesource.com/1172003
Cr-Commit-Position: refs/heads/master@{#582430}
[modify] https://crrev.com/3a30357690bfd391e2ddab461c6bd48e2c8a5580/chrome/browser/chromeos/BUILD.gn
[delete] https://crrev.com/6df1590559bd965c71b181b4d948e0ea3318189a/chrome/browser/chromeos/policy/device_auto_update_time_restrictions_utils.cc
[delete] https://crrev.com/6df1590559bd965c71b181b4d948e0ea3318189a/chrome/browser/chromeos/policy/device_auto_update_time_restrictions_utils.h
[delete] https://crrev.com/6df1590559bd965c71b181b4d948e0ea3318189a/chrome/browser/chromeos/policy/device_auto_update_time_restrictions_utils_unittest.cc
[modify] https://crrev.com/3a30357690bfd391e2ddab461c6bd48e2c8a5580/chrome/browser/chromeos/policy/weekly_time/time_utils.cc
[modify] https://crrev.com/3a30357690bfd391e2ddab461c6bd48e2c8a5580/chrome/browser/chromeos/policy/weekly_time/time_utils.h
[modify] https://crrev.com/3a30357690bfd391e2ddab461c6bd48e2c8a5580/chrome/browser/chromeos/policy/weekly_time/time_utils_unittest.cc
[modify] https://crrev.com/3a30357690bfd391e2ddab461c6bd48e2c8a5580/chrome/browser/chromeos/policy/weekly_time/weekly_time.h
[modify] https://crrev.com/3a30357690bfd391e2ddab461c6bd48e2c8a5580/chrome/browser/chromeos/policy/weekly_time/weekly_time_interval.h

Project Member

Comment 17 by bugdroid1@chromium.org, Aug 11

The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/update_engine/+/20262ad1d3f8d620b7227fd8d9e13938243ce32f

commit 20262ad1d3f8d620b7227fd8d9e13938243ce32f
Author: Adolfo Victoria <adokar@google.com>
Date: Sat Aug 11 05:44:01 2018

update_engine: Reset update pending callback after update actions

Reset the update pending callback after update actions have been
carried out, this is so that the current forced update status are
available to the policies that are called in UpdateActions.

BUG=chromium:852860
TEST=Tested updating locally

Change-Id: I44a70b408c83c2adea4cd8c386f777c4799a19f5
Reviewed-on: https://chromium-review.googlesource.com/1164527
Commit-Ready: Adolfo Higueros <adokar@google.com>
Tested-by: Adolfo Higueros <adokar@google.com>
Reviewed-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Sen Jiang <senj@chromium.org>

[modify] https://crrev.com/20262ad1d3f8d620b7227fd8d9e13938243ce32f/update_attempter.cc

Project Member

Comment 18 by bugdroid1@chromium.org, Aug 14

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

commit 04902d11c614f8d1218d20948e7ee5dd1c548f21
Author: Pavol Marko <pmarko@chromium.org>
Date: Tue Aug 14 12:36:08 2018

[Reland] Add vector of intervals range checking to Time Restrictions policy

* Add function GetEndOfCurrent interval to obtain the last end of the
interval which contains the current time.
* Add function MaterializeIntervalsToLocalTimezone, which materializes
the given intervals to the current local timezone.
* Add function TimeIsInsideCrosSettingsIntervals, which checks if the
current time is inside the intervals set in cros settings. It then
returns the localized string of the end of the interval that contains
the current time.

This is a reland of CL:1132676 which fixes memory leaks that lead to a
revert (revert was CL:1172003).

BUG=chromium:852860
TEST=added unittests

Change-Id: I8ad01d4ab90d4e699e4b7377c24e4932513ebe1b
Reviewed-on: https://chromium-review.googlesource.com/1172128
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Commit-Queue: Pavol Marko <pmarko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582892}
[modify] https://crrev.com/04902d11c614f8d1218d20948e7ee5dd1c548f21/chrome/browser/chromeos/BUILD.gn
[add] https://crrev.com/04902d11c614f8d1218d20948e7ee5dd1c548f21/chrome/browser/chromeos/policy/device_auto_update_time_restrictions_utils.cc
[add] https://crrev.com/04902d11c614f8d1218d20948e7ee5dd1c548f21/chrome/browser/chromeos/policy/device_auto_update_time_restrictions_utils.h
[add] https://crrev.com/04902d11c614f8d1218d20948e7ee5dd1c548f21/chrome/browser/chromeos/policy/device_auto_update_time_restrictions_utils_unittest.cc
[modify] https://crrev.com/04902d11c614f8d1218d20948e7ee5dd1c548f21/chrome/browser/chromeos/policy/weekly_time/time_utils.cc
[modify] https://crrev.com/04902d11c614f8d1218d20948e7ee5dd1c548f21/chrome/browser/chromeos/policy/weekly_time/time_utils.h
[modify] https://crrev.com/04902d11c614f8d1218d20948e7ee5dd1c548f21/chrome/browser/chromeos/policy/weekly_time/time_utils_unittest.cc
[modify] https://crrev.com/04902d11c614f8d1218d20948e7ee5dd1c548f21/chrome/browser/chromeos/policy/weekly_time/weekly_time.h
[modify] https://crrev.com/04902d11c614f8d1218d20948e7ee5dd1c548f21/chrome/browser/chromeos/policy/weekly_time/weekly_time_interval.h

Project Member

Comment 19 by bugdroid1@chromium.org, Aug 30

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

commit ab1e95f63731525e5ea3340cddf6bbdea278ed1d
Author: May Lippert <maybelle@chromium.org>
Date: Thu Aug 30 20:45:58 2018

Add histograms for time to update.

This change adds histograms for the total time from when the update is
first seen by the client to when the update is downloaded (but before
the device has rebooted). There are two histograms, one for the total
time in minutes when the client has a time restriction policy for
updates, and one for when the client has no policy in place.

These two histograms are only reported for enterprise devices.

CL where the histogram is used:
https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/1179230


BUG=chromium:852860
TEST=Run through scenarios and checked chrome://histograms

Change-Id: Ib4b8d0126087d5951b79d7f4d2022c2c776744a7
Reviewed-on: https://chromium-review.googlesource.com/1181649
Commit-Queue: May Lippert <maybelle@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587752}
[modify] https://crrev.com/ab1e95f63731525e5ea3340cddf6bbdea278ed1d/tools/metrics/histograms/histograms.xml

Labels: Merge-Request-70
Requesting merge for CL in Comment#19 which is for additional metrics.
Project Member

Comment 21 by sheriffbot@chromium.org, Sep 10

Labels: -Merge-Request-70 Merge-Review-70 Hotlist-Merge-Review
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: M-70
Project Member

Comment 23 by bugdroid1@chromium.org, Sep 17

The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/update_engine/+/60aa3ca7309f45b9b6242d8722a9ebda52017f9d

commit 60aa3ca7309f45b9b6242d8722a9ebda52017f9d
Author: May Lippert <maybelle@chromium.org>
Date: Mon Sep 17 20:17:55 2018

update_engine: Add metrics to duration to apply an update.

Add new metrics that record the time between an update being seen by the client
to the time when the update is actually applied.

This metric will be recorded only for Enterprise enrolled devices.

UpdateEngine.SuccessfulUpdate.TimeRestrictedDurationFromSeenToUpdateMinutes is
recorded when an update is applied while the
DeviceAutoUpdateTimeRestrictions device policy exists.

UpdateEngine.SuccessfulUpdate.DurationFromSeenToUpdateMinutes is
recorded when an update is applied without the policy.

BUG=chromium:852860
TEST=run updater manually and checked chrome://histograms

Change-Id: I4dd54fc4404ef2e66902113617841808c1f9e616
Reviewed-on: https://chromium-review.googlesource.com/1179230
Commit-Ready: May Lippert <maybelle@chromium.org>
Tested-by: May Lippert <maybelle@chromium.org>
Reviewed-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Sen Jiang <senj@chromium.org>

[modify] https://crrev.com/60aa3ca7309f45b9b6242d8722a9ebda52017f9d/mock_metrics_reporter.h
[modify] https://crrev.com/60aa3ca7309f45b9b6242d8722a9ebda52017f9d/update_attempter_unittest.cc
[modify] https://crrev.com/60aa3ca7309f45b9b6242d8722a9ebda52017f9d/omaha_request_action_unittest.cc
[modify] https://crrev.com/60aa3ca7309f45b9b6242d8722a9ebda52017f9d/metrics_reporter_stub.h
[modify] https://crrev.com/60aa3ca7309f45b9b6242d8722a9ebda52017f9d/metrics_reporter_omaha.cc
[modify] https://crrev.com/60aa3ca7309f45b9b6242d8722a9ebda52017f9d/metrics_reporter_omaha_unittest.cc
[modify] https://crrev.com/60aa3ca7309f45b9b6242d8722a9ebda52017f9d/omaha_request_action.cc
[modify] https://crrev.com/60aa3ca7309f45b9b6242d8722a9ebda52017f9d/metrics_reporter_interface.h
[modify] https://crrev.com/60aa3ca7309f45b9b6242d8722a9ebda52017f9d/update_attempter.cc
[modify] https://crrev.com/60aa3ca7309f45b9b6242d8722a9ebda52017f9d/update_attempter.h
[modify] https://crrev.com/60aa3ca7309f45b9b6242d8722a9ebda52017f9d/metrics_reporter_android.h
[modify] https://crrev.com/60aa3ca7309f45b9b6242d8722a9ebda52017f9d/metrics_reporter_omaha.h
[modify] https://crrev.com/60aa3ca7309f45b9b6242d8722a9ebda52017f9d/omaha_request_action.h

Requesting merge for CL in Comment#23 also, which implements the metrics.

Labels: -Merge-Review-70 Merge-Approved-70
Project Member

Comment 26 by bugdroid1@chromium.org, Sep 24

Labels: merge-merged-release-R70-11021.B
The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/update_engine/+/c0c5a397156fbe7ec9cfa86ea8a9af601c30559e

commit c0c5a397156fbe7ec9cfa86ea8a9af601c30559e
Author: May Lippert <maybelle@chromium.org>
Date: Mon Sep 24 21:14:02 2018

update_engine: Add metrics to duration to apply an update.

Add new metrics that record the time between an update being seen by the client
to the time when the update is actually applied.

This metric will be recorded only for Enterprise enrolled devices.

UpdateEngine.SuccessfulUpdate.TimeRestrictedDurationFromSeenToUpdateMinutes is
recorded when an update is applied while the
DeviceAutoUpdateTimeRestrictions device policy exists.

UpdateEngine.SuccessfulUpdate.DurationFromSeenToUpdateMinutes is
recorded when an update is applied without the policy.

BUG=chromium:852860
TEST=run updater manually and checked chrome://histograms

Change-Id: I4dd54fc4404ef2e66902113617841808c1f9e616
Reviewed-on: https://chromium-review.googlesource.com/1179230
Commit-Ready: May Lippert <maybelle@chromium.org>
Tested-by: May Lippert <maybelle@chromium.org>
Reviewed-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Sen Jiang <senj@chromium.org>
(cherry picked from commit 60aa3ca7309f45b9b6242d8722a9ebda52017f9d)

[modify] https://crrev.com/c0c5a397156fbe7ec9cfa86ea8a9af601c30559e/mock_metrics_reporter.h
[modify] https://crrev.com/c0c5a397156fbe7ec9cfa86ea8a9af601c30559e/update_attempter_unittest.cc
[modify] https://crrev.com/c0c5a397156fbe7ec9cfa86ea8a9af601c30559e/omaha_request_action_unittest.cc
[modify] https://crrev.com/c0c5a397156fbe7ec9cfa86ea8a9af601c30559e/metrics_reporter_stub.h
[modify] https://crrev.com/c0c5a397156fbe7ec9cfa86ea8a9af601c30559e/metrics_reporter_omaha.cc
[modify] https://crrev.com/c0c5a397156fbe7ec9cfa86ea8a9af601c30559e/metrics_reporter_omaha_unittest.cc
[modify] https://crrev.com/c0c5a397156fbe7ec9cfa86ea8a9af601c30559e/omaha_request_action.cc
[modify] https://crrev.com/c0c5a397156fbe7ec9cfa86ea8a9af601c30559e/metrics_reporter_interface.h
[modify] https://crrev.com/c0c5a397156fbe7ec9cfa86ea8a9af601c30559e/update_attempter.cc
[modify] https://crrev.com/c0c5a397156fbe7ec9cfa86ea8a9af601c30559e/update_attempter.h
[modify] https://crrev.com/c0c5a397156fbe7ec9cfa86ea8a9af601c30559e/metrics_reporter_android.h
[modify] https://crrev.com/c0c5a397156fbe7ec9cfa86ea8a9af601c30559e/metrics_reporter_omaha.h
[modify] https://crrev.com/c0c5a397156fbe7ec9cfa86ea8a9af601c30559e/omaha_request_action.h

Project Member

Comment 27 by sheriffbot@chromium.org, Sep 28

Cc: geo...@google.com
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Approved-70

Sign in to add a comment