Prevent ObjectBackedNativeHandler from calling itself once invalid.

chrome/renderer/extensions/object_backed_native_handler.cc

Index: chrome/renderer/extensions/object_backed_native_handler.cc
diff --git a/chrome/renderer/extensions/object_backed_native_handler.cc b/chrome/renderer/extensions/object_backed_native_handler.cc
index 2a4043cb74b0226d5f465a8b9684e9cc5dffbb79..bc6d6ece0ddaa710aa1f8443b3ce54f69828e8ca 100644
--- a/chrome/renderer/extensions/object_backed_native_handler.cc
+++ b/chrome/renderer/extensions/object_backed_native_handler.cc
@@ -11,19 +11,13 @@
 
 namespace extensions {
 
-// Data to pass to ObjectBackedNativeHandler::Router.
-struct ObjectBackedNativeHandler::RouterData {
-  RouterData(ObjectBackedNativeHandler* self, HandlerFunction function)
-    : self(self), function(function) {}
+namespace {
 
-  ~RouterData() {}
+// Keys for the router data objects.
+const char* kIsValid = "is_valid";
+const char* kHandlerFunction = "handler_function";
 
-  // The owner of the routed data.
-  ObjectBackedNativeHandler* const self;
-
-  // The function to route calls to.
-  HandlerFunction function;
-};
+}  // namespace
 
 ObjectBackedNativeHandler::ObjectBackedNativeHandler(
     v8::Handle<v8::Context> context)
@@ -42,29 +36,36 @@ v8::Handle<v8::Object> ObjectBackedNativeHandler::NewInstance() {
 // static
 v8::Handle<v8::Value> ObjectBackedNativeHandler::Router(
     const v8::Arguments& args) {
-  RouterData* router_data = static_cast<RouterData*>(
-      args.Data().As<v8::External>()->Value());
-  // Router can be called during context destruction. Stop.
-  if (!router_data->self->is_valid())
-    return v8::Handle<v8::Value>();
-  return router_data->function.Run(args);
+  v8::Handle<v8::Object> data = args.Data().As<v8::Object>();
+  if (!data->GetHiddenValue(v8::String::New(kIsValid))->BooleanValue()) {

[koz (OOO until 15th September), 2013/03/13 23:08:43]

Another option would be to add an is_valid field to RouterData itself, rather
than replacing it with a v8::Object...

Ah, but that would defeat the whole purpose here because RouterDatas are owned
by the ObjectBackedNativeHandler, which gets destroyed and then called into.
Right.

It might be good to add an explanatory comment to the class explaining the
motivations for this design.

[not at google - send to devlin, 2013/03/13 23:49:25]

Done.

+    return v8::ThrowException(v8::String::New(
+        "Extension view no longer exists"));
+  }
+  v8::Handle<v8::Value> handler_function =
+      data->GetHiddenValue(v8::String::New(kHandlerFunction));
+  CHECK(!handler_function.IsEmpty());
+  return static_cast<HandlerFunction*>(
+      handler_function.As<v8::External>()->Value())->Run(args);
 }
 
 void ObjectBackedNativeHandler::RouteFunction(
     const std::string& name,
     const HandlerFunction& handler_function) {
-  linked_ptr<RouterData> data(new RouterData(this, handler_function));
-  // TODO(koz): Investigate using v8's MakeWeak() function instead of holding
-  // on to these pointers here.
+  v8::HandleScope handle_scope;
+  v8::Persistent<v8::Object> data = v8::Persistent<v8::Object>::New(
+      v8_context_->GetIsolate(), v8::Object::New());
+  data->SetHiddenValue(v8::String::New(kIsValid), v8::Boolean::New(true));
+  data->SetHiddenValue(v8::String::New(kHandlerFunction),
+      v8::External::New(new HandlerFunction(handler_function)));
   router_data_.push_back(data);
   v8::Handle<v8::FunctionTemplate> function_template =
-      v8::FunctionTemplate::New(Router, v8::External::New(data.get()));
+      v8::FunctionTemplate::New(Router, data);
   object_template_->Set(name.c_str(), function_template);
 }
 
 void ObjectBackedNativeHandler::RouteStaticFunction(
     const std::string& name,
-    const HandlerFunc handler_func) {
+    const HandlerFunc& handler_func) {
   v8::Handle<v8::FunctionTemplate> function_template =
       v8::FunctionTemplate::New(handler_func, v8::External::New(this));
   object_template_->Set(name.c_str(), function_template);
@@ -73,6 +74,18 @@ void ObjectBackedNativeHandler::RouteStaticFunction(
 void ObjectBackedNativeHandler::Invalidate() {
   if (!is_valid())
     return;
+  for (RouterData::iterator it = router_data_.begin();
+       it != router_data_.end(); ++it) {
+    v8::Persistent<v8::Object> data = *it;
+    data->SetHiddenValue(v8::String::New(kIsValid), v8::Boolean::New(false));
+    v8::Handle<v8::Value> handler_function =
+        data->GetHiddenValue(v8::String::New(kHandlerFunction));
+    CHECK(!handler_function.IsEmpty());
+    delete static_cast<HandlerFunction*>(
+        handler_function.As<v8::External>()->Value());
+    data->DeleteHiddenValue(v8::String::New(kHandlerFunction));
+    data.Dispose(v8_context_->GetIsolate());
+  }
   object_template_.reset();
   v8_context_.reset();
   NativeHandler::Invalidate();