Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(395)

Issue 2860443002: Chromad: Create AuthPolicyCredentialsManager (Closed)

Created:
3 years, 7 months ago by Roman Sorokin (ftl)
Modified:
3 years, 6 months 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

Messages

Total messages: 45 (24 generated)
Roman Sorokin (ftl)
Hi Thiemo, Lutz, PTAL.
3 years, 7 months 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 ...
3 years, 7 months 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 ...
3 years, 7 months 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 ...
3 years, 7 months 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 ...
3 years, 7 months 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: ...
3 years, 7 months 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) ...
3 years, 7 months 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 ...
3 years, 7 months 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 ...
3 years, 7 months 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" ...
3 years, 7 months 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 ...
3 years, 7 months 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 ...
3 years, 7 months ago (2017-05-26 13:06:33 UTC) #31
Thiemo Nagel
Thank you! Lgtm!
3 years, 7 months 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.
3 years, 6 months 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 years, 6 months ago (2017-05-30 17:32:58 UTC) #35
Elliot Glaysher
lgtm
3 years, 6 months 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 years, 6 months 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 years, 6 months 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 years, 6 months 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 years, 6 months ago (2017-06-01 09:31:44 UTC) #42
commit-bot: I haz the power
3 years, 6 months 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...

Powered by Google App Engine
This is Rietveld 408576698