Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(1)

Issue 2860443002: Chromad: Create AuthPolicyCredentialsManager (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 month, 3 weeks ago by Roman Sorokin (ftl)
Modified:
3 weeks, 3 days ago
CC:
chromium-reviews, srahim+watch_chromium.org, oshima+watch_chromium.org, davemoore+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Chromad: Create AuthPolicyCredentialsManager AuthPolicyCredentialsManager KeyedService service is created for every Active Directory user profile. It calls GetUserStatus at the start of service, each hour and on every network connection. Tests are coming... BUG=662400 Review-Url: https://codereview.chromium.org/2860443002 Cr-Commit-Position: refs/heads/master@{#476248} Committed: https://chromium.googlesource.com/chromium/src/+/7ccdfaf7c1639a6f1e12d206fea9a608b6c33f44

Patch Set 1 #

Total comments: 55

Patch Set 2 : Rebase, Update after review #

Total comments: 4

Patch Set 3 : Update after review #

Total comments: 10

Patch Set 4 : More addressing comments #

Total comments: 35

Patch Set 5 : Address comments #

Patch Set 6 : Fix notification id. #

Patch Set 7 : Fix notification id. #

Total comments: 4

Patch Set 8 : Address comments #

Total comments: 10

Patch Set 9 : Update after review #

Total comments: 2

Patch Set 10 : nits #

Unified diffs Side-by-side diffs Delta from patch set Stats (+415 lines, -0 lines) Patch
M chrome/app/generated_resources.grd View 1 2 3 4 1 chunk +9 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/BUILD.gn View 1 2 chunks +3 lines, -0 lines 0 comments Download
A chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.h View 1 2 3 4 5 6 7 8 9 1 chunk +115 lines, -0 lines 0 comments Download
A chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc View 1 2 3 4 5 6 7 8 9 1 chunk +286 lines, -0 lines 0 comments Download
M chrome/browser/profiles/profile_impl.cc View 1 2 chunks +2 lines, -0 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 45 (24 generated)
Roman Sorokin (ftl)
Hi Thiemo, Lutz, PTAL.
1 month, 3 weeks ago (2017-05-02 14:09:25 UTC) #4
ljusten (tachyonic)
https://codereview.chromium.org/2860443002/diff/1/chrome/app/generated_resources.grd File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/2860443002/diff/1/chrome/app/generated_resources.grd#newcode9566 chrome/app/generated_resources.grd:9566: Your password has expired. Please sign out then sign ...
1 month, 3 weeks ago (2017-05-03 10:12:17 UTC) #7
Roman Sorokin (ftl)
Thanks Lutz, PTAL! https://codereview.chromium.org/2860443002/diff/1/chrome/app/generated_resources.grd File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/2860443002/diff/1/chrome/app/generated_resources.grd#newcode9566 chrome/app/generated_resources.grd:9566: Your password has expired. Please sign ...
1 month, 1 week ago (2017-05-19 12:13:48 UTC) #10
ljusten (tachyonic)
https://codereview.chromium.org/2860443002/diff/1/chrome/app/generated_resources.grd File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/2860443002/diff/1/chrome/app/generated_resources.grd#newcode9566 chrome/app/generated_resources.grd:9566: Your password has expired. Please sign out then sign ...
1 month, 1 week ago (2017-05-19 16:06:14 UTC) #13
Roman Sorokin (ftl)
Thanks, Lutz! PTAL. https://codereview.chromium.org/2860443002/diff/1/chrome/app/generated_resources.grd File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/2860443002/diff/1/chrome/app/generated_resources.grd#newcode9566 chrome/app/generated_resources.grd:9566: Your password has expired. Please sign ...
1 month ago (2017-05-22 12:35:27 UTC) #16
ljusten (tachyonic)
https://codereview.chromium.org/2860443002/diff/1/chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc File chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc (right): https://codereview.chromium.org/2860443002/diff/1/chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc#newcode50 chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc:50: void FixSignIn(); On 2017/05/22 12:35:27, Roman Sorokin (ftl) wrote: ...
1 month ago (2017-05-22 14:27:49 UTC) #19
Roman Sorokin (ftl)
Thanks Lutz, PTAL! https://codereview.chromium.org/2860443002/diff/40001/chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc File chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc (right): https://codereview.chromium.org/2860443002/diff/40001/chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc#newcode107 chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc:107: DCHECK_CURRENTLY_ON(content::BrowserThread::UI); On 2017/05/22 14:27:49, ljusten (instantaneous) ...
1 month ago (2017-05-22 16:38:24 UTC) #22
ljusten (tachyonic)
Almost there... https://codereview.chromium.org/2860443002/diff/60001/chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc File chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc (right): https://codereview.chromium.org/2860443002/diff/60001/chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc#newcode122 chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc:122: last_error_ = error; I believe this will ...
1 month ago (2017-05-22 16:52:00 UTC) #23
Thiemo Nagel
https://codereview.chromium.org/2860443002/diff/60001/chrome/app/generated_resources.grd File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/2860443002/diff/60001/chrome/app/generated_resources.grd#newcode9436 chrome/app/generated_resources.grd:9436: <message name="IDS_ACTIVE_DIRECTORY_PASSWORD_EXPIRED" desc="The text to display on the hyperlink ...
1 month ago (2017-05-24 10:11:05 UTC) #26
Roman Sorokin (ftl)
Thiemo, Lutz, thanks for the review! PTAL https://codereview.chromium.org/2860443002/diff/60001/chrome/app/generated_resources.grd File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/2860443002/diff/60001/chrome/app/generated_resources.grd#newcode9436 chrome/app/generated_resources.grd:9436: <message name="IDS_ACTIVE_DIRECTORY_PASSWORD_EXPIRED" ...
1 month ago (2017-05-26 12:03:37 UTC) #29
Thiemo Nagel
Thank you! Almost done. https://codereview.chromium.org/2860443002/diff/60001/chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc File chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc (right): https://codereview.chromium.org/2860443002/diff/60001/chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc#newcode77 chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc:77: DCHECK_CURRENTLY_ON(content::BrowserThread::UI); > I dunno, just ...
1 month ago (2017-05-26 12:46:21 UTC) #30
Roman Sorokin (ftl)
Thanks, Thiemo! PTAL. https://codereview.chromium.org/2860443002/diff/60001/chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc File chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc (right): https://codereview.chromium.org/2860443002/diff/60001/chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc#newcode77 chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc:77: DCHECK_CURRENTLY_ON(content::BrowserThread::UI); On 2017/05/26 12:46:21, Thiemo Nagel ...
1 month ago (2017-05-26 13:06:33 UTC) #31
Thiemo Nagel
Thank you! Lgtm!
1 month ago (2017-05-26 13:50:29 UTC) #32
Roman Sorokin (ftl)
Hi Elliot, PTAL at chrome/browser/profiles/profile_impl.cc Hi Xiyuan, PTAL.
4 weeks ago (2017-05-29 09:16:41 UTC) #34
xiyuan
https://codereview.chromium.org/2860443002/diff/140001/chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc File chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc (right): https://codereview.chromium.org/2860443002/diff/140001/chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc#newcode85 chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc:85: StopObserveNetwork(); What if this is called after OnShuttingDown() is ...
3 weeks, 5 days ago (2017-05-30 17:32:58 UTC) #35
Elliot Glaysher
lgtm
3 weeks, 5 days ago (2017-05-30 19:27:08 UTC) #36
Roman Sorokin (ftl)
Thanks Xiyuan! PTAL. https://codereview.chromium.org/2860443002/diff/140001/chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc File chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc (right): https://codereview.chromium.org/2860443002/diff/140001/chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc#newcode85 chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc:85: StopObserveNetwork(); On 2017/05/30 17:32:58, xiyuan wrote: ...
3 weeks, 4 days ago (2017-05-31 15:37:10 UTC) #37
xiyuan
lgtm with nits https://codereview.chromium.org/2860443002/diff/140001/chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.h File chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.h (right): https://codereview.chromium.org/2860443002/diff/140001/chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.h#newcode74 chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.h:74: Profile* profile_; On 2017/05/31 15:37:10, Roman ...
3 weeks, 4 days ago (2017-05-31 15:42:00 UTC) #38
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2860443002/180001
3 weeks, 4 days ago (2017-06-01 09:31:43 UTC) #41
Roman Sorokin (ftl)
https://codereview.chromium.org/2860443002/diff/140001/chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.h File chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.h (right): https://codereview.chromium.org/2860443002/diff/140001/chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.h#newcode74 chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.h:74: Profile* profile_; On 2017/05/31 15:41:59, xiyuan wrote: > On ...
3 weeks, 4 days ago (2017-06-01 09:31:44 UTC) #42
commit-bot: I haz the power
3 weeks, 3 days ago (2017-06-01 10:56:36 UTC) #45
Message was sent while issue was closed.
Committed patchset #10 (id:180001) as
https://chromium.googlesource.com/chromium/src/+/7ccdfaf7c1639a6f1e12d206fea9...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cb946e318