Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2512733004:
Request device policy from authpolicyd (Closed)
|
Created:
4 years, 1 month ago by Thiemo Nagel Modified:
4 years, 1 month ago Reviewers:
Roman Sorokin (ftl) CC:
chromium-reviews, oshima+watch_chromium.org, davemoore+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionRequest device policy from authpolicyd
Wire up the D-Bus call for device policy fetch in
DeviceActiveDirectoryPolicyManager::RefreshPolicies().
BUG=656558
Committed: https://crrev.com/0f18dd0634ded648d1577bcf786555801e224401
Cr-Commit-Position: refs/heads/master@{#433580}
Patch Set 1 #
Total comments: 3
Patch Set 2 : Address Roman's comments #
Total comments: 2
Patch Set 3 : Expand AD --> Active Directory #
Messages
Total messages: 21 (13 generated)
The CQ bit was checked by tnagel@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
tnagel@chromium.org changed reviewers: + rsorokin@chromium.org
Hi Roman, could you please take a look? (I haven't tested this on a real device.) Thank you, Thiemo
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm with nits https://codereview.chromium.org/2512733004/diff/1/chrome/browser/chromeos/pol... File chrome/browser/chromeos/policy/device_active_directory_policy_manager.cc (right): https://codereview.chromium.org/2512733004/diff/1/chrome/browser/chromeos/pol... chrome/browser/chromeos/policy/device_active_directory_policy_manager.cc:57: } Don't check for the null or use DCHECK https://codereview.chromium.org/2512733004/diff/1/chrome/browser/chromeos/pol... chrome/browser/chromeos/policy/device_active_directory_policy_manager.cc:97: } Add error log in case of a failure.
The CQ bit was checked by tnagel@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Roman, could you please take another look? Thank you, Thiemo https://codereview.chromium.org/2512733004/diff/1/chrome/browser/chromeos/pol... File chrome/browser/chromeos/policy/device_active_directory_policy_manager.cc (right): https://codereview.chromium.org/2512733004/diff/1/chrome/browser/chromeos/pol... chrome/browser/chromeos/policy/device_active_directory_policy_manager.cc:57: } On 2016/11/18 15:07:48, Roman Sorokin wrote: > Don't check for the null or use DCHECK Done.
lgtm with nit https://codereview.chromium.org/2512733004/diff/20001/chrome/browser/chromeos... File chrome/browser/chromeos/policy/device_active_directory_policy_manager.cc (right): https://codereview.chromium.org/2512733004/diff/20001/chrome/browser/chromeos... chrome/browser/chromeos/policy/device_active_directory_policy_manager.cc:94: LOG(ERROR) << "AD policy refresh failed."; nit: s/AD/Active Directory
https://codereview.chromium.org/2512733004/diff/20001/chrome/browser/chromeos... File chrome/browser/chromeos/policy/device_active_directory_policy_manager.cc (right): https://codereview.chromium.org/2512733004/diff/20001/chrome/browser/chromeos... chrome/browser/chromeos/policy/device_active_directory_policy_manager.cc:94: LOG(ERROR) << "AD policy refresh failed."; On 2016/11/21 16:47:53, Roman Sorokin wrote: > nit: s/AD/Active Directory Done, although this is bloating the Chrome executable by 15 Bytes.
The CQ bit was checked by tnagel@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rsorokin@chromium.org Link to the patchset: https://codereview.chromium.org/2512733004/#ps40001 (title: "Expand AD --> Active Directory")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1479747985407430,
"parent_rev": "6e19981d4faad76e0f204db8d6ee765ba1d3bd81", "commit_rev":
"012cbc1aabdf11e11e4ec307febbd47265a73a78"}
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Request device policy from authpolicyd Wire up the D-Bus call for device policy fetch in DeviceActiveDirectoryPolicyManager::RefreshPolicies(). BUG=656558 ========== to ========== Request device policy from authpolicyd Wire up the D-Bus call for device policy fetch in DeviceActiveDirectoryPolicyManager::RefreshPolicies(). BUG=656558 Committed: https://crrev.com/0f18dd0634ded648d1577bcf786555801e224401 Cr-Commit-Position: refs/heads/master@{#433580} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/0f18dd0634ded648d1577bcf786555801e224401 Cr-Commit-Position: refs/heads/master@{#433580} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
