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

extensions/common/extension.h

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef EXTENSIONS_COMMON_EXTENSION_H_
 #define EXTENSIONS_COMMON_EXTENSION_H_
 
 #include <algorithm>
 #include <iosfwd>
 #include <map>
@@ skipping 152 matching lines @@
     REQUIRE_PERMISSIONS_CONSENT = 1 << 8,
 
     // Unused - this flag has been moved to ExtensionPrefs.
     IS_EPHEMERAL = 1 << 9,
 
     // |WAS_INSTALLED_BY_OEM| installed by an OEM (e.g on Chrome OS) and should
     // be placed in a special OEM folder in the App Launcher. Note: OEM apps are
     // 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.
   };
 
   // This is the highest bit index of the flags defined above.
   static const int kInitFromValueFlagBits;
 
   static scoped_refptr<Extension> Create(const base::FilePath& path,
                                          Manifest::Location location,
                                          const base::DictionaryValue& value,
                                          int flags,
@@ skipping 144 matching lines @@
   }
   int creation_flags() const { return creation_flags_; }
   bool from_webstore() const { return (creation_flags_ & FROM_WEBSTORE) != 0; }
   bool from_bookmark() const { return (creation_flags_ & FROM_BOOKMARK) != 0; }
   bool was_installed_by_default() const {
     return (creation_flags_ & WAS_INSTALLED_BY_DEFAULT) != 0;
   }
   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;
   bool is_platform_app() const;
   bool is_hosted_app() const;
   bool is_legacy_packaged_app() const;
   bool is_extension() const;
   bool can_be_incognito_enabled() const;
 
   void AddWebExtentPattern(const URLPattern& pattern);
@@ skipping 215 matching lines @@
 
   UpdatedExtensionPermissionsInfo(
       const Extension* extension,
       const PermissionSet* permissions,
       Reason reason);
 };
 
 }  // namespace extensions
 
 #endif  // EXTENSIONS_COMMON_EXTENSION_H_