OBSOLETE REVIEW. See http://codereview.chromium.org/10382149/

chrome/browser/extensions/extension_management_policy.h

Index: chrome/browser/extensions/extension_management_policy.h
===================================================================
--- chrome/browser/extensions/extension_management_policy.h	(revision 0)
+++ chrome/browser/extensions/extension_management_policy.h	(revision 0)
@@ -0,0 +1,80 @@
+// 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_MANAGEMENT_POLICY_H_
+#define CHROME_BROWSER_EXTENSIONS_EXTENSION_MANAGEMENT_POLICY_H_
+#pragma once
+
+#include <set>
+
+#include "base/basictypes.h"
+#include "chrome/common/extensions/extension.h"
+
+// Combiner. FIXME comment.
+class ExtensionManagementPolicy {

[Aaron Boodman, 2012/05/17 22:41:06]

extensions::ManagementPolicy - then you can make the filename shorter, too.

+ public:
+  class Delegate {

[Aaron Boodman, 2012/05/17 22:41:06]

A class can typically only have a single delegate. PolicyProvider?

[Pam (message me for reviews), 2012/05/18 20:15:15]

There's precedent in the list of delegates in resource_queue.cc. I'm open to
other names, but I want to avoid (even more) confusion with admin policy and
those policy providers. "Source"? Or "Provider", and give up on not overloading
the names? I'll try "Provider" and a comment.

+   public:
+    Delegate() {}
+    virtual ~Delegate() {}
+
+    // FIXME all comments
+    virtual bool UserMayInstall(const std::string& extension_id,
+                                Extension::Location location,
+                                const std::string& extension_name,
+                                string16* error) const;
+
+    // Enable, disable, remove
+    virtual bool UserMayModifyStatus(const Extension* extension,
+                                     string16* error) const;
+
+    // Incognito, file access, etc.
+    virtual bool UserMayModifyUsage(const Extension* extension,

[Aaron Boodman, 2012/05/17 22:41:06]

ModifySettings?

+                                    string16* error) const;
+
+    // Forced on; you probably also want to set the others
+    virtual bool MustRemainEnabled(const Extension* extension,

[Aaron Boodman, 2012/05/17 22:41:06]

It seems like these can be simplified:

// Better name needed.
enum ExtensionRunDecision {
  DEFAULT,  // It's up to the user
  EXTENSION_MUST_RUN,
  EXTENSION_MUST_NOT_RUN,
};

ExtensionRunDecision GetRunDecision(const std::string& extension_id);
bool CanUserModifySettings(const std::string& extension_id);

If an extension must run, that means it can't be disabled or removed, so there's
no need for those to be called out separately.

If it must not run, that means it can't be installed or loaded (if it was
previously installed).

[Pam (message me for reviews), 2012/05/18 20:15:15]

That would be enough for policy force- and blacklists. But there's also the case
where an extension can either run or not run, but whichever it is, the user
can't change it. That's what managed mode will be applying.

I don't like much MustRemainEnabled() as an outlier, but it's the one that
differs between policy forcelist and managed mode.

Looking at the callers, though, UserMayModifyStatus and UserMayModifyUsage are
mutually redundant. I'll combine them.

+                                   string16* error) const;
+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(Delegate);
+  };
+
+  ExtensionManagementPolicy();
+  ~ExtensionManagementPolicy();
+
+  // Does not take ownership
+  void RegisterDelegate(Delegate* delegate);
+  void UnregisterDelegate(Delegate* delegate);
+
+  // FIXME all comments - combiners
+  bool UserMayInstall(const std::string& extension_id,
+                      Extension::Location location,
+                      const std::string& extension_name,
+                      string16* error) const;
+
+  // Enable, disable, remove
+  bool UserMayModifyStatus(const Extension* extension,
+                           string16* error) const;
+
+  // Incognito, file access, etc.
+  bool UserMayModifyUsage(const Extension* extension,
+                          string16* error) const;
+
+  // Forced on
+  bool MustRemainEnabled(const Extension* extension,
+                         string16* error) const;
+
+  // For use in testing.
+  void UnregisterAllDelegates();
+  size_t CountDelegates();

[Aaron Boodman, 2012/05/17 22:41:06]

Use int.

[Aaron Boodman, 2012/05/17 22:41:06]

GetNumDelegates().

+
+ private:
+  typedef std::set<Delegate*> DelegateList;
+  DelegateList delegates_;
+
+  DISALLOW_COPY_AND_ASSIGN(ExtensionManagementPolicy);
+};
+
+#endif  // CHROME_BROWSER_EXTENSIONS_EXTENSION_MANAGEMENT_POLICY_H_
Property changes on: chrome\browser\extensions\extension_management_policy.h
___________________________________________________________________
Added: svn:eol-style
   + LF