Apps&Extensions for supervised users: Add Extension::WAS_INSTALLED_BY_CUSTODIAN flag and proto entry

extensions/common/extension.h

Index: extensions/common/extension.h
diff --git a/extensions/common/extension.h b/extensions/common/extension.h
index cf5e85ca490988918947d3729c492fac7297c1c4..71e58e5ddb5549951b1d420d432e33d2f9311d05 100644
--- a/extensions/common/extension.h
+++ b/extensions/common/extension.h
@@ -170,6 +170,10 @@ class Extension : public base::RefCountedThreadSafe<Extension> {
     // also installed by Default (i.e. WAS_INSTALLED_BY_DEFAULT is also true).
     WAS_INSTALLED_BY_OEM = 1 << 10,
 
+    // |WAS_INSTALLED_BY_CUSTODIAN| means this extension was installed by the
+    // custodian of a supervised user.

[not at google - send to devlin, 2014/07/01 17:22:47]

this would be better as a flag in ExtensionPrefs

[not at google - send to devlin, 2014/07/01 17:23:30]

like
https://code.google.com/p/chromium/codesearch#chromium/src/extensions/browser...

[Marc Treib, 2014/07/02 11:58:12]

The problem with that approach is that ExtensionPrefs are populated only after a
successful installation, but I want to use this flag to check if the
installation should be allowed in the first place (in
SupervisedUserService::UserMayLoad). Populating just this one pref before the
installation is probably possible, but seems like a bad idea - WDYT?

[not at google - send to devlin, 2014/07/02 16:18:56]

I see. I agree that writing something to preferences *before* installation would
be wrong.

Ideally many of these flags wouldn't be here, though. They're concerned with how
the environment interacts with the extension, not anything to do with the
extension model. I'd rather prevent this InitFromValueFlags enum from steadily
growing as people need to attach flags for their feature. 

Perhaps you can pass through the InstallFlag to the UserMayLoad method? I put a
comment on an example of where another place it'd be nice. I'm a little worried
that the end result here is threading through an install flag basically
everywhere, and that would be bad.

Perhaps we should be passing around PendingExtensionInfo objects (and
incidentally PendingExtensionInfo should be holding onto an InstallFlag not all
of those bools), not Extensions? That actually seems more correct in the case of
deciding if an Extension can be installed.

Any ideas?

[Marc Treib, 2014/07/07 08:58:17]

At the time UserMayLoad is called, the extension may or may not be already
installed, so passing around PendingExtensionInfos or InstallFlags doesn't seem
right either.
I can't see a good way forward here - maybe you can add someone more
knowledgeable about the extension system than me, so we'll get some ideas?

[Bernhard Bauer, 2014/07/07 09:11:58]

Does it actually ever make sense to call UserMayLoad for an extension that is
already installed? Maybe we could catch those cases before calling out to
UserMayLoad.

[Marc Treib, 2014/07/07 09:20:04]

E.g. on a policy change, all existing (installed) extensions should be checked
again. And maybe there are other cases?

[Yoyo Zhou, 2014/07/08 01:55:11]

This already happens. We re-check on startup in case the policy blacklist has
changed. See installed_loader.cc:172. We also check whenever we call
ExtensionService::CheckManagementPolicy (which apparently is also only at
startup).

Generally speaking, the install flag seems fine to me for install time from sync
(there are no real good alternatives), and as kalman says, a pref in
ExtensionPrefs is what we should check at load time.

+    WAS_INSTALLED_BY_CUSTODIAN = 1 << 11,
+
     // When adding new flags, make sure to update kInitFromValueFlagBits.
   };
 
@@ -334,6 +338,9 @@ class Extension : public base::RefCountedThreadSafe<Extension> {
   bool was_installed_by_oem() const {
     return (creation_flags_ & WAS_INSTALLED_BY_OEM) != 0;
   }
+  bool was_installed_by_custodian() const {
+    return (creation_flags_ & WAS_INSTALLED_BY_CUSTODIAN) != 0;
+  }
 
   // App-related.
   bool is_app() const;