|
|
Chromium Code Reviews|
Created:
3 years, 12 months ago by Roman Sorokin (ftl) Modified:
3 years, 12 months ago CC:
chromium-reviews, alemate+watch_chromium.org, achuith+watch_chromium.org, oshima+watch_chromium.org, davemoore+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionExplicitly forbid multiprofiles on the Active Directory devices.
BUG=675619
TEST=none
Committed: https://crrev.com/65b885d310b46d4817fcd0ede8e37ed3f0ca8254
Cr-Commit-Position: refs/heads/master@{#440639}
Patch Set 1 #
Total comments: 4
Patch Set 2 : Remove nullptr check #
Total comments: 2
Patch Set 3 : Rewrite comment. #Messages
Total messages: 23 (11 generated)
The CQ bit was checked by rsorokin@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...
rsorokin@chromium.org changed reviewers: + alemate@chromium.org
Hey Alex, PTAL. Thanks!
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
tnagel@chromium.org changed reviewers: + tnagel@chromium.org
https://codereview.chromium.org/2599033002/diff/1/chrome/browser/chromeos/log... File chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc (right): https://codereview.chromium.org/2599033002/diff/1/chrome/browser/chromeos/log... chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:304: if (connector && connector->IsActiveDirectoryManaged()) In which case could the connector be nullptr?
https://codereview.chromium.org/2599033002/diff/1/chrome/browser/chromeos/log... File chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc (right): https://codereview.chromium.org/2599033002/diff/1/chrome/browser/chromeos/log... chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:304: if (connector && connector->IsActiveDirectoryManaged()) On 2016/12/22 13:52:08, Thiemo Nagel (slow) wrote: > In which case could the connector be nullptr? I dunno. Probably never could.
https://codereview.chromium.org/2599033002/diff/1/chrome/browser/chromeos/log... File chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc (right): https://codereview.chromium.org/2599033002/diff/1/chrome/browser/chromeos/log... chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:304: if (connector && connector->IsActiveDirectoryManaged()) On 2016/12/22 13:56:05, Roman Sorokin wrote: > On 2016/12/22 13:52:08, Thiemo Nagel (slow) wrote: > > In which case could the connector be nullptr? > > I dunno. Probably never could. Then maybe we shouldn't check for it?
https://codereview.chromium.org/2599033002/diff/1/chrome/browser/chromeos/log... File chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc (right): https://codereview.chromium.org/2599033002/diff/1/chrome/browser/chromeos/log... chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:304: if (connector && connector->IsActiveDirectoryManaged()) On 2016/12/22 13:57:49, Thiemo Nagel (slow) wrote: > On 2016/12/22 13:56:05, Roman Sorokin wrote: > > On 2016/12/22 13:52:08, Thiemo Nagel (slow) wrote: > > > In which case could the connector be nullptr? > > > > I dunno. Probably never could. > > Then maybe we shouldn't check for it? Done.
lgtm
lgtm https://codereview.chromium.org/2599033002/diff/20001/chrome/browser/chromeos... File chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc (right): https://codereview.chromium.org/2599033002/diff/20001/chrome/browser/chromeos... chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:301: // Multiprofiles are not allowed on the Active Directory managed devices. nit: "Multiprofile mode is not allowed..."
The CQ bit was checked by rsorokin@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from tnagel@chromium.org, alemate@chromium.org Link to the patchset: https://codereview.chromium.org/2599033002/#ps40001 (title: "Rewrite comment.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2599033002/diff/20001/chrome/browser/chromeos... File chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc (right): https://codereview.chromium.org/2599033002/diff/20001/chrome/browser/chromeos... chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:301: // Multiprofiles are not allowed on the Active Directory managed devices. On 2016/12/23 15:41:41, Alexander Alekseev wrote: > nit: "Multiprofile mode is not allowed..." Done.
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1482509178138950,
"parent_rev": "b15a0db2af98aaba90f546fa278ab985be5b488c", "commit_rev":
"549d84609ff93fbdbd5ab89399c0a2734fe943bd"}
Message was sent while issue was closed.
Description was changed from ========== Explicitly forbid multiprofiles on the Active Directory devices. BUG=675619 TEST=none ========== to ========== Explicitly forbid multiprofiles on the Active Directory devices. BUG=675619 TEST=none Review-Url: https://codereview.chromium.org/2599033002 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Explicitly forbid multiprofiles on the Active Directory devices. BUG=675619 TEST=none Review-Url: https://codereview.chromium.org/2599033002 ========== to ========== Explicitly forbid multiprofiles on the Active Directory devices. BUG=675619 TEST=none Committed: https://crrev.com/65b885d310b46d4817fcd0ede8e37ed3f0ca8254 Cr-Commit-Position: refs/heads/master@{#440639} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/65b885d310b46d4817fcd0ede8e37ed3f0ca8254 Cr-Commit-Position: refs/heads/master@{#440639} |
