Make a process-wide cache for the v8::Value representation of extension APIs.

TEST=browser_tests


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

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

4 files changed, 108 insertions, 55 deletions

diff --git a/chrome/renderer/extensions/extension_dispatcher.h b/chrome/renderer/extensions/extension_dispatcher.h
index 3a0d2c1..d16b8b9 100644
--- a/chrome/renderer/extensions/extension_dispatcher.h
+++ b/chrome/renderer/extensions/extension_dispatcher.h
@@ -17,6 +17,7 @@
 #include "chrome/common/extensions/feature.h"
 #include "chrome/renderer/extensions/chrome_v8_context.h"
 #include "chrome/renderer/extensions/chrome_v8_context_set.h"
+#include "chrome/renderer/extensions/v8_schema_registry.h"
 #include "chrome/renderer/resource_bundle_source_map.h"
 #include "v8/include/v8.h"
 
@@ -55,6 +56,9 @@ class ExtensionDispatcher : public content::RenderProcessObserver {
     return v8_context_set_;
   }
   UserScriptSlave* user_script_slave() { return user_script_slave_.get(); }
+  extensions::V8SchemaRegistry* v8_schema_registry() {
+    return &v8_schema_registry_;
+  }
 
   bool IsExtensionActive(const std::string& extension_id) const;
 
@@ -198,6 +202,9 @@ class ExtensionDispatcher : public content::RenderProcessObserver {
 
   ResourceBundleSourceMap source_map_;
 
+  // Cache for the v8 representation of extension API schemas.
+  extensions::V8SchemaRegistry v8_schema_registry_;
+
   DISALLOW_COPY_AND_ASSIGN(ExtensionDispatcher);
 };
 
diff --git a/chrome/renderer/extensions/schema_generated_bindings.cc b/chrome/renderer/extensions/schema_generated_bindings.cc
index 257f3f9..525c0e8 100644
--- a/chrome/renderer/extensions/schema_generated_bindings.cc
+++ b/chrome/renderer/extensions/schema_generated_bindings.cc
@@ -81,38 +81,7 @@ class ExtensionImpl : public ChromeV8Extension {
     RouteStaticFunction("SetIconCommon", &SetIconCommon);
   }
 
-  ~ExtensionImpl() {
-    // TODO(aa): It seems that v8 never deletes us, so this doesn't get called.
-    // Leaving this in here in case v8's implementation ever changes.
-    for (CachedSchemaMap::iterator it = schemas_.begin(); it != schemas_.end();
-        ++it) {
-      if (!it->second.IsEmpty())
-        it->second.Dispose();
-    }
-  }
-
  private:
-  static v8::Handle<v8::Value> GetV8SchemaForAPI(
-      ExtensionImpl* self,
-      v8::Handle<v8::Context> context,
-      const std::string& api_name) {
-    CachedSchemaMap::iterator maybe_api = self->schemas_.find(api_name);
-    if (maybe_api != self->schemas_.end())
-      return maybe_api->second;
-
-    scoped_ptr<V8ValueConverter> v8_value_converter(V8ValueConverter::create());
-    const base::DictionaryValue* schema =
-        ExtensionAPI::GetInstance()->GetSchema(api_name);
-    CHECK(schema) << api_name;
-
-    self->schemas_[api_name] =
-        v8::Persistent<v8::Object>::New(v8::Handle<v8::Object>::Cast(
-            v8_value_converter->ToV8Value(schema, context)));
-    CHECK(!self->schemas_[api_name].IsEmpty());
-
-    return self->schemas_[api_name];
-  }
-
   static v8::Handle<v8::Value> GetExtensionAPIDefinition(
       const v8::Arguments& args) {
     ExtensionImpl* self = GetFromArguments<ExtensionImpl>(args);
@@ -141,24 +110,7 @@ class ExtensionImpl : public ChromeV8Extension {
           UserScriptSlave::GetDataSourceURLForFrame(v8_context->web_frame()));
     }
 
-    v8::Persistent<v8::Context> context(v8::Context::New());
-    v8::Context::Scope context_scope(context);
-    v8::Handle<v8::Array> api(v8::Array::New(apis->size()));
-    size_t api_index = 0;
-    for (std::set<std::string>::iterator i = apis->begin(); i != apis->end();
-        ++i) {
-      // TODO(kalman): this caching is actually useless now, because
-      // SchemaGeneratedBindings is per-context not per-process. We should
-      // (e.g.) hang a SchemaRegistry off ExtensionDispatcher (which maintains
-      // a *single* v8::Context rather than multiple ones as here).
-      api->Set(api_index, GetV8SchemaForAPI(self, context, *i));
-      ++api_index;
-    }
-
-    // The persistent extension_api_ will keep the context alive.
-    context.Dispose();
-
-    return api;
+    return dispatcher->v8_schema_registry()->GetSchemas(*apis);
   }
 
   static v8::Handle<v8::Value> GetNextRequestId(const v8::Arguments& args) {
@@ -321,12 +273,6 @@ class ExtensionImpl : public ChromeV8Extension {
 
     return StartRequestCommon(args, &list_value);
   }
-
-  // Cached JS Array representation of each namespace in extension_api.json.
-  // We store this so that we don't have to parse it over and over again for
-  // every context that uses it.
-  typedef std::map<std::string, v8::Persistent<v8::Object> > CachedSchemaMap;
-  CachedSchemaMap schemas_;
 };
 
 }  // namespace
diff --git a/chrome/renderer/extensions/v8_schema_registry.cc b/chrome/renderer/extensions/v8_schema_registry.cc
new file mode 100644
index 0000000..35a22b2
--- /dev/null
+++ b/chrome/renderer/extensions/v8_schema_registry.cc
@@ -0,0 +1,56 @@
+// 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.
+
+#include "chrome/renderer/extensions/v8_schema_registry.h"
+
+#include "base/logging.h"
+#include "base/values.h"
+#include "chrome/common/extensions/api/extension_api.h"
+#include "content/public/renderer/v8_value_converter.h"
+
+using content::V8ValueConverter;
+
+namespace extensions {
+
+V8SchemaRegistry::V8SchemaRegistry() : context_(v8::Context::New()) {}
+
+V8SchemaRegistry::~V8SchemaRegistry() {
+  for (SchemaCache::iterator i = schema_cache_.begin();
+      i != schema_cache_.end(); ++i) {
+    i->second.Dispose();
+  }
+  context_.Dispose();
+}
+
+v8::Handle<v8::Array> V8SchemaRegistry::GetSchemas(
+    const std::set<std::string>& apis) {
+  v8::Context::Scope context_scope(context_);
+  v8::Handle<v8::Array> v8_apis(v8::Array::New(apis.size()));
+  size_t api_index = 0;
+  for (std::set<std::string>::const_iterator i = apis.begin(); i != apis.end();
+      ++i) {
+    v8_apis->Set(api_index++, GetSchema(*i));
+  }
+  return v8_apis;
+}
+
+v8::Handle<v8::Object> V8SchemaRegistry::GetSchema(const std::string& api) {
+  SchemaCache::iterator maybe_schema = schema_cache_.find(api);
+  if (maybe_schema != schema_cache_.end())
+    return maybe_schema->second;
+
+  const base::DictionaryValue* schema =
+      ExtensionAPI::GetInstance()->GetSchema(api);
+  CHECK(schema) << api;
+
+  scoped_ptr<V8ValueConverter> v8_value_converter(V8ValueConverter::create());
+  v8::Persistent<v8::Object> v8_schema =
+      v8::Persistent<v8::Object>::New(v8::Handle<v8::Object>::Cast(
+          v8_value_converter->ToV8Value(schema, context_)));
+  CHECK(!v8_schema.IsEmpty());
+  schema_cache_[api] = v8_schema;
+  return v8_schema;
+}
+
+}  // namespace extensions
diff --git a/chrome/renderer/extensions/v8_schema_registry.h b/chrome/renderer/extensions/v8_schema_registry.h
new file mode 100644
index 0000000..987f48e
--- /dev/null
+++ b/chrome/renderer/extensions/v8_schema_registry.h
@@ -0,0 +1,44 @@
+// 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_RENDERER_EXTENSIONS_V8_SCHEMA_REGISTRY_H_
+#define CHROME_RENDERER_EXTENSIONS_V8_SCHEMA_REGISTRY_H_
+#pragma once
+
+#include <map>
+#include <set>
+#include <string>
+
+#include "base/basictypes.h"
+#include "v8/include/v8.h"
+
+namespace extensions {
+
+// A registry for the v8::Value representations of extension API schemas.
+// In a way, the v8 counterpart to ExtensionAPI.
+class V8SchemaRegistry {
+ public:
+  V8SchemaRegistry();
+  ~V8SchemaRegistry();
+
+  // Returns a v8::Array with all the schemas for the APIs in |apis|.
+  v8::Handle<v8::Array> GetSchemas(const std::set<std::string>& apis);
+
+ private:
+  // Returns a v8::Object for the schema for |api|, possibly from the cache.
+  v8::Handle<v8::Object> GetSchema(const std::string& api);
+
+  // Cache of schemas.
+  typedef std::map<std::string, v8::Persistent<v8::Object> > SchemaCache;
+  SchemaCache schema_cache_;
+
+  // Single per-instance v8::Context to create v8::Values.
+  v8::Persistent<v8::Context> context_;
+
+  DISALLOW_COPY_AND_ASSIGN(V8SchemaRegistry);
+};
+
+}  // namespace extensions
+
+#endif  // CHROME_RENDERER_EXTENSIONS_V8_SCHEMA_REGISTRY_H_