Nukes ContextSupplement and makes PerContextData extend SupportUserData

This gives the same functionality and ownership model while making it
simpler to associate arbitrary data with a context.

BUG=none
TEST=none
R=abarth@chromium.org

Review URL: https://codereview.chromium.org/183433002

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

5 files changed, 32 insertions, 83 deletions

diff --git a/gin/context_holder.cc b/gin/context_holder.cc
index fba2a2a..4a7be7c 100644
--- a/gin/context_holder.cc
+++ b/gin/context_holder.cc
@@ -14,10 +14,7 @@ ContextHolder::ContextHolder(v8::Isolate* isolate)
 }
 
 ContextHolder::~ContextHolder() {
-  v8::HandleScope handle_scope(isolate());
-  v8::Handle<v8::Context> context = this->context();
-
-  data_->Detach(context);
+  // PerContextData needs to be destroyed before the context.
   data_.reset();
 }
 
diff --git a/gin/modules/module_registry.cc b/gin/modules/module_registry.cc
index 0c1a976..d4e3067 100644
--- a/gin/modules/module_registry.cc
+++ b/gin/modules/module_registry.cc
@@ -10,6 +10,7 @@
 #include "base/logging.h"
 #include "gin/arguments.h"
 #include "gin/converter.h"
+#include "gin/per_context_data.h"
 #include "gin/per_isolate_data.h"
 #include "gin/public/wrapper_info.h"
 #include "gin/runner.h"
@@ -47,6 +48,13 @@ PendingModule::~PendingModule() {
 
 namespace {
 
+// Key for base::SupportsUserData::Data.
+const char kModuleRegistryKey[] = "ModuleRegistry";
+
+struct ModuleRegistryData : public base::SupportsUserData::Data {
+  scoped_ptr<ModuleRegistry> registry;
+};
+
 void Define(const v8::FunctionCallbackInfo<Value>& info) {
   Arguments args(info);
 
@@ -87,10 +95,6 @@ Local<FunctionTemplate> GetDefineTemplate(Isolate* isolate) {
   return templ;
 }
 
-v8::Handle<String> GetHiddenValueKey(Isolate* isolate) {
-  return StringToSymbol(isolate, "::gin::ModuleRegistry");
-}
-
 }  // namespace
 
 ModuleRegistry::ModuleRegistry(Isolate* isolate)
@@ -109,20 +113,19 @@ void ModuleRegistry::RegisterGlobals(Isolate* isolate,
 
 // static
 ModuleRegistry* ModuleRegistry::From(v8::Handle<Context> context) {
-  Isolate* isolate = context->GetIsolate();
-  v8::Handle<String> key = GetHiddenValueKey(isolate);
-  v8::Handle<Value> value = context->Global()->GetHiddenValue(key);
-  v8::Handle<External> external;
-  if (value.IsEmpty() || !ConvertFromV8(isolate, value, &external)) {
-    PerContextData* data = PerContextData::From(context);
-    if (!data)
-      return NULL;
-    ModuleRegistry* registry = new ModuleRegistry(isolate);
-    context->Global()->SetHiddenValue(key, External::New(isolate, registry));
-    data->AddSupplement(scoped_ptr<ContextSupplement>(registry));
-    return registry;
+  PerContextData* data = PerContextData::From(context);
+  if (!data)
+    return NULL;
+
+  ModuleRegistryData* registry_data = static_cast<ModuleRegistryData*>(
+      data->GetUserData(kModuleRegistryKey));
+  if (!registry_data) {
+    // PerContextData takes ownership of ModuleRegistryData.
+    registry_data = new ModuleRegistryData;
+    registry_data->registry.reset(new ModuleRegistry(context->GetIsolate()));
+    data->SetUserData(kModuleRegistryKey, registry_data);
   }
-  return static_cast<ModuleRegistry*>(external->Value());
+  return registry_data->registry.get();
 }
 
 void ModuleRegistry::AddBuiltinModule(Isolate* isolate, const std::string& id,
@@ -170,11 +173,6 @@ void ModuleRegistry::RegisterModule(Isolate* isolate,
   callback.Run(module);
 }
 
-void ModuleRegistry::Detach(v8::Handle<Context> context) {
-  context->Global()->SetHiddenValue(GetHiddenValueKey(context->GetIsolate()),
-                                    v8::Handle<Value>());
-}
-
 bool ModuleRegistry::CheckDependencies(PendingModule* pending) {
   size_t num_missing_dependencies = 0;
   size_t len = pending->dependencies.size();
diff --git a/gin/modules/module_registry.h b/gin/modules/module_registry.h
index 755875c..2270d31 100644
--- a/gin/modules/module_registry.h
+++ b/gin/modules/module_registry.h
@@ -13,8 +13,9 @@
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/scoped_vector.h"
 #include "gin/gin_export.h"
-#include "gin/per_context_data.h"
+#include "v8/include/v8.h"
 
 namespace gin {
 
@@ -31,7 +32,7 @@ struct PendingModule;
 // function. The spec says we should only add that property once our
 // implementation complies with the specification.
 //
-class GIN_EXPORT ModuleRegistry : public ContextSupplement {
+class GIN_EXPORT ModuleRegistry {
  public:
   typedef base::Callback<void (v8::Handle<v8::Value>)> LoadModuleCallback;
 
@@ -71,9 +72,6 @@ class GIN_EXPORT ModuleRegistry : public ContextSupplement {
 
   explicit ModuleRegistry(v8::Isolate* isolate);
 
-  // From ContextSupplement:
-  virtual void Detach(v8::Handle<v8::Context> context) OVERRIDE;
-
   void Load(v8::Isolate* isolate, scoped_ptr<PendingModule> pending);
   void RegisterModule(v8::Isolate* isolate,
                       const std::string& id,
diff --git a/gin/per_context_data.cc b/gin/per_context_data.cc
index 5fedf52..3960528 100644
--- a/gin/per_context_data.cc
+++ b/gin/per_context_data.cc
@@ -10,12 +10,6 @@
 
 namespace gin {
 
-ContextSupplement::ContextSupplement() {
-}
-
-ContextSupplement::~ContextSupplement() {
-}
-
 PerContextData::PerContextData(v8::Handle<v8::Context> context)
     : runner_(NULL) {
   context->SetAlignedPointerInEmbedderData(
@@ -23,21 +17,6 @@ PerContextData::PerContextData(v8::Handle<v8::Context> context)
 }
 
 PerContextData::~PerContextData() {
-  DCHECK(supplements_.empty());
-}
-
-void PerContextData::Detach(v8::Handle<v8::Context> context) {
-  DCHECK(From(context) == this);
-  context->SetAlignedPointerInEmbedderData(
-      kPerContextDataStartIndex + kEmbedderNativeGin, NULL);
-
-  SuplementVector supplements;
-  supplements.swap(supplements_);
-
-  for (SuplementVector::iterator it = supplements.begin();
-       it != supplements.end(); ++it) {
-    (*it)->Detach(context);
-  }
 }
 
 // static
@@ -46,8 +25,4 @@ PerContextData* PerContextData::From(v8::Handle<v8::Context> context) {
       context->GetAlignedPointerFromEmbedderData(kEncodedValueIndex));
 }
 
-void PerContextData::AddSupplement(scoped_ptr<ContextSupplement> supplement) {
-  supplements_.push_back(supplement.release());
-}
-
 }  // namespace gin
diff --git a/gin/per_context_data.h b/gin/per_context_data.h
index 3ad68d2..5f9d0f7 100644
--- a/gin/per_context_data.h
+++ b/gin/per_context_data.h
@@ -6,8 +6,7 @@
 #define GIN_PER_CONTEXT_DATA_H_
 
 #include "base/basictypes.h"
-#include "base/memory/scoped_ptr.h"
-#include "base/memory/scoped_vector.h"
+#include "base/supports_user_data.h"
 #include "gin/gin_export.h"
 #include "v8/include/v8.h"
 
@@ -15,44 +14,26 @@ namespace gin {
 
 class Runner;
 
-// Embedders can store additional per-context data by subclassing
-// ContextSupplement.
-class GIN_EXPORT ContextSupplement {
- public:
-  ContextSupplement();
-  virtual ~ContextSupplement();
-
-  // Detach will be called before ContextHolder disposes the v8::Context.
-  // Embedders should not interact with |context| after Detach has been called.
-  virtual void Detach(v8::Handle<v8::Context> context) = 0;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(ContextSupplement);
-};
-
 // There is one instance of PerContextData per v8::Context managed by Gin. This
-// class stores all the Gin-related data that varies per context.
-class GIN_EXPORT PerContextData {
+// class stores all the Gin-related data that varies per context. Arbitrary data
+// can be associated with this class by way of the SupportsUserData methods.
+// Instances of this class (and any associated user data) are destroyed before
+// the associated v8::Context.
+class GIN_EXPORT PerContextData : public base::SupportsUserData {
  public:
   explicit PerContextData(v8::Handle<v8::Context> context);
-  ~PerContextData();
+  virtual ~PerContextData();
 
   // Can return NULL after the ContextHolder has detached from context.
-  static PerContextData* From(v8::Handle<v8::Context>);
-  void Detach(v8::Handle<v8::Context> context);
+  static PerContextData* From(v8::Handle<v8::Context> context);
 
   // The Runner associated with this context. To execute script in this context,
   // please use the appropriate API on Runner.
   Runner* runner() const { return runner_; }
   void set_runner(Runner* runner) { runner_ = runner; }
 
-  void AddSupplement(scoped_ptr<ContextSupplement> supplement);
-
  private:
-  typedef ScopedVector<ContextSupplement> SuplementVector;
-
   Runner* runner_;
-  SuplementVector supplements_;
 
   DISALLOW_COPY_AND_ASSIGN(PerContextData);
 };