Add more management policy checking after extension installed

chrome/browser/extensions/extension_service.h

 // Copyright (c) 2012 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 CHROME_BROWSER_EXTENSIONS_EXTENSION_SERVICE_H_
 #define CHROME_BROWSER_EXTENSIONS_EXTENSION_SERVICE_H_
 
 #include <map>
 #include <set>
 #include <string>
@@ skipping 530 matching lines @@
 
   // Disables the extension if the privilege level has increased
   // (e.g., due to an upgrade).
   void CheckPermissionsIncrease(const extensions::Extension* extension,
                                 bool is_extension_installed);
 
   // Helper that updates the active extension list used for crash reporting.
   void UpdateActiveExtensionsInCrashReporter();
 
   // Helper to determine whether we should initially enable an installed
-  // (or upgraded) extension.
-  bool ShouldEnableOnInstall(const extensions::Extension* extension);
+  // (or upgraded) extension. |disable_reason| will be ignored if NULL,

[not at google - send to devlin, 2014/11/12 18:02:15]

This comment is misleading, it should mention that disable_reason is an out
param. However, you could just return the disable reason from this method
instead. You may also want to rename the method to something more appropriate.

[binjin, 2014/11/12 18:39:01]

Done(editing the comments). Also this pointer is not supposed to be NULL, fixed
that in comments as well.

I'm not sure if it's a good idea to combine the boolean value and Disable
Reason, since DISABLE_NONE didn't imply enabling the extension.

[not at google - send to devlin, 2014/11/12 19:35:17]

It looks to me to be precisely the reason that this returns a bool.

|initial_enable| is set to true if the external extension was uninstalled. You
could set it to DISABLE_NONE instead.

|initial_enable| is set to false if the extension has requirement errors. You
could set it to DISABLE_UNSUPPORTED_REQUIEMENT instead.

etc.

[binjin, 2014/11/13 12:03:31]

I'm sorry, I think I didn't get your point. Do you mean that we should return a
DisableReason and use a pointer like |initial_enable| as out params for setting
a bool?

[not at google - send to devlin, 2014/11/13 17:41:21]

No I mean that you should change the return type of this function from a bool to
the DisableReason enum.

+  // otherwise contains why |extension| should not be enabled.
+  bool ShouldEnableOnInstall(
+      const extensions::Extension* extension,
+      extensions::Extension::DisableReason* disable_reason);
 
   // Helper to determine if updating an extensions should proceed immediately,
   // or if we should delay the update until further notice.
   bool ShouldDelayExtensionUpdate(const std::string& extension_id,
                                   bool install_immediately) const;
 
   // Manages the blacklisted extensions, intended as callback from
   // Blacklist::GetBlacklistedIDs.
   void ManageBlacklist(
       const extensions::Blacklist::BlacklistStateMap& blacklisted_ids);
@@ skipping 165 matching lines @@
   FRIEND_TEST_ALL_PREFIXES(ExtensionServiceTest,
                            WillNotLoadBlacklistedExtensionsFromDirectory);
   FRIEND_TEST_ALL_PREFIXES(ExtensionServiceTest, ReloadBlacklistedExtension);
   FRIEND_TEST_ALL_PREFIXES(ExtensionServiceTest, BlacklistedInPrefsFromStartup);
   FRIEND_TEST_ALL_PREFIXES(ExtensionServiceTest,
                            GreylistedExtensionDisabled);
   FRIEND_TEST_ALL_PREFIXES(ExtensionServiceTest,
                            GreylistDontEnableManuallyDisabled);
   FRIEND_TEST_ALL_PREFIXES(ExtensionServiceTest,
                            GreylistUnknownDontChange);
+  FRIEND_TEST_ALL_PREFIXES(ExtensionServiceTest,
+                           ManagementPolicyProhibitsEnableOnInstalled);
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionService);
 };
 
 #endif  // CHROME_BROWSER_EXTENSIONS_EXTENSION_SERVICE_H_