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

Issue 849506 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task
Launch-Accessibility: NA
Launch-Exp-Leadership: NA
Launch-Leadership: NA
Launch-Legal: NA
Launch-Privacy: NA
Launch-Security: NA
Launch-Test: NA
Launch-UI: NA
Rollout-Type: TBD



Sign in to add a comment

Refactor OffHoursInterval code to account for the change to WeeklyTimeInterval

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

Issue description

The proto OffHoursIntervalProto will have a name change to WeeklyTimeIntervalProto per https://docs.google.com/document/d/1vBAdh6LdYkGzcjnpukooefl1LWgiO5Z7BFnnbRfxHEk. The code for OffHoursInterval can also be reused for parsing in the Update Time Restriction policy, it doesn't depend 
on any specifics of the Off Hours Policy so it can be repurposed and moved to a more general place.
 

Comment 1 Deleted

Comment 2 Deleted

Comment 3 Deleted

Project Member

Comment 4 by bugdroid1@chromium.org, Jun 12 2018

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

commit 4a37bfa0551175a4e516c9f0ad4155206bf3482e
Author: Adolfo Victoria <adokar@google.com>
Date: Tue Jun 12 05:01:49 2018

Refactor OffHoursInterval to WeeklyTimeInterval

Change OffHoursIntervalProto to WeeklyTimeIntervalProto so its name
reflects that it's not being used only by DeviceOffHoursProto. The
change also refactors and moves the DeviceOffHours code related to time
utilities since it can be reused when the Update Time Restriction Policy
code is written. The WeeklyTime, WeeklyTimeInterval and time_util code
doesn't require any OffHours specific classes so it can be reused.

BUG= chromium:849506 
TEST= Ran existing OffHours tests after refactor

Change-Id: Idb1d94ee0c0af3e8bf8672a51bc63bb7dc538340
Reviewed-on: https://chromium-review.googlesource.com/1086495
Commit-Queue: Adolfo Higueros <adokar@google.com>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566318}
[modify] https://crrev.com/4a37bfa0551175a4e516c9f0ad4155206bf3482e/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/4a37bfa0551175a4e516c9f0ad4155206bf3482e/chrome/browser/chromeos/policy/off_hours/device_off_hours_controller.cc
[modify] https://crrev.com/4a37bfa0551175a4e516c9f0ad4155206bf3482e/chrome/browser/chromeos/policy/off_hours/device_off_hours_controller.h
[modify] https://crrev.com/4a37bfa0551175a4e516c9f0ad4155206bf3482e/chrome/browser/chromeos/policy/off_hours/device_off_hours_controller_unittest.cc
[delete] https://crrev.com/f443df68be7a0ac090e6297f2286767c709fde31/chrome/browser/chromeos/policy/off_hours/off_hours_interval.cc
[delete] https://crrev.com/f443df68be7a0ac090e6297f2286767c709fde31/chrome/browser/chromeos/policy/off_hours/off_hours_interval.h
[modify] https://crrev.com/4a37bfa0551175a4e516c9f0ad4155206bf3482e/chrome/browser/chromeos/policy/off_hours/off_hours_policy_applier_unittest.cc
[modify] https://crrev.com/4a37bfa0551175a4e516c9f0ad4155206bf3482e/chrome/browser/chromeos/policy/off_hours/off_hours_proto_parser.cc
[modify] https://crrev.com/4a37bfa0551175a4e516c9f0ad4155206bf3482e/chrome/browser/chromeos/policy/off_hours/off_hours_proto_parser.h
[modify] https://crrev.com/4a37bfa0551175a4e516c9f0ad4155206bf3482e/chrome/browser/chromeos/policy/off_hours/off_hours_proto_parser_unittest.cc
[delete] https://crrev.com/f443df68be7a0ac090e6297f2286767c709fde31/chrome/browser/chromeos/policy/off_hours/time_utils.h
[rename] https://crrev.com/4a37bfa0551175a4e516c9f0ad4155206bf3482e/chrome/browser/chromeos/policy/weekly_time/time_utils.cc
[add] https://crrev.com/4a37bfa0551175a4e516c9f0ad4155206bf3482e/chrome/browser/chromeos/policy/weekly_time/time_utils.h
[rename] https://crrev.com/4a37bfa0551175a4e516c9f0ad4155206bf3482e/chrome/browser/chromeos/policy/weekly_time/weekly_time.cc
[rename] https://crrev.com/4a37bfa0551175a4e516c9f0ad4155206bf3482e/chrome/browser/chromeos/policy/weekly_time/weekly_time.h
[add] https://crrev.com/4a37bfa0551175a4e516c9f0ad4155206bf3482e/chrome/browser/chromeos/policy/weekly_time/weekly_time_interval.cc
[add] https://crrev.com/4a37bfa0551175a4e516c9f0ad4155206bf3482e/chrome/browser/chromeos/policy/weekly_time/weekly_time_interval.h
[rename] https://crrev.com/4a37bfa0551175a4e516c9f0ad4155206bf3482e/chrome/browser/chromeos/policy/weekly_time/weekly_time_interval_unittest.cc
[rename] https://crrev.com/4a37bfa0551175a4e516c9f0ad4155206bf3482e/chrome/browser/chromeos/policy/weekly_time/weekly_time_unittest.cc
[modify] https://crrev.com/4a37bfa0551175a4e516c9f0ad4155206bf3482e/components/policy/proto/chrome_device_policy.proto
[modify] https://crrev.com/4a37bfa0551175a4e516c9f0ad4155206bf3482e/components/policy/resources/policy_templates.json

Project Member

Comment 5 by bugdroid1@chromium.org, Jun 13 2018

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

commit fd6e3df036d67c2dda7a515bae1dfc2c0d35d52c
Author: Adolfo Victoria <adokar@google.com>
Date: Wed Jun 13 22:58:01 2018

Refactor WeeklyTime proto parsing to be more general

Move the proto parsing utilities into the WeeklyTime classes to make
them reusable by policies that use the WeeklyTime protos. Added unit
tests for the proto parsing.

BUG= chromium:849506 
TEST=WeeklyTimeInterval and WeeklyTime unit tests

Change-Id: Ic6b327e9b75f6bf9f2c1d1d1a949ed30ec8f18f7
Reviewed-on: https://chromium-review.googlesource.com/1097698
Commit-Queue: Adolfo Higueros <adokar@google.com>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567036}
[modify] https://crrev.com/fd6e3df036d67c2dda7a515bae1dfc2c0d35d52c/chrome/browser/chromeos/policy/off_hours/off_hours_proto_parser.cc
[modify] https://crrev.com/fd6e3df036d67c2dda7a515bae1dfc2c0d35d52c/chrome/browser/chromeos/policy/off_hours/off_hours_proto_parser.h
[modify] https://crrev.com/fd6e3df036d67c2dda7a515bae1dfc2c0d35d52c/chrome/browser/chromeos/policy/weekly_time/weekly_time.cc
[modify] https://crrev.com/fd6e3df036d67c2dda7a515bae1dfc2c0d35d52c/chrome/browser/chromeos/policy/weekly_time/weekly_time.h
[modify] https://crrev.com/fd6e3df036d67c2dda7a515bae1dfc2c0d35d52c/chrome/browser/chromeos/policy/weekly_time/weekly_time_interval.cc
[modify] https://crrev.com/fd6e3df036d67c2dda7a515bae1dfc2c0d35d52c/chrome/browser/chromeos/policy/weekly_time/weekly_time_interval.h
[modify] https://crrev.com/fd6e3df036d67c2dda7a515bae1dfc2c0d35d52c/chrome/browser/chromeos/policy/weekly_time/weekly_time_interval_unittest.cc
[modify] https://crrev.com/fd6e3df036d67c2dda7a515bae1dfc2c0d35d52c/chrome/browser/chromeos/policy/weekly_time/weekly_time_unittest.cc

Comment 6 by adokar@google.com, Jun 14 2018

Components: Enterprise
Status: Fixed (was: Assigned)

Sign in to add a comment