Added affiliation IDs for the new affiliation determination.

Added affiliation IDs, that in future will be used to determine user affiliation to the device. Added unit-test as well.

BUG=491210
Committed: https://crrev.com/2bd626caac141ef01a2396d232fb6391d23b1e79
Cr-Commit-Position: refs/heads/master@{#341159}

[peletskyi, 2015-07-29 13:55:13]

peletskyi@chromium.org changed reviewers:
+ mnissler@chromium.org

[Mattias Nissler (ping if slow), 2015-07-29 20:08:14]

See comment on chrome_user_manager_impl.cc for high-level thoughts. In general,
the direction looks reasonable, assuming we still do want to move the affilation
field into User.

https://codereview.chromium.org/1266563002/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc (right):

https://codereview.chromium.org/1266563002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:726:
std::set<std::string> ChromeUserManagerImpl::GetDeviceAffiliationIDs() const {
Any good reason to have this as a separate function?

https://codereview.chromium.org/1266563002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:732: std::string
ChromeUserManagerImpl::GetEnterpriseDomain() const {
ditto, can't this simply be inlined in IsUserAffiliated?

https://codereview.chromium.org/1266563002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:1153: <<
IsUserAffiliated(user_affiliation_ids, user->GetEmail());
If you really just need to set the affiliation field in the user object, you
might just as well compute the affiliation in the caller, and then pass it in
here. However, neither solution will handle the case where the device-level
affiliation IDs change after policy fetch, and hence user affiliation needs to
be adjusted. Or maybe that is an edge case we don't care about?

On a related note: Wasn't the whole reason we decided to move the affiliation
field to User to enable caching there? Will we still cache something? If not, it
might actually not make much sense to move the affiliation field into the User
class at all... I might be missing something though.

https://codereview.chromium.org/1266563002/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/policy/browser_policy_connector_chromeos.cc
(right):

https://codereview.chromium.org/1266563002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/browser_policy_connector_chromeos.cc:392: }
Instead of the loop, I think you can just do this:

affiliation_ids.insert(policy_data->device_affiliation_ids().begin(),
                       policy_data->device_affiliation_ids().end());

https://codereview.chromium.org/1266563002/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc
(right):

https://codereview.chromium.org/1266563002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc:76:
std::set<std::string> AffiliationIDsToSet(const em::PolicyData& policy_data) {
No need to break out into a separate function.

https://codereview.chromium.org/1266563002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc:80: }
You can just use std::set::insert as noted in the other comment.

https://codereview.chromium.org/1266563002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc:293: if
(policy_data && chromeos::ChromeUserManager::Get()) {
Add a comment explaining when and why ChromeUserManager can return NULL. If this
is just to avoid crashes in tests, consider setting up a user manager in tests.

https://codereview.chromium.org/1266563002/diff/40001/chrome/browser/policy/t...
File chrome/browser/policy/test/policy_testserver.py (right):

https://codereview.chromium.org/1266563002/diff/40001/chrome/browser/policy/t...
chrome/browser/policy/test/policy_testserver.py:820:
policy_data.device_affiliation_ids.append(aff_id)
Doesn't this work?

policy_data.device_affiliation_ids.extend(device_affiliation_ids)

https://codereview.chromium.org/1266563002/diff/40001/chrome/browser/policy/t...
chrome/browser/policy/test/policy_testserver.py:824: for aff_id in
user_affiliation_ids:
ditto

[peletskyi, 2015-07-30 09:19:40]

peletskyi@chromium.org changed reviewers:
+ atwilson@chromium.org

[peletskyi, 2015-07-30 11:02:27]

PTAL

https://codereview.chromium.org/1266563002/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc (right):

https://codereview.chromium.org/1266563002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:726:
std::set<std::string> ChromeUserManagerImpl::GetDeviceAffiliationIDs() const {
On 2015/07/29 20:08:13, Mattias Nissler wrote:
> Any good reason to have this as a separate function?

Done.

https://codereview.chromium.org/1266563002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:732: std::string
ChromeUserManagerImpl::GetEnterpriseDomain() const {
On 2015/07/29 20:08:13, Mattias Nissler wrote:
> ditto, can't this simply be inlined in IsUserAffiliated?

Done.

https://codereview.chromium.org/1266563002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:1153: <<
IsUserAffiliated(user_affiliation_ids, user->GetEmail());
On 2015/07/29 20:08:13, Mattias Nissler wrote:
> If you really just need to set the affiliation field in the user object, you
> might just as well compute the affiliation in the caller, and then pass it in
> here. However, neither solution will handle the case where the device-level
> affiliation IDs change after policy fetch, and hence user affiliation needs to
> be adjusted. Or maybe that is an edge case we don't care about?
Many features are not updated when affiliation is changed after user is
logged-in. Thus we only care about login-time value and guarantee that new
affiliation will fully work after user would log in again. On that time new
device affiliation IDs are fetched and will have affect. Besides this after each
user policy fetch affiliation status is updated. So, if admin wants to have
updated affiliation at any time, he can just enforce user policy update.
> 
> On a related note: Wasn't the whole reason we decided to move the affiliation
> field to User to enable caching there? Will we still cache something? If not,
it
> might actually not make much sense to move the affiliation field into the User
> class at all... I might be missing something though.
The whole idea was that we should keep affiliation value somewhere. And User
seems to be the best place.

https://codereview.chromium.org/1266563002/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/policy/browser_policy_connector_chromeos.cc
(right):

https://codereview.chromium.org/1266563002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/browser_policy_connector_chromeos.cc:392: }
On 2015/07/29 20:08:13, Mattias Nissler wrote:
> Instead of the loop, I think you can just do this:
> 
> affiliation_ids.insert(policy_data->device_affiliation_ids().begin(),
>                        policy_data->device_affiliation_ids().end());

Done.

https://codereview.chromium.org/1266563002/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc
(right):

https://codereview.chromium.org/1266563002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc:76:
std::set<std::string> AffiliationIDsToSet(const em::PolicyData& policy_data) {
On 2015/07/29 20:08:13, Mattias Nissler wrote:
> No need to break out into a separate function.

Done.

https://codereview.chromium.org/1266563002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc:80: }
On 2015/07/29 20:08:13, Mattias Nissler wrote:
> You can just use std::set::insert as noted in the other comment.

Done.

https://codereview.chromium.org/1266563002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc:293: if
(policy_data && chromeos::ChromeUserManager::Get()) {
On 2015/07/29 20:08:13, Mattias Nissler wrote:
> Add a comment explaining when and why ChromeUserManager can return NULL. If
this
> is just to avoid crashes in tests, consider setting up a user manager in
tests.

No case when ChromeUserManager is NULL found. Condition removed.

https://codereview.chromium.org/1266563002/diff/40001/chrome/browser/policy/t...
File chrome/browser/policy/test/policy_testserver.py (right):

https://codereview.chromium.org/1266563002/diff/40001/chrome/browser/policy/t...
chrome/browser/policy/test/policy_testserver.py:820:
policy_data.device_affiliation_ids.append(aff_id)
On 2015/07/29 20:08:13, Mattias Nissler wrote:
> Doesn't this work?
> 
> policy_data.device_affiliation_ids.extend(device_affiliation_ids)

Done.

https://codereview.chromium.org/1266563002/diff/40001/chrome/browser/policy/t...
chrome/browser/policy/test/policy_testserver.py:824: for aff_id in
user_affiliation_ids:
On 2015/07/29 20:08:13, Mattias Nissler wrote:
> ditto

Done.

[peletskyi, 2015-07-30 11:39:27]

peletskyi@chromium.org changed reviewers:
+ dpolukhin@chromium.org

[peletskyi, 2015-07-30 11:39:28]

+dpolukhin@
dpolukhin@chromium.org: Please review changes in 
chrome/browser/chromeos/login/

[Mattias Nissler (ping if slow), 2015-07-30 12:10:57]

https://codereview.chromium.org/1266563002/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc (right):

https://codereview.chromium.org/1266563002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:1153: <<
IsUserAffiliated(user_affiliation_ids, user->GetEmail());
On 2015/07/30 11:02:27, peletskyi wrote:
> On 2015/07/29 20:08:13, Mattias Nissler wrote:
> > If you really just need to set the affiliation field in the user object, you
> > might just as well compute the affiliation in the caller, and then pass it
in
> > here. However, neither solution will handle the case where the device-level
> > affiliation IDs change after policy fetch, and hence user affiliation needs
to
> > be adjusted. Or maybe that is an edge case we don't care about?
> Many features are not updated when affiliation is changed after user is
> logged-in. Thus we only care about login-time value and guarantee that new
> affiliation will fully work after user would log in again. On that time new
> device affiliation IDs are fetched and will have affect. Besides this after
each
> user policy fetch affiliation status is updated. So, if admin wants to have
> updated affiliation at any time, he can just enforce user policy update.

OK, fair.

> > 
> > On a related note: Wasn't the whole reason we decided to move the
affiliation
> > field to User to enable caching there? Will we still cache something? If
not,
> it
> > might actually not make much sense to move the affiliation field into the
User
> > class at all... I might be missing something though.
> The whole idea was that we should keep affiliation value somewhere. And User
> seems to be the best place.

OK - now that components/User doesn't do any data caching etc., please make sure
that components/user/OWNERS are fine with this. I see you already added
dpolukhin@, so let's see what he says.

https://codereview.chromium.org/1266563002/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/affiliation.cc (right):

https://codereview.chromium.org/1266563002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/affiliation.cc:34: LOG(ERROR) <<
"GetUserAffiliation user_name: " << email;
All these LOG lines should be removed.

https://codereview.chromium.org/1266563002/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/affiliation.h (right):

https://codereview.chromium.org/1266563002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/affiliation.h:18: // TODO(peletskyi): Remove
email and enterprise domain after affiliation
nit: Please add a bug reference to the TODO

https://codereview.chromium.org/1266563002/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc (right):

https://codereview.chromium.org/1266563002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:1129: LOG(ERROR)
<< "SetUserAffiliation is not implemented yet. Called for user: "
remove LOG lines

https://codereview.chromium.org/1266563002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:1135: LOG(ERROR)
<< "Affiliation to set: "
You should actually set the affiliation here by calling User->SetAffiliation().
Any reason to not add User::SetAffiliation and the corresponding getter in this
CL?

https://codereview.chromium.org/1266563002/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc
(right):

https://codereview.chromium.org/1266563002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc:291:
policy_data->username(), set_of_user_affiliation_ids);
Do we need to canonicalize the user name here? We've had problems in the past
where upper/lower case spelling mismatches caused policy validation errors, you
might run into the same problem here.

https://codereview.chromium.org/1266563002/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h
(right):

https://codereview.chromium.org/1266563002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h:106: //
CloudPolicyManager
nit: Add colon to the end of the line.

https://codereview.chromium.org/1266563002/diff/100001/components/policy/prot...
File components/policy/proto/device_management_backend.proto (right):

https://codereview.chromium.org/1266563002/diff/100001/components/policy/prot...
components/policy/proto/device_management_backend.proto:391: repeated string
user_affiliation_ids = 24;
Is there a good reason to have two different fields for this? It seems that
using the same field would work fine, given that user policy and device policy
will always have a separate PolicyFetchResponse, and hence separate PolicyData
messages.

[peletskyi, 2015-07-30 14:10:58]

PTAL

https://codereview.chromium.org/1266563002/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/affiliation.cc (right):

https://codereview.chromium.org/1266563002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/affiliation.cc:34: LOG(ERROR) <<
"GetUserAffiliation user_name: " << email;
On 2015/07/30 12:10:57, Mattias Nissler wrote:
> All these LOG lines should be removed.

Done.

https://codereview.chromium.org/1266563002/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/affiliation.h (right):

https://codereview.chromium.org/1266563002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/affiliation.h:18: // TODO(peletskyi): Remove
email and enterprise domain after affiliation
On 2015/07/30 12:10:57, Mattias Nissler wrote:
> nit: Please add a bug reference to the TODO

Done.

https://codereview.chromium.org/1266563002/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc (right):

https://codereview.chromium.org/1266563002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:1129: LOG(ERROR)
<< "SetUserAffiliation is not implemented yet. Called for user: "
On 2015/07/30 12:10:57, Mattias Nissler wrote:
> remove LOG lines

Done.

https://codereview.chromium.org/1266563002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:1135: LOG(ERROR)
<< "Affiliation to set: "
On 2015/07/30 12:10:57, Mattias Nissler wrote:
> You should actually set the affiliation here by calling
User->SetAffiliation().
> Any reason to not add User::SetAffiliation and the corresponding getter in
this
> CL?

Done.

https://codereview.chromium.org/1266563002/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc
(right):

https://codereview.chromium.org/1266563002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc:291:
policy_data->username(), set_of_user_affiliation_ids);
On 2015/07/30 12:10:57, Mattias Nissler wrote:
> Do we need to canonicalize the user name here? We've had problems in the past
> where upper/lower case spelling mismatches caused policy validation errors,
you
> might run into the same problem here.

Done.

https://codereview.chromium.org/1266563002/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h
(right):

https://codereview.chromium.org/1266563002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h:106: //
CloudPolicyManager
On 2015/07/30 12:10:57, Mattias Nissler wrote:
> nit: Add colon to the end of the line.

Done.

https://codereview.chromium.org/1266563002/diff/100001/components/policy/prot...
File components/policy/proto/device_management_backend.proto (right):

https://codereview.chromium.org/1266563002/diff/100001/components/policy/prot...
components/policy/proto/device_management_backend.proto:391: repeated string
user_affiliation_ids = 24;
On 2015/07/30 12:10:57, Mattias Nissler wrote:
> Is there a good reason to have two different fields for this? It seems that
> using the same field would work fine, given that user policy and device policy
> will always have a separate PolicyFetchResponse, and hence separate PolicyData
> messages.

Both approaches have minuses. If we use only one list, this will lead  to usage
the same variable for different purposes in different case.
I and Drew stopped on this approach because
1. Maybe we will fetch both device and user IDs in the same policy in future.
2. If because of an error device affiliation IDs are sent with user policy, they
will be just ignored. No error occurred.
3. Easy to use. In code it is clear how to use user/device affiliation IDs. In
case of one field it may be complicated to understand for new developers how
this feature works because we will always refer to affiliation_id, that will
have different meaning in each case.

[Dmitry Polukhin, 2015-07-30 15:14:12]

LGTM chromeos/login

https://codereview.chromium.org/1266563002/diff/160001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/affiliation.h (right):

https://codereview.chromium.org/1266563002/diff/160001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/affiliation.h:15: bool
HaveCommonElement(const std::set<std::string>& set1,
I think it would be good to add typedef for std::set<std::string> to be able to
later changes underlying type of AffiliationSet.

[Mattias Nissler (ping if slow), 2015-07-30 15:34:08]

https://codereview.chromium.org/1266563002/diff/100001/components/policy/prot...
File components/policy/proto/device_management_backend.proto (right):

https://codereview.chromium.org/1266563002/diff/100001/components/policy/prot...
components/policy/proto/device_management_backend.proto:391: repeated string
user_affiliation_ids = 24;
On 2015/07/30 14:10:58, peletskyi wrote:
> On 2015/07/30 12:10:57, Mattias Nissler wrote:
> > Is there a good reason to have two different fields for this? It seems that
> > using the same field would work fine, given that user policy and device
policy
> > will always have a separate PolicyFetchResponse, and hence separate
PolicyData
> > messages.
> 
> Both approaches have minuses. If we use only one list, this will lead  to
usage
> the same variable for different purposes in different case.
> I and Drew stopped on this approach because
> 1. Maybe we will fetch both device and user IDs in the same policy in future.
> 2. If because of an error device affiliation IDs are sent with user policy,
they
> will be just ignored. No error occurred.
> 3. Easy to use. In code it is clear how to use user/device affiliation IDs. In
> case of one field it may be complicated to understand for new developers how
> this feature works because we will always refer to affiliation_id, that will
> have different meaning in each case.

Fair enough.

https://codereview.chromium.org/1266563002/diff/180001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/affiliation.cc (right):

https://codereview.chromium.org/1266563002/diff/180001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/affiliation.cc:49: if
(!device_affiliation_ids.empty() || !user_affiliation_ids.empty()) {
Do we need to worry about the case where the server is ready, but we have only
received IDs for either user or device yet? It might be better to check whether
both are present, i.e. fall back to the old way if either list is empty.

https://codereview.chromium.org/1266563002/diff/180001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/affiliation.h (right):

https://codereview.chromium.org/1266563002/diff/180001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/affiliation.h:18: // TODO(b/515476): Remove
email and enterprise domain after affiliation
b/<number> notation is misleading, as it's usually used for Google-internal
bugs. The convention here is to write
TODO(<person-who-knows-what-this-is-about>): comment goes here, see also
http://crbug.com/number

[peletskyi, 2015-07-30 16:39:50]

PTAL

https://codereview.chromium.org/1266563002/diff/160001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/affiliation.h (right):

https://codereview.chromium.org/1266563002/diff/160001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/affiliation.h:15: bool
HaveCommonElement(const std::set<std::string>& set1,
On 2015/07/30 15:14:11, Dmitry Polukhin wrote:
> I think it would be good to add typedef for std::set<std::string> to be able
to
> later changes underlying type of AffiliationSet.

Done everywhere except this function. To HaveCommonElement() we must provide set
only.

https://codereview.chromium.org/1266563002/diff/180001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/affiliation.cc (right):

https://codereview.chromium.org/1266563002/diff/180001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/affiliation.cc:49: if
(!device_affiliation_ids.empty() || !user_affiliation_ids.empty()) {
On 2015/07/30 15:34:08, Mattias Nissler wrote:
> Do we need to worry about the case where the server is ready, but we have only
> received IDs for either user or device yet? It might be better to check
whether
> both are present, i.e. fall back to the old way if either list is empty.

Done.

https://codereview.chromium.org/1266563002/diff/180001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/affiliation.h (right):

https://codereview.chromium.org/1266563002/diff/180001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/affiliation.h:18: // TODO(b/515476): Remove
email and enterprise domain after affiliation
On 2015/07/30 15:34:08, Mattias Nissler wrote:
> b/<number> notation is misleading, as it's usually used for Google-internal
> bugs. The convention here is to write
> TODO(<person-who-knows-what-this-is-about>): comment goes here, see also
> http://crbug.com/number

Done.

[Mattias Nissler (ping if slow), 2015-07-30 17:04:18]

LGTM w/ nits.

https://codereview.chromium.org/1266563002/diff/220001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/affiliation.h (right):

https://codereview.chromium.org/1266563002/diff/220001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/affiliation.h:11: namespace chromeos {
nit: newline

https://codereview.chromium.org/1266563002/diff/220001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/affiliation_unittest.cc (right):

https://codereview.chromium.org/1266563002/diff/220001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/affiliation_unittest.cc:8: #include
"chrome/browser/chromeos/login/users/affiliation.h"
nit: This header should be first #include in the file, followed by a newline.

https://codereview.chromium.org/1266563002/diff/220001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/chrome_user_manager_impl.h (right):

https://codereview.chromium.org/1266563002/diff/220001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.h:121: const
AffiliationIDSet& user_affiliation_ids) override;
nit: #include affiliation.h

https://codereview.chromium.org/1266563002/diff/220001/chrome/browser/chromeo...
File chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc
(right):

https://codereview.chromium.org/1266563002/diff/220001/chrome/browser/chromeo...
chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc:287:
chromeos::AffiliationIDSet set_of_user_affiliation_ids(
#include affilation.h

[Mattias Nissler (ping if slow), 2015-07-30 17:05:26]

Please also update issue description to list the correct bug in the BUG line.

[peletskyi, 2015-07-30 17:13:34]

Done!

https://codereview.chromium.org/1266563002/diff/220001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/affiliation.h (right):

https://codereview.chromium.org/1266563002/diff/220001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/affiliation.h:11: namespace chromeos {
On 2015/07/30 17:04:17, Mattias Nissler wrote:
> nit: newline

Done.

https://codereview.chromium.org/1266563002/diff/220001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/affiliation_unittest.cc (right):

https://codereview.chromium.org/1266563002/diff/220001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/affiliation_unittest.cc:8: #include
"chrome/browser/chromeos/login/users/affiliation.h"
On 2015/07/30 17:04:17, Mattias Nissler wrote:
> nit: This header should be first #include in the file, followed by a newline.

Done.

https://codereview.chromium.org/1266563002/diff/220001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/users/chrome_user_manager_impl.h (right):

https://codereview.chromium.org/1266563002/diff/220001/chrome/browser/chromeo...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.h:121: const
AffiliationIDSet& user_affiliation_ids) override;
On 2015/07/30 17:04:17, Mattias Nissler wrote:
> nit: #include affiliation.h

Done.

https://codereview.chromium.org/1266563002/diff/220001/chrome/browser/chromeo...
File chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc
(right):

https://codereview.chromium.org/1266563002/diff/220001/chrome/browser/chromeo...
chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc:287:
chromeos::AffiliationIDSet set_of_user_affiliation_ids(
On 2015/07/30 17:04:18, Mattias Nissler wrote:
> #include affilation.h

Done.

[peletskyi, 2015-07-30 17:13:56]

The CQ bit was checked by peletskyi@chromium.org

[peletskyi, 2015-07-30 17:13:57]

The patchset sent to the CQ was uploaded after l-g-t-m from
dpolukhin@chromium.org, mnissler@chromium.org
Link to the patchset: https://codereview.chromium.org/1266563002/#ps240001
(title: " ")

[commit-bot: I haz the power, 2015-07-30 17:15:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1266563002/240001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1266563002/240001

[commit-bot: I haz the power, 2015-07-30 18:04:27]

Committed patchset #13 (id:240001)

[commit-bot: I haz the power, 2015-07-30 18:05:18]

Patchset 13 (id:??) landed as
https://crrev.com/2bd626caac141ef01a2396d232fb6391d23b1e79
Cr-Commit-Position: refs/heads/master@{#341159}

[dvh, 2015-07-30 19:29:16]

A revert of this CL (patchset #13 id:240001) has been created in
https://codereview.chromium.org/1262883003/ by dvh@chromium.org.

The reason for reverting is: Breaking the following build: Linux ChromiumOS
Tests (1) Build #6802

https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%...
.