Extensions: run "custom bindings" v8-extensions in content scripts.

Currently they're only run in "privileged" extension processes (e.g. background pages).

As part of this, plug some places where API bindings are set up unnecessarily in content scripts: firstly in ExtensionDispatcher, where the "custom bindings" v8-extensions are allowed to run in content scripts, and secondly in SchemaGeneratedBindings where schemas for privileged APIs are sent to content scripts.


BUG=80310
TEST=browser_tests


Review URL: http://codereview.chromium.org/9403006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@122243 0039d316-1c4b-4281-b951-d872f2087c98

12 files changed, 272 insertions, 120 deletions

diff --git a/chrome/common/extensions/api/extension_api.cc b/chrome/common/extensions/api/extension_api.cc
index 6a728b8..0f4956e 100644
--- a/chrome/common/extensions/api/extension_api.cc
+++ b/chrome/common/extensions/api/extension_api.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/common/extensions/api/extension_api.h"
 
+#include <algorithm>
 #include <string>
 #include <vector>
 
@@ -37,6 +38,26 @@ void GetMissingDependencies(
   }
 }
 
+// Returns whether the list at |name_space_node|.|child_kind| contains any
+// children with an { "unprivileged": true } property.
+bool HasUnprivilegedChild(const DictionaryValue* name_space_node,
+                          const std::string& child_kind) {
+  ListValue* child_list = NULL;
+  name_space_node->GetList(child_kind, &child_list);
+  if (!child_list)
+    return false;
+
+  for (size_t i = 0; i < child_list->GetSize(); ++i) {
+    DictionaryValue* item = NULL;
+    CHECK(child_list->GetDictionary(i, &item));
+    bool unprivileged = false;
+    if (item->GetBoolean("unprivileged", &unprivileged))
+      return unprivileged;
+  }
+
+  return false;
+}
+
 }  // namespace
 
 // static
@@ -137,31 +158,59 @@ ExtensionAPI::ExtensionAPI() {
   for (size_t i = 0; i < arraysize(kJsonApiResourceIds); i++) {
     LoadSchemaFromResource(kJsonApiResourceIds[i]);
   }
+
+  // Populate {completely,partially}_unprivileged_apis_.
+  for (SchemaMap::iterator it = schemas_.begin(); it != schemas_.end(); ++it) {
+    bool unprivileged = false;
+    it->second->GetBoolean("unprivileged", &unprivileged);
+    if (unprivileged) {
+      completely_unprivileged_apis_.insert(it->first);
+      continue;
+    }
+
+    // Only need to look at functions/events; even though there are unprivileged
+    // properties (e.g. in extensions), access to those never reaches C++ land.
+    if (HasUnprivilegedChild(it->second.get(), "functions") ||
+        HasUnprivilegedChild(it->second.get(), "events")) {
+      partially_unprivileged_apis_.insert(it->first);
+    }
+  }
 }
 
 ExtensionAPI::~ExtensionAPI() {
 }
 
 bool ExtensionAPI::IsPrivileged(const std::string& full_name) const {
-  std::vector<std::string> split_full_name;
-  base::SplitString(full_name, '.', &split_full_name);
-  std::string name = split_full_name.back();
-  split_full_name.pop_back();
-  std::string name_space = JoinString(split_full_name, '.');
-
-  // HACK(kalman): explicitly mark all Storage API methods as unprivileged.
-  // TODO(kalman): solve this in a more general way; the problem is that
-  // functions-on-properties are not found with the following algorithm.
-  if (name_space == "storage")
-    return false;
+  std::string api_name;
+  std::string child_name;
 
-  const base::DictionaryValue* name_space_node = GetSchema(name_space);
-  if (!name_space_node)
-    return true;
+  {
+    std::vector<std::string> split;
+    base::SplitString(full_name, '.', &split);
+    std::reverse(split.begin(), split.end());
 
-  if (!IsChildNamePrivileged(name_space_node, "functions", name) ||
-      !IsChildNamePrivileged(name_space_node, "events", name)) {
+    api_name = split.back();
+    split.pop_back();
+    if (api_name == "experimental") {
+      api_name += "." + split.back();
+      split.pop_back();
+    }
+
+    // This only really works properly if split.size() == 1, however:
+    //  - if it's empty, it's ok to leave child_name empty; presumably there's
+    //    no functions or events with empty names.
+    //  - if it's > 1, we can just do our best.
+    if (split.size() > 0)
+      child_name = split[0];
+  }
+
+  if (completely_unprivileged_apis_.count(api_name))
     return false;
+
+  if (partially_unprivileged_apis_.count(api_name)) {
+    const DictionaryValue* schema = GetSchema(api_name);
+    return IsChildNamePrivileged(schema, "functions", child_name) &&
+           IsChildNamePrivileged(schema, "events", child_name);
   }
 
   return true;
@@ -243,4 +292,9 @@ void ExtensionAPI::GetSchemasForPermissions(
   }
 }
 
+bool ExtensionAPI::IsWholeAPIPrivileged(const std::string& api_name) const {
+  return !completely_unprivileged_apis_.count(api_name) &&
+         !partially_unprivileged_apis_.count(api_name);
 }
+
+}  // namespace extensions
diff --git a/chrome/common/extensions/api/extension_api.h b/chrome/common/extensions/api/extension_api.h
index cd97f44..4ba7c20 100644
--- a/chrome/common/extensions/api/extension_api.h
+++ b/chrome/common/extensions/api/extension_api.h
@@ -7,6 +7,7 @@
 #pragma once
 
 #include <map>
+#include <set>
 #include <string>
 
 #include "base/basictypes.h"
@@ -33,12 +34,17 @@ class ExtensionAPI {
   // Returns the single instance of this class.
   static ExtensionAPI* GetInstance();
 
-  // Returns ture if |name| is a privileged API. Privileged APIs can only be
-  // called from extension code which is running in its own designated extension
-  // process. They cannot be called from extension code running in content
-  // scripts, or other low-privileged processes.
+  // Returns true if |name| is a privileged API path. Privileged paths can only
+  // be called from extension code which is running in its own designated
+  // extension process. They cannot be called from extension code running in
+  // content scripts, or other low-privileged contexts.
   bool IsPrivileged(const std::string& name) const;
 
+  // Returns whether *every* path in the API is privileged. This will be false
+  // for APIs such as "storage" which is entirely unprivileged, and "test"
+  // which has unprivileged components.
+  bool IsWholeAPIPrivileged(const std::string& api_name) const;
+
   // Gets a map of API name (aka namespace) to API schema.
   const SchemaMap& schemas() { return schemas_; }
 
@@ -88,6 +94,12 @@ class ExtensionAPI {
   // Schemas for each namespace.
   SchemaMap schemas_;
 
+  // APIs that are entirely unprivileged.
+  std::set<std::string> completely_unprivileged_apis_;
+
+  // APIs that are not entirely unprivileged, but have unprivileged components.
+  std::set<std::string> partially_unprivileged_apis_;
+
   DISALLOW_COPY_AND_ASSIGN(ExtensionAPI);
 };
 
diff --git a/chrome/common/extensions/api/extension_api_unittest.cc b/chrome/common/extensions/api/extension_api_unittest.cc
index 66c76f5..ccddaac 100644
--- a/chrome/common/extensions/api/extension_api_unittest.cc
+++ b/chrome/common/extensions/api/extension_api_unittest.cc
@@ -31,6 +31,31 @@ TEST(ExtensionAPI, IsPrivileged) {
   // Exists, but privileged.
   EXPECT_TRUE(extension_api->IsPrivileged("extension.getViews"));
   EXPECT_TRUE(extension_api->IsPrivileged("history.search"));
+
+  // Whole APIs that are unprivileged.
+  EXPECT_FALSE(extension_api->IsPrivileged("storage.local"));
+  EXPECT_FALSE(extension_api->IsPrivileged("storage.local.onChanged"));
+  EXPECT_FALSE(extension_api->IsPrivileged("storage.local.set"));
+  EXPECT_FALSE(extension_api->IsPrivileged("storage.local.MAX_ITEMS"));
+  EXPECT_FALSE(extension_api->IsPrivileged("storage.set"));
+}
+
+TEST(ExtensionAPI, IsWholeAPIPrivileged) {
+  ExtensionAPI* extension_api = ExtensionAPI::GetInstance();
+
+  // Completely unprivileged.
+  EXPECT_FALSE(extension_api->IsWholeAPIPrivileged("storage"));
+
+  // Partially unprivileged.
+  EXPECT_FALSE(extension_api->IsWholeAPIPrivileged("extension"));
+  EXPECT_FALSE(extension_api->IsWholeAPIPrivileged("test"));
+
+  // Nothing unprivileged.
+  EXPECT_TRUE(extension_api->IsWholeAPIPrivileged("history"));
+
+  // Paranoid above... paranoid here, too.
+  EXPECT_TRUE(extension_api->IsWholeAPIPrivileged(""));
+  EXPECT_TRUE(extension_api->IsWholeAPIPrivileged("<unknown-namespace>"));
 }
 
 TEST(ExtensionAPI, Depends) {
@@ -47,7 +72,7 @@ TEST(ExtensionAPI, Depends) {
 
   std::string error;
   scoped_refptr<Extension> extension(Extension::Create(
-      FilePath(), Extension::LOAD, manifest, Extension::NO_FLAGS, "x", &error));
+      FilePath(), Extension::LOAD, manifest, Extension::NO_FLAGS, &error));
   CHECK(extension.get());
   CHECK(error.empty());
 
diff --git a/chrome/common/extensions/api/storage.json b/chrome/common/extensions/api/storage.json
index 63309bb..c26c943 100644
--- a/chrome/common/extensions/api/storage.json
+++ b/chrome/common/extensions/api/storage.json
@@ -1,6 +1,7 @@
 [
   {
     "namespace": "storage",
+    "unprivileged": true,
     "types": [
       {
         "id": "StorageChange",
@@ -24,7 +25,6 @@
         "functions": [
           {
             "name": "get",
-            "unprivileged": true,
             "type": "function",
             "description": "Gets one or more items from storage.",
             "parameters": [
@@ -61,7 +61,6 @@
           },
           {
             "name": "getBytesInUse",
-            "unprivileged": true,
             "type": "function",
             "description": "Gets the amount of space (in bytes) being used by one or more items.",
             "parameters": [
@@ -90,7 +89,6 @@
           },
           {
             "name": "set",
-            "unprivileged": true,
             "type": "function",
             "description": "Sets multiple items.",
             "parameters": [
@@ -112,7 +110,6 @@
           },
           {
             "name": "remove",
-            "unprivileged": true,
             "type": "function",
             "description": "Removes one or more items from storage.",
             "parameters": [
@@ -135,7 +132,6 @@
           },
           {
             "name": "clear",
-            "unprivileged": true,
             "type": "function",
             "description": "Removes all items from storage.",
             "parameters": [
@@ -154,7 +150,6 @@
     "events": [
       {
         "name": "onChanged",
-        "unprivileged": true,
         "type": "function",
         "description": "Fired when one or more items change.",
         "parameters": [
@@ -177,25 +172,21 @@
       "sync": {
         "$ref": "StorageArea",
         "description": "Items under the \"sync\" namespace are synced using Chrome Sync.",
-        "unprivileged": true,
         "value": [ "sync" ],
         "properties": {
           "QUOTA_BYTES": {
             "type": "integer",
             "value": "102400",
-            "unprivileged": true,
             "description": "The maximum total amount (in bytes) of data that can be stored in sync storage."
           },
           "QUOTA_BYTES_PER_ITEM": {
             "type": "integer",
             "value": "2048",
-            "unprivileged": true,
             "description": "The maximum size (in bytes) of each individual item in sync storage."
           },
           "MAX_ITEMS": {
             "type": "integer",
             "value": "512",
-            "unprivileged": true,
             "description": "The maximum number of items that can be stored in sync storage."
           }
         }
@@ -203,13 +194,11 @@
       "local": {
         "$ref": "StorageArea",
         "description": "Items under the \"local\" namespace are local to each machine.",
-        "unprivileged": true,
         "value": [ "local" ],
         "properties": {
           "QUOTA_BYTES": {
             "type": "integer",
             "value": "5120000",
-            "unprivileged": true,
             "description": "The maximum amount (in bytes) of data that can be stored in local storage. This value may be ignored if the extension has the <code>unlimitedStorage</code> permission."
           }
         }
diff --git a/chrome/renderer/extensions/chrome_v8_context.cc b/chrome/renderer/extensions/chrome_v8_context.cc
index 98b0c07..3e214f5 100644
--- a/chrome/renderer/extensions/chrome_v8_context.cc
+++ b/chrome/renderer/extensions/chrome_v8_context.cc
@@ -22,18 +22,30 @@ const char kChromeHidden[] = "chromeHidden";
 const char kValidateCallbacks[] = "validateCallbacks";
 #endif
 
-}  // namespace
+std::string GetContextTypeDescription(
+    ChromeV8Context::ContextType context_type) {
+  switch (context_type) {
+    case ChromeV8Context::OTHER:          return "other";
+    case ChromeV8Context::CONTENT_SCRIPT: return "content script";
+  }
+  NOTREACHED();
+  return "";
+}
 
+}  // namespace
 
 ChromeV8Context::ChromeV8Context(v8::Handle<v8::Context> v8_context,
                                  WebKit::WebFrame* web_frame,
-                                 const std::string& extension_id)
+                                 const std::string& extension_id,
+                                 ChromeV8Context::ContextType context_type)
     : v8_context_(v8::Persistent<v8::Context>::New(v8_context)),
       web_frame_(web_frame),
-      extension_id_(extension_id) {
+      extension_id_(extension_id),
+      context_type_(context_type) {
   VLOG(1) << "Created context for extension\n"
-          << "  id:    " << extension_id << "\n"
-          << "  frame: " << web_frame_;
+          << "  id:           " << extension_id << "\n"
+          << "  frame:        " << web_frame_ << "\n"
+          << "  context type: " << GetContextTypeDescription(context_type);
 }
 
 ChromeV8Context::~ChromeV8Context() {
diff --git a/chrome/renderer/extensions/chrome_v8_context.h b/chrome/renderer/extensions/chrome_v8_context.h
index 19165ab..11a4a87 100644
--- a/chrome/renderer/extensions/chrome_v8_context.h
+++ b/chrome/renderer/extensions/chrome_v8_context.h
@@ -26,9 +26,19 @@ class RenderView;
 // we won't need this object and it's a bit less state to keep track of.
 class ChromeV8Context {
  public:
+  enum ContextType {
+    CONTENT_SCRIPT,
+
+    // TODO(kalman): for now, have this as OTHER, since we only currently need
+    // know whether something is a content script or not. However, when
+    // necessary this should enumerate the other types, such as FRAME.
+    OTHER
+  };
+
   ChromeV8Context(v8::Handle<v8::Context> context,
                   WebKit::WebFrame* frame,
-                  const std::string& extension_id);
+                  const std::string& extension_id,
+                  ContextType context_type);
   ~ChromeV8Context();
 
   v8::Handle<v8::Context> v8_context() const {
@@ -46,6 +56,10 @@ class ChromeV8Context {
     web_frame_ = NULL;
   }
 
+  ContextType context_type() const {
+    return context_type_;
+  }
+
   // Returns a special Chrome-specific hidden object that is associated with a
   // context, but not reachable from the JavaScript in that context. This is
   // used by our v8::Extension implementations as a way to share code and as a
@@ -96,6 +110,9 @@ class ChromeV8Context {
   // The extension ID this context is associated with.
   std::string extension_id_;
 
+  // The type of context.
+  ContextType context_type_;
+
   DISALLOW_COPY_AND_ASSIGN(ChromeV8Context);
 };
 
diff --git a/chrome/renderer/extensions/chrome_v8_context_set_unittest.cc b/chrome/renderer/extensions/chrome_v8_context_set_unittest.cc
index 54a2400..56f4697 100644
--- a/chrome/renderer/extensions/chrome_v8_context_set_unittest.cc
+++ b/chrome/renderer/extensions/chrome_v8_context_set_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -22,7 +22,8 @@ TEST(ChromeV8ContextSet, Lifecycle) {
   WebKit::WebFrame* frame = reinterpret_cast<WebKit::WebFrame*>(1);
   std::string extension_id = "00000000000000000000000000000000";
   ChromeV8Context* context =
-      new ChromeV8Context(v8_context, frame, extension_id);
+      new ChromeV8Context(
+          v8_context, frame, extension_id, ChromeV8Context::OTHER);
 
   context_set.Add(context);
   EXPECT_EQ(1u, context_set.GetAll().count(context));
diff --git a/chrome/renderer/extensions/custom_bindings_util.cc b/chrome/renderer/extensions/custom_bindings_util.cc
index 5d9277f..28ce8f7 100644
--- a/chrome/renderer/extensions/custom_bindings_util.cc
+++ b/chrome/renderer/extensions/custom_bindings_util.cc
@@ -8,6 +8,7 @@
 
 #include "base/logging.h"
 #include "base/string_util.h"
+#include "chrome/common/extensions/api/extension_api.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/renderer/extensions/chrome_v8_extension.h"
 #include "chrome/renderer/extensions/chrome_private_custom_bindings.h"
@@ -144,13 +145,20 @@ std::string GetAPIName(const std::string& v8_extension_name) {
 }
 
 bool AllowAPIInjection(const std::string& api_name,
-                       const Extension& extension) {
+                       const Extension& extension,
+                       ChromeV8Context::ContextType context_type) {
   CHECK(api_name != "");
 
   // As in ExtensionAPI::GetSchemasForExtension, we need to allow any bindings
   // for an API that the extension *might* have permission to use.
-  return extension.required_permission_set()->HasAnyAccessToAPI(api_name) ||
-         extension.optional_permission_set()->HasAnyAccessToAPI(api_name);
+  bool allowed =
+      extension.required_permission_set()->HasAnyAccessToAPI(api_name) ||
+      extension.optional_permission_set()->HasAnyAccessToAPI(api_name);
+
+  if (allowed && context_type == ChromeV8Context::CONTENT_SCRIPT)
+    allowed = !ExtensionAPI::GetInstance()->IsWholeAPIPrivileged(api_name);
+
+  return allowed;
 }
 
 }  // namespace custom_bindings_util
diff --git a/chrome/renderer/extensions/custom_bindings_util.h b/chrome/renderer/extensions/custom_bindings_util.h
index 8b0b429..8746321 100644
--- a/chrome/renderer/extensions/custom_bindings_util.h
+++ b/chrome/renderer/extensions/custom_bindings_util.h
@@ -9,6 +9,7 @@
 #include <string>
 #include <vector>
 
+#include "chrome/renderer/extensions/chrome_v8_context.h"
 #include "chrome/renderer/extensions/chrome_v8_extension.h"
 
 class Extension;
@@ -33,10 +34,12 @@ std::vector<v8::Extension*> GetAll(ExtensionDispatcher* extension_dispatcher);
 std::string GetAPIName(const std::string& v8_extension_name);
 
 // Returns whether the custom binding for an API should be allowed to run for
-// |extension|.  This is based on whether the extension has any permission
-// (required or optional) for that API.
+// |extension|. This is based on whether the extension has any permission
+// (required or optional) for that API, and what context the APIs are intended
+// to run in.
 bool AllowAPIInjection(const std::string& api_name,
-                       const Extension& extension);
+                       const Extension& extension,
+                       ChromeV8Context::ContextType context_type);
 
 }  // namespace custom_bindings_util
 
diff --git a/chrome/renderer/extensions/extension_dispatcher.cc b/chrome/renderer/extensions/extension_dispatcher.cc
index 0d4169b..458e7d4 100644
--- a/chrome/renderer/extensions/extension_dispatcher.cc
+++ b/chrome/renderer/extensions/extension_dispatcher.cc
@@ -35,8 +35,16 @@
 #include "v8/include/v8.h"
 
 namespace {
+
 static const int64 kInitialExtensionIdleHandlerDelayMs = 5*1000;
 static const int64 kMaxExtensionIdleHandlerDelayMs = 5*60*1000;
+
+ChromeV8Context::ContextType ExtensionGroupToContextType(int extension_group) {
+  if (extension_group == EXTENSION_GROUP_CONTENT_SCRIPTS)
+    return ChromeV8Context::CONTENT_SCRIPT;
+  return ChromeV8Context::OTHER;
+}
+
 }
 
 using namespace extensions;
@@ -272,11 +280,25 @@ bool ExtensionDispatcher::AllowScriptExtension(
   return AllowScriptExtension(frame, v8_extension_name, extension_group, 0);
 }
 
+namespace {
+
+// This is what the extension_group variable will be when DidCreateScriptContext
+// is called. We know because it's the same as what AllowScriptExtension gets
+// passed, and the two functions are called sequentially from WebKit.
+//
+// TODO(koz): Plumb extension_group through to AllowScriptExtension() from
+// WebKit.
+static int hack_DidCreateScriptContext_extension_group = 0;
+
+}
+
 bool ExtensionDispatcher::AllowScriptExtension(
     WebFrame* frame,
     const std::string& v8_extension_name,
     int extension_group,
     int world_id) {
+  hack_DidCreateScriptContext_extension_group = extension_group;
+
   // NULL in unit tests.
   if (!RenderThread::Get())
     return true;
@@ -291,7 +313,10 @@ bool ExtensionDispatcher::AllowScriptExtension(
 
   // Extension-only bindings should be restricted to content scripts and
   // extension-blessed URLs.
-  if (extension_group == EXTENSION_GROUP_CONTENT_SCRIPTS ||
+  ChromeV8Context::ContextType context_type =
+      ExtensionGroupToContextType(extension_group);
+
+  if (context_type == ChromeV8Context::CONTENT_SCRIPT ||
       extensions_.ExtensionBindingsAllowed(ExtensionURLInfo(
           frame->document().securityOrigin(),
           UserScriptSlave::GetDataSourceURLForFrame(frame)))) {
@@ -315,7 +340,7 @@ bool ExtensionDispatcher::AllowScriptExtension(
         return false;
       }
       return custom_bindings_util::AllowAPIInjection(
-          custom_binding_api_name, *extension);
+          custom_binding_api_name, *extension, context_type);
     }
 
     return true;
@@ -327,7 +352,12 @@ bool ExtensionDispatcher::AllowScriptExtension(
 void ExtensionDispatcher::DidCreateScriptContext(
     WebFrame* frame, v8::Handle<v8::Context> v8_context, int world_id) {
   ChromeV8Context* context =
-      new ChromeV8Context(v8_context, frame, GetExtensionID(frame, world_id));
+      new ChromeV8Context(
+          v8_context,
+          frame,
+          GetExtensionID(frame, world_id),
+          ExtensionGroupToContextType(
+              hack_DidCreateScriptContext_extension_group));
   v8_context_set_.Add(context);
 
   const Extension* extension = extensions_.GetByID(context->extension_id());
diff --git a/chrome/renderer/extensions/schema_generated_bindings.cc b/chrome/renderer/extensions/schema_generated_bindings.cc
index c05d797..a8a5f75 100644
--- a/chrome/renderer/extensions/schema_generated_bindings.cc
+++ b/chrome/renderer/extensions/schema_generated_bindings.cc
@@ -160,7 +160,13 @@ class ExtensionImpl : public ChromeV8Extension {
     size_t api_index = 0;
     for (ExtensionAPI::SchemaMap::iterator it = schemas.begin();
         it != schemas.end(); ++it) {
-      api->Set(api_index, GetV8SchemaForAPI(self, context, it->first));
+      std::string api_name = it->first;
+      // For content scripts, only allow APIs that have unprivileged components.
+      if (v8_context->context_type() == ChromeV8Context::CONTENT_SCRIPT &&
+          ExtensionAPI::GetInstance()->IsWholeAPIPrivileged(api_name)) {
+        continue;
+      }
+      api->Set(api_index, GetV8SchemaForAPI(self, context, api_name));
       ++api_index;
     }
 
diff --git a/chrome/renderer/resources/extensions/schema_generated_bindings.js b/chrome/renderer/resources/extensions/schema_generated_bindings.js
index df12690..ae53c85 100644
--- a/chrome/renderer/resources/extensions/schema_generated_bindings.js
+++ b/chrome/renderer/resources/extensions/schema_generated_bindings.js
@@ -404,32 +404,34 @@ var chrome = chrome || {};
         });
       }
 
+      // Returns whether access to the content of a schema should be denied,
+      // based on the presence of "unprivileged" and whether this is an
+      // extension process (versus e.g. a content script).
+      function isSchemaAccessAllowed(itemSchema) {
+        return isExtensionProcess ||
+               apiDef.unprivileged ||
+               itemSchema.unprivileged;
+      }
+
       // Adds a getter that throws an access denied error to object |module|
       // for property |name|.
-      //
-      // Returns true if the getter was necessary (access is disallowed), or
-      // false otherwise.
-      function addUnprivilegedAccessGetter(module, name, allowUnprivileged) {
-        if (allowUnprivileged || isExtensionProcess)
-          return false;
-
+      function addUnprivilegedAccessGetter(module, name) {
         module.__defineGetter__(name, function() {
           throw new Error(
               '"' + name + '" can only be used in extension processes. See ' +
               'the content scripts documentation for more details.');
         });
-        return true;
       }
 
       // Setup Functions.
       if (apiDef.functions) {
         apiDef.functions.forEach(function(functionDef) {
+          if (functionDef.name in module)
+            return;
           if (!isSchemaNodeSupported(functionDef, platform, manifestVersion))
             return;
-
-          if (functionDef.name in module ||
-              addUnprivilegedAccessGetter(module, functionDef.name,
-                                          functionDef.unprivileged)) {
+          if (!isSchemaAccessAllowed(functionDef)) {
+            addUnprivilegedAccessGetter(module, functionDef.name);
             return;
           }
 
@@ -469,12 +471,12 @@ var chrome = chrome || {};
       // Setup Events
       if (apiDef.events) {
         apiDef.events.forEach(function(eventDef) {
+          if (eventDef.name in module)
+            return;
           if (!isSchemaNodeSupported(eventDef, platform, manifestVersion))
             return;
-
-          if (eventDef.name in module ||
-              addUnprivilegedAccessGetter(module, eventDef.name,
-                                          eventDef.unprivileged)) {
+          if (!isSchemaAccessAllowed(eventDef)) {
+            addUnprivilegedAccessGetter(module, eventDef.name);
             return;
           }
 
@@ -491,66 +493,59 @@ var chrome = chrome || {};
         });
       }
 
-      function addProperties(m, def) {
-        // Parse any values defined for properties.
-        if (def.properties) {
-          forEach(def.properties, function(prop, property) {
-            if (!isSchemaNodeSupported(property, platform, manifestVersion))
-              return;
+      function addProperties(m, parentDef) {
+        var properties = parentDef.properties;
+        if (!properties)
+          return;
 
-            if (prop in m ||
-                addUnprivilegedAccessGetter(m, prop, property.unprivileged)) {
-              return;
-            }
+        forEach(properties, function(propertyName, propertyDef) {
+          if (propertyName in m)
+            return;
+          if (!isSchemaNodeSupported(propertyDef, platform, manifestVersion))
+            return;
+          if (!isSchemaAccessAllowed(propertyDef)) {
+            addUnprivilegedAccessGetter(m, propertyName);
+            return;
+          }
 
-            var value = property.value;
-            if (value) {
-              if (property.type === 'integer') {
-                value = parseInt(value);
-              } else if (property.type === 'boolean') {
-                value = value === "true";
-              } else if (property["$ref"]) {
-                var constructor = customTypes[property["$ref"]];
-                if (!constructor)
-                  throw new Error("No custom binding for " + property["$ref"]);
-                var args = value;
-                // For an object property, |value| is an array of constructor
-                // arguments, but we want to pass the arguments directly
-                // (i.e. not as an array), so we have to fake calling |new| on
-                // the constructor.
-                value = { __proto__: constructor.prototype };
-                constructor.apply(value, args);
-                // Recursively add properties.
-                addProperties(value, property);
-              } else if (property.type === 'object') {
-                // Recursively add properties.
-                addProperties(value, property);
-              } else if (property.type !== 'string') {
-                throw "NOT IMPLEMENTED (extension_api.json error): Cannot " +
-                    "parse values for type \"" + property.type + "\"";
-              }
-            }
-            if (value) {
-              m[prop] = value;
+          var value = propertyDef.value;
+          if (value) {
+            if (propertyDef.type === 'integer') {
+              value = parseInt(value);
+            } else if (propertyDef.type === 'boolean') {
+              value = value === "true";
+            } else if (propertyDef["$ref"]) {
+              var constructor = customTypes[propertyDef["$ref"]];
+              if (!constructor)
+                throw new Error("No custom binding for " + propertyDef["$ref"]);
+              var args = value;
+              // For an object propertyDef, |value| is an array of constructor
+              // arguments, but we want to pass the arguments directly
+              // (i.e. not as an array), so we have to fake calling |new| on
+              // the constructor.
+              value = { __proto__: constructor.prototype };
+              constructor.apply(value, args);
+              // Recursively add properties.
+              addProperties(value, propertyDef);
+            } else if (propertyDef.type === 'object') {
+              // Recursively add properties.
+              addProperties(value, propertyDef);
+            } else if (propertyDef.type !== 'string') {
+              throw "NOT IMPLEMENTED (extension_api.json error): Cannot " +
+                  "parse values for type \"" + propertyDef.type + "\"";
             }
-          });
-        }
+          }
+          if (value) {
+            m[propertyName] = value;
+          }
+        });
       }
 
       addProperties(module, apiDef);
     });
 
-    // TODO(aa): The rest of the crap below this really needs to be factored out
-    // with a clean API boundary. Right now it is too soupy for me to feel
-    // comfortable running in content scripts. What if people are just
-    // overwriting random APIs? That would bypass our content script access
-    // checks.
-    if (!isExtensionProcess)
-      return;
-
-    // TODO(kalman/aa): "The rest of this crap..." comment above. Only run the
-    // custom hooks in extension processes, to maintain current behaviour. We
-    // should fix this this with a smaller hammer.
+    // Run the non-declarative custom hooks after all the schemas have been
+    // generated, in case hooks depend on other APIs being available.
     apiDefinitions.forEach(function(apiDef) {
       if (!isSchemaNodeSupported(apiDef, platform, manifestVersion))
         return;
@@ -570,7 +565,7 @@ var chrome = chrome || {};
       }, extensionId);
     });
 
-    // TOOD(mihaip): remove this alias once the webstore stops calling
+    // TODO(mihaip): remove this alias once the webstore stops calling
     // beginInstallWithManifest2.
     // See http://crbug.com/100242
     if (chrome.webstorePrivate) {