Remove unused email from install attributes and add support for AD.

Remove unused email from install attributes and add support for AD.

Notable changes:
* Simplify attribute accessors by ensuring that attribute members
  are always self-consistent.
* Drop distinction between unset and empty attributes.
* Strict enforcement of *all* attributes being identical on
  re-locking.
* IsEnterpriseDevice() evaluates the device mode instead of
  the (deprecated) user.
* Improve encapsulation of StubInstallAttributes by disabling
  direct member access, provide high-level setters instead.

BUG=639295, 638713
TBR=achuith (chrome/browser/chromeos/{app_mode,login,system})
TBR=reillyg (chrome/browser/extensions)
TBR=eroman (chrome/browser/net)
TBR=bauerb (chrome/browser/ui/webui)

Committed: https://crrev.com/42d7ea0a129585044fa4461e7d581ad88e4525dc
Cr-Commit-Position: refs/heads/master@{#424994}

[Thiemo Nagel, 2016-10-10 18:56:22]

The CQ bit was checked by tnagel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-10 18:56:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-10 19:10:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-10 19:10:33]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)

[Thiemo Nagel, 2016-10-10 19:15:20]

The CQ bit was checked by tnagel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-10 19:15:43]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-10 19:30:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-10 19:30:46]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)

[Thiemo Nagel, 2016-10-11 09:39:18]

The CQ bit was checked by tnagel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-11 09:39:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Thiemo Nagel, 2016-10-11 09:42:18]

Patchset #1 (id:1) has been deleted

[commit-bot: I haz the power, 2016-10-11 09:52:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-11 09:52:58]

Dry run: Try jobs failed on following builders:
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)

[Thiemo Nagel, 2016-10-11 10:13:38]

The CQ bit was checked by tnagel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-11 10:13:44]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Thiemo Nagel, 2016-10-11 10:14:43]

The CQ bit was checked by tnagel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-11 10:14:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Thiemo Nagel, 2016-10-11 10:15:10]

Patchset #3 (id:60001) has been deleted

[Thiemo Nagel, 2016-10-11 11:59:09]

Patchset #1 (id:20001) has been deleted

[Thiemo Nagel, 2016-10-11 11:59:21]

Patchset #1 (id:40001) has been deleted

[Thiemo Nagel, 2016-10-11 12:03:06]

The CQ bit was checked by tnagel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-11 12:03:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-11 12:12:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-11 12:12:32]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Thiemo Nagel, 2016-10-11 12:47:09]

Description was changed from

==========
work in progress -- install attributes

BUG=
==========

to

==========
Remove unused email from install attributes and add support for AD.

Notable changes:
* Strict enforcement of *all* attributes being identical on re-locking.
* Drop distinction between unset and empty attributes.
* IsEnterpriseDevice() evaluates the device mode instead of the (deprecated)
user.
* Simplify attribute accessors by ensuring that attribute members are always
self-consistent.
* Improve encapsulation of StubInstallAttributes by disabling direct member
access, provide high-level setters instead.

BUG=639295
==========

[Thiemo Nagel, 2016-10-11 12:48:23]

The CQ bit was checked by tnagel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-11 12:48:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Thiemo Nagel, 2016-10-11 12:48:46]

Patchset #1 (id:80001) has been deleted

[commit-bot: I haz the power, 2016-10-11 13:00:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-11 13:00:28]

Dry run: Try jobs failed on following builders:
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)

[Thiemo Nagel, 2016-10-11 13:16:07]

The CQ bit was checked by tnagel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-11 13:16:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Thiemo Nagel, 2016-10-11 13:31:39]

The CQ bit was checked by tnagel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-11 13:31:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Thiemo Nagel, 2016-10-11 13:32:34]

Patchset #1 (id:100001) has been deleted

[Thiemo Nagel, 2016-10-11 13:32:43]

Patchset #1 (id:120001) has been deleted

[Thiemo Nagel, 2016-10-11 13:44:19]

The CQ bit was checked by tnagel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-11 13:44:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Thiemo Nagel, 2016-10-11 13:44:58]

tnagel@chromium.org changed reviewers:
+ emaxx@chromium.org

[Thiemo Nagel, 2016-10-11 13:44:59]

Hi Maksim,

could you please take a look?

Thank you,
Thiemo

[Thiemo Nagel, 2016-10-11 13:46:31]

Description was changed from

==========
Remove unused email from install attributes and add support for AD.

Notable changes:
* Strict enforcement of *all* attributes being identical on re-locking.
* Drop distinction between unset and empty attributes.
* IsEnterpriseDevice() evaluates the device mode instead of the (deprecated)
user.
* Simplify attribute accessors by ensuring that attribute members are always
self-consistent.
* Improve encapsulation of StubInstallAttributes by disabling direct member
access, provide high-level setters instead.

BUG=639295
==========

to

==========
Remove unused email from install attributes and add support for AD.

Notable changes:
* Simplify attribute accessors by ensuring that attribute members
  are always self-consistent.
* Drop distinction between unset and empty attributes.
* Strict enforcement of *all* attributes being identical on
  re-locking.
* IsEnterpriseDevice() evaluates the device mode instead of
  the (deprecated) user.
* Improve encapsulation of StubInstallAttributes by disabling
  direct member access, provide high-level setters instead.

BUG=639295
==========

[Thiemo Nagel, 2016-10-11 13:47:18]

+mnissler, +rsorokin FYI

[commit-bot: I haz the power, 2016-10-11 14:59:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-11 14:59:03]

Dry run: This issue passed the CQ dry run.

[emaxx, 2016-10-11 17:26:37]

lgtm in general (at least according to my knowledge of the install attributes).

https://codereview.chromium.org/2403053003/diff/180001/chrome/browser/chromeo...
File chrome/browser/chromeos/app_mode/kiosk_app_manager_browsertest.cc (right):

https://codereview.chromium.org/2403053003/diff/180001/chrome/browser/chromeo...
chrome/browser/chromeos/app_mode/kiosk_app_manager_browsertest.cc:296:
"user@domain.com",
I guess, "user@" should be removed here too, as this argument becomes the domain
now?

https://codereview.chromium.org/2403053003/diff/180001/chrome/browser/chromeo...
chrome/browser/chromeos/app_mode/kiosk_app_manager_browsertest.cc:297:
std::string(),  // realm
Did you consider defining a special constant for this empty realm in some header
file? This would remove the need to comment each occurrence of this string.
(However, on the other side, it would become non-obvious for the reader that
it's actually an empty string. Not sure whether that's important.)

https://codereview.chromium.org/2403053003/diff/180001/chrome/browser/chromeo...
File chrome/browser/chromeos/settings/install_attributes.cc (right):

https://codereview.chromium.org/2403053003/diff/180001/chrome/browser/chromeo...
chrome/browser/chromeos/settings/install_attributes.cc:477: LOG(WARNING) << 
"Couldn't read domain.";
nit: Two spaces instead of one.

https://codereview.chromium.org/2403053003/diff/180001/chrome/browser/chromeo...
File chrome/browser/chromeos/settings/stub_install_attributes.h (right):

https://codereview.chromium.org/2403053003/diff/180001/chrome/browser/chromeo...
chrome/browser/chromeos/settings/stub_install_attributes.h:49:
ScopedStubInstallAttributes ScopedStubInstallAttributesUnset();
Isn't it more common to make such simple factories just static members of the
class?

[Thiemo Nagel, 2016-10-11 18:22:12]

The CQ bit was checked by tnagel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-11 18:22:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Thiemo Nagel, 2016-10-11 18:23:30]

Thanks a lot, Maksim.  I've addressed your comments in a new upload.

https://codereview.chromium.org/2403053003/diff/180001/chrome/browser/chromeo...
File chrome/browser/chromeos/app_mode/kiosk_app_manager_browsertest.cc (right):

https://codereview.chromium.org/2403053003/diff/180001/chrome/browser/chromeo...
chrome/browser/chromeos/app_mode/kiosk_app_manager_browsertest.cc:296:
"user@domain.com",
On 2016/10/11 17:26:37, emaxx wrote:
> I guess, "user@" should be removed here too, as this argument becomes the
domain
> now?

Good catch, thanks.  Fixed.

https://codereview.chromium.org/2403053003/diff/180001/chrome/browser/chromeo...
chrome/browser/chromeos/app_mode/kiosk_app_manager_browsertest.cc:297:
std::string(),  // realm
On 2016/10/11 17:26:37, emaxx wrote:
> Did you consider defining a special constant for this empty realm in some
header
> file? This would remove the need to comment each occurrence of this string.
> (However, on the other side, it would become non-obvious for the reader that
> it's actually an empty string. Not sure whether that's important.)

Interesting point.  Actually I think it's probably clearer the way it's written
right now.  (I'm not bothered by the comment.)

https://codereview.chromium.org/2403053003/diff/180001/chrome/browser/chromeo...
File chrome/browser/chromeos/settings/install_attributes.cc (right):

https://codereview.chromium.org/2403053003/diff/180001/chrome/browser/chromeo...
chrome/browser/chromeos/settings/install_attributes.cc:477: LOG(WARNING) << 
"Couldn't read domain.";
On 2016/10/11 17:26:37, emaxx wrote:
> nit: Two spaces instead of one.

Thanks.  Fixed.

https://codereview.chromium.org/2403053003/diff/180001/chrome/browser/chromeo...
File chrome/browser/chromeos/settings/stub_install_attributes.h (right):

https://codereview.chromium.org/2403053003/diff/180001/chrome/browser/chromeo...
chrome/browser/chromeos/settings/stub_install_attributes.h:49:
ScopedStubInstallAttributes ScopedStubInstallAttributesUnset();
On 2016/10/11 17:26:37, emaxx wrote:
> Isn't it more common to make such simple factories just static members of the
> class?

Yes, you're right.  Looks much better now.  :)

[Thiemo Nagel, 2016-10-11 18:27:55]

tnagel@chromium.org changed reviewers:
+ achuith@chromium.org

[Thiemo Nagel, 2016-10-11 18:27:56]

Hi Achuith,

may I ask you to take a look at chrome/browser/chromeos/{app_mode,login,system}?

Thank you,
Thiemo

[Thiemo Nagel, 2016-10-11 18:28:45]

Patchset #1 (id:140001) has been deleted

[Thiemo Nagel, 2016-10-11 18:29:01]

Patchset #1 (id:160001) has been deleted

[commit-bot: I haz the power, 2016-10-11 19:45:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-11 19:45:53]

Dry run: This issue passed the CQ dry run.

[Thiemo Nagel, 2016-10-12 13:08:31]

Description was changed from

==========
Remove unused email from install attributes and add support for AD.

Notable changes:
* Simplify attribute accessors by ensuring that attribute members
  are always self-consistent.
* Drop distinction between unset and empty attributes.
* Strict enforcement of *all* attributes being identical on
  re-locking.
* IsEnterpriseDevice() evaluates the device mode instead of
  the (deprecated) user.
* Improve encapsulation of StubInstallAttributes by disabling
  direct member access, provide high-level setters instead.

BUG=639295
==========

to

==========
Remove unused email from install attributes and add support for AD.

Notable changes:
* Simplify attribute accessors by ensuring that attribute members
  are always self-consistent.
* Drop distinction between unset and empty attributes.
* Strict enforcement of *all* attributes being identical on
  re-locking.
* IsEnterpriseDevice() evaluates the device mode instead of
  the (deprecated) user.
* Improve encapsulation of StubInstallAttributes by disabling
  direct member access, provide high-level setters instead.

BUG=639295, 638713
==========

[Thiemo Nagel, 2016-10-13 09:22:26]

Description was changed from

==========
Remove unused email from install attributes and add support for AD.

Notable changes:
* Simplify attribute accessors by ensuring that attribute members
  are always self-consistent.
* Drop distinction between unset and empty attributes.
* Strict enforcement of *all* attributes being identical on
  re-locking.
* IsEnterpriseDevice() evaluates the device mode instead of
  the (deprecated) user.
* Improve encapsulation of StubInstallAttributes by disabling
  direct member access, provide high-level setters instead.

BUG=639295, 638713
==========

to

==========
Remove unused email from install attributes and add support for AD.

Notable changes:
* Simplify attribute accessors by ensuring that attribute members
  are always self-consistent.
* Drop distinction between unset and empty attributes.
* Strict enforcement of *all* attributes being identical on
  re-locking.
* IsEnterpriseDevice() evaluates the device mode instead of
  the (deprecated) user.
* Improve encapsulation of StubInstallAttributes by disabling
  direct member access, provide high-level setters instead.

BUG=639295, 638713
TBR=achuith (chrome/browser/chromeos/{app_mode,login,system})
TBR=reillyg (chrome/browser/extensions)
TBR=eroman (chrome/browser/net)
TBR=msw (chrome/browser/ui)
==========

[Thiemo Nagel, 2016-10-13 09:22:49]

The CQ bit was checked by tnagel@chromium.org

[Thiemo Nagel, 2016-10-13 09:22:49]

The patchset sent to the CQ was uploaded after l-g-t-m from emaxx@chromium.org
Link to the patchset: https://codereview.chromium.org/2403053003/#ps200001
(title: "Address Maksim's comments.")

[commit-bot: I haz the power, 2016-10-13 09:23:09]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Thiemo Nagel, 2016-10-13 09:24:03]

Description was changed from

==========
Remove unused email from install attributes and add support for AD.

Notable changes:
* Simplify attribute accessors by ensuring that attribute members
  are always self-consistent.
* Drop distinction between unset and empty attributes.
* Strict enforcement of *all* attributes being identical on
  re-locking.
* IsEnterpriseDevice() evaluates the device mode instead of
  the (deprecated) user.
* Improve encapsulation of StubInstallAttributes by disabling
  direct member access, provide high-level setters instead.

BUG=639295, 638713
TBR=achuith (chrome/browser/chromeos/{app_mode,login,system})
TBR=reillyg (chrome/browser/extensions)
TBR=eroman (chrome/browser/net)
TBR=msw (chrome/browser/ui)
==========

to

==========
Remove unused email from install attributes and add support for AD.

Notable changes:
* Simplify attribute accessors by ensuring that attribute members
  are always self-consistent.
* Drop distinction between unset and empty attributes.
* Strict enforcement of *all* attributes being identical on
  re-locking.
* IsEnterpriseDevice() evaluates the device mode instead of
  the (deprecated) user.
* Improve encapsulation of StubInstallAttributes by disabling
  direct member access, provide high-level setters instead.

BUG=639295, 638713
TBR=achuith (chrome/browser/chromeos/{app_mode,login,system})
TBR=reillyg (chrome/browser/extensions)
TBR=eroman (chrome/browser/net)
TBR=bauerb (chrome/browser/ui/webui)
==========

[commit-bot: I haz the power, 2016-10-13 10:14:40]

Description was changed from

==========
Remove unused email from install attributes and add support for AD.

Notable changes:
* Simplify attribute accessors by ensuring that attribute members
  are always self-consistent.
* Drop distinction between unset and empty attributes.
* Strict enforcement of *all* attributes being identical on
  re-locking.
* IsEnterpriseDevice() evaluates the device mode instead of
  the (deprecated) user.
* Improve encapsulation of StubInstallAttributes by disabling
  direct member access, provide high-level setters instead.

BUG=639295, 638713
TBR=achuith (chrome/browser/chromeos/{app_mode,login,system})
TBR=reillyg (chrome/browser/extensions)
TBR=eroman (chrome/browser/net)
TBR=bauerb (chrome/browser/ui/webui)
==========

to

==========
Remove unused email from install attributes and add support for AD.

Notable changes:
* Simplify attribute accessors by ensuring that attribute members
  are always self-consistent.
* Drop distinction between unset and empty attributes.
* Strict enforcement of *all* attributes being identical on
  re-locking.
* IsEnterpriseDevice() evaluates the device mode instead of
  the (deprecated) user.
* Improve encapsulation of StubInstallAttributes by disabling
  direct member access, provide high-level setters instead.

BUG=639295, 638713
TBR=achuith (chrome/browser/chromeos/{app_mode,login,system})
TBR=reillyg (chrome/browser/extensions)
TBR=eroman (chrome/browser/net)
TBR=bauerb (chrome/browser/ui/webui)
==========

[commit-bot: I haz the power, 2016-10-13 10:14:42]

Committed patchset #2 (id:200001)

[commit-bot: I haz the power, 2016-10-13 10:16:45]

Description was changed from

==========
Remove unused email from install attributes and add support for AD.

Notable changes:
* Simplify attribute accessors by ensuring that attribute members
  are always self-consistent.
* Drop distinction between unset and empty attributes.
* Strict enforcement of *all* attributes being identical on
  re-locking.
* IsEnterpriseDevice() evaluates the device mode instead of
  the (deprecated) user.
* Improve encapsulation of StubInstallAttributes by disabling
  direct member access, provide high-level setters instead.

BUG=639295, 638713
TBR=achuith (chrome/browser/chromeos/{app_mode,login,system})
TBR=reillyg (chrome/browser/extensions)
TBR=eroman (chrome/browser/net)
TBR=bauerb (chrome/browser/ui/webui)
==========

to

==========
Remove unused email from install attributes and add support for AD.

Notable changes:
* Simplify attribute accessors by ensuring that attribute members
  are always self-consistent.
* Drop distinction between unset and empty attributes.
* Strict enforcement of *all* attributes being identical on
  re-locking.
* IsEnterpriseDevice() evaluates the device mode instead of
  the (deprecated) user.
* Improve encapsulation of StubInstallAttributes by disabling
  direct member access, provide high-level setters instead.

BUG=639295, 638713
TBR=achuith (chrome/browser/chromeos/{app_mode,login,system})
TBR=reillyg (chrome/browser/extensions)
TBR=eroman (chrome/browser/net)
TBR=bauerb (chrome/browser/ui/webui)

Committed: https://crrev.com/42d7ea0a129585044fa4461e7d581ad88e4525dc
Cr-Commit-Position: refs/heads/master@{#424994}
==========

[commit-bot: I haz the power, 2016-10-13 10:16:46]

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