Convert app_bindings.js to the schema_generated_bindings.js infrastructure.

chrome/renderer/extensions/schema_generated_bindings.cc

Index: chrome/renderer/extensions/schema_generated_bindings.cc
diff --git a/chrome/renderer/extensions/schema_generated_bindings.cc b/chrome/renderer/extensions/schema_generated_bindings.cc
index 0af9c0e145afd0218543cd7de9cfdc8bfabc0f0c..b1102ca372c27b3d2a21ecb0cd7300e258a31cc6 100644
--- a/chrome/renderer/extensions/schema_generated_bindings.cc
+++ b/chrome/renderer/extensions/schema_generated_bindings.cc
@@ -118,30 +118,28 @@ class ExtensionImpl : public ChromeV8Extension {
     ChromeV8Context* v8_context = dispatcher->v8_context_set().GetCurrent();
     CHECK(v8_context);
 
-    std::string extension_id = v8_context->extension_id();
-    ExtensionAPI::SchemaMap schemas;
-    ExtensionAPI::GetSchemasFilter filter =
-        dispatcher->is_extension_process() ?
-            ExtensionAPI::ALL : ExtensionAPI::ONLY_UNPRIVILEGED;
+    // TODO(kalman): can we just cache this in the ChromeV8Context instance?

[Aaron Boodman, 2012/03/06 01:17:21]

What is the value in that. This should only be getting called once per context
anyway.

[not at google - send to devlin, 2012/03/06 03:53:00]

Because it's getting called many times per context: each time IsAPIAllowed
(native call, see ExtensionDispatcher and setup_bindings.js) is called involves
calculating the allowed APIs. This is pretty unfortunate, in fact, it gets
called num_allowed_apis times (+ once more here).

The other way to avoid this is to just forget about the extra safety of
IsAPIAllowed (since in reality it's called only from setup_bindings.js in a loop
over APIs that are allowed).

Stepping back, the check in setup_bindings.js should really be put on require()
somehow, else it's only a half-baked security mechanism.

Anyway, in conclusion, I'm most in favour of deleting IsAPIAllowed.

[Aaron Boodman, 2012/03/06 08:18:12]

Completely agree about removing IsAPIAllowed.

[not at google - send to devlin, 2012/03/06 11:36:46]

Done. Also means that ContextInfoNativeHandler can be entirely deleted.

+    scoped_ptr<std::set<std::string> > apis;
 
+    const std::string& extension_id = v8_context->extension_id();
     if (dispatcher->IsTestExtensionId(extension_id)) {
-      ExtensionAPI::GetInstance()->GetDefaultSchemas(filter, &schemas);
+      apis.reset(new std::set<std::string>());
+      // The minimal set of APIs that tests need.
+      apis->insert("extension");
     } else {
-      const ::Extension* extension =
-          dispatcher->extensions()->GetByID(extension_id);
-      CHECK(extension) << extension_id << " not found";
-      ExtensionAPI::GetInstance()->GetSchemasForExtension(
-          *extension, filter, &schemas);
+      apis = ExtensionAPI::GetInstance()->GetAPIsForContext(
+          v8_context->context_type(),
+          dispatcher->extensions()->GetByID(extension_id),
+          UserScriptSlave::GetDataSourceURLForFrame(v8_context->web_frame()));
     }
 
     v8::Persistent<v8::Context> context(v8::Context::New());

[Aaron Boodman, 2012/03/06 01:17:21]

Not your bug, but we should just create one utility context that we reuse. The
way this is now, we can end up creating lots of contexts which is kinda a waste.

[not at google - send to devlin, 2012/03/06 03:53:00]

Chatting to koz, he says it shouldn't be using Context at all.  Should this just
be fixed in a later patch?

[koz (OOO until 15th September), 2012/03/06 04:06:58]

I'm not familiar with this usage of Context - isn't creating and disposing one
here a no-op because we don't execute any JS? As such I think those lines can be
deleted.

[Aaron Boodman, 2012/03/06 08:18:12]

Explained to kalman in irc.

[not at google - send to devlin, 2012/03/06 11:36:46]

TODO added.

     v8::Context::Scope context_scope(context);
-    v8::Handle<v8::Array> api(v8::Array::New(schemas.size()));
+    v8::Handle<v8::Array> api(v8::Array::New(apis->size()));
     size_t api_index = 0;
-    for (ExtensionAPI::SchemaMap::iterator it = schemas.begin();
-        it != schemas.end(); ++it) {
-      std::string api_name = it->first;
-      api->Set(api_index, GetV8SchemaForAPI(self, context, api_name));
+    for (std::set<std::string>::iterator i = apis->begin(); i != apis->end();
+        ++i) {
+      api->Set(api_index, GetV8SchemaForAPI(self, context, *i));
       ++api_index;
     }