3 files changed, 45 insertions, 21 deletions

diff --git a/chrome/renderer/extensions/event_bindings.cc b/chrome/renderer/extensions/event_bindings.cc
index bfa9e48..59ed319 100755
--- a/chrome/renderer/extensions/event_bindings.cc
+++ b/chrome/renderer/extensions/event_bindings.cc
@@ -25,6 +25,7 @@ ContextList& GetRegisteredContexts() {
 }
 
 const char* kExtensionDeps[] = { JsonJsV8Extension::kName };
+const char* kContextAttachCount = "chromium.attachCount";
 
 class ExtensionImpl : public v8::Extension {
  public:
@@ -50,40 +51,66 @@ class ExtensionImpl : public v8::Extension {
   // The idea is to eventually notify the browser about what events are being
   // listened to, so it can dispatch appropriately.
   static v8::Handle<v8::Value> AttachEvent(const v8::Arguments& args) {
-    const char* kContextRegistered = "chromium.extension.contextRegistered";
-
     v8::Persistent<v8::Context> context =
         v8::Persistent<v8::Context>::New(v8::Context::GetCurrent());
     v8::Local<v8::Object> global = context->Global();
-    v8::Local<v8::Value> is_registered = global->GetHiddenValue(
-        v8::String::New(kContextRegistered));
-    if (is_registered.IsEmpty() || is_registered->IsUndefined()) {
+
+    // Remember how many times this context has been attached, so we can
+    // register the context on first attach and unregister on last detach.
+    v8::Local<v8::Value> attach_count = global->GetHiddenValue(
+        v8::String::New(kContextAttachCount));
+    int32_t account_count_value =
+        (!attach_count.IsEmpty() && attach_count->IsNumber()) ?
+        attach_count->Int32Value() : 0;
+    if (account_count_value == 0) {
+      // First time attaching.
       GetRegisteredContexts().push_back(context);
       context.MakeWeak(NULL, WeakContextCallback);
-      global->SetHiddenValue(
-          v8::String::New(kContextRegistered), v8::Boolean::New(true));
     }
+    global->SetHiddenValue(
+        v8::String::New(kContextAttachCount),
+        v8::Integer::New(account_count_value + 1));
+
     return v8::Undefined();
   }
 
-  // TODO(mpcomplete): Implement this.  This should do the reverse of
-  // AttachEvent, when that actually does what we plan on.
   static v8::Handle<v8::Value> DetachEvent(const v8::Arguments& args) {
+    v8::Local<v8::Context> context = v8::Context::GetCurrent();
+    v8::Local<v8::Object> global = context->Global();
+    v8::Local<v8::Value> attach_count = global->GetHiddenValue(
+        v8::String::New(kContextAttachCount));
+    DCHECK(!attach_count.IsEmpty() && attach_count->IsNumber());
+    int32_t account_count_value = attach_count->Int32Value();
+    DCHECK(account_count_value > 0);
+    if (account_count_value == 1) {
+      // Clean up after last detach.
+      UnregisterContext(context);
+    }
+    global->SetHiddenValue(
+        v8::String::New(kContextAttachCount),
+        v8::Integer::New(account_count_value - 1));
+
     return v8::Undefined();
   }
 
   // Called when a registered context is garbage collected.
-  static void WeakContextCallback(v8::Persistent<v8::Value> obj, void*) {
-    ContextList::iterator it = std::find(GetRegisteredContexts().begin(),
-                                         GetRegisteredContexts().end(), obj);
-    if (it == GetRegisteredContexts().end()) {
+  static void UnregisterContext(v8::Handle<void> context) {
+    ContextList& contexts = GetRegisteredContexts();
+    ContextList::iterator it = std::find(contexts.begin(), contexts.end(),
+                                         context);
+    if (it == contexts.end()) {
       NOTREACHED();
       return;
     }
 
     it->Dispose();
     it->Clear();
-    GetRegisteredContexts().erase(it);
+    contexts.erase(it);
+  }
+
+  // Called when a registered context is garbage collected.
+  static void WeakContextCallback(v8::Persistent<v8::Value> obj, void*) {
+    UnregisterContext(obj);
   }
 };
 
@@ -103,12 +130,6 @@ void EventBindings::CallFunction(const std::string& function_name,
     v8::Context::Scope context_scope(*it);
     v8::Local<v8::Object> global = (*it)->Global();
 
-    // Check if the window object is gone, which means this context's frame
-    // has been unloaded.
-    v8::Local<v8::Value> window = global->Get(v8::String::New("window"));
-    if (!window->IsObject())
-      continue;
-
     v8::Local<v8::Script> script = v8::Script::Compile(
         v8::String::New(function_name.c_str()));
     v8::Local<v8::Value> function_obj = script->Run();
diff --git a/chrome/renderer/renderer_resources.grd b/chrome/renderer/renderer_resources.grd
index c59e98c..cda253b 100755
--- a/chrome/renderer/renderer_resources.grd
+++ b/chrome/renderer/renderer_resources.grd
@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <!-- This comment is only here because changes to resources are not picked up
-without changes to the corresponding grd file.  -->
+without changes to the corresponding grd file. -->
 <grit latest_public_release="0" current_release="1">
   <outputs>
     <output filename="grit/renderer_resources.h" type="rc_header">
diff --git a/chrome/renderer/resources/event_bindings.js b/chrome/renderer/resources/event_bindings.js
index 62e249a..ed097dc 100644
--- a/chrome/renderer/resources/event_bindings.js
+++ b/chrome/renderer/resources/event_bindings.js
@@ -105,6 +105,8 @@ var chromium = chromium || {};
   // name.
   chromium.Event.prototype.attach_ = function() {
     AttachEvent(this.eventName_);
+    this.unloadHandler_ = this.detach_.bind(this);
+    window.addEventListener('unload', this.unloadHandler_, false);
     if (!this.eventName_)
       return;
 
@@ -118,6 +120,7 @@ var chromium = chromium || {};
 
   // Detaches this event object from its name.
   chromium.Event.prototype.detach_ = function() {
+    window.removeEventListener('unload', this.unloadHandler_, false);
     DetachEvent(this.eventName_);
     if (!this.eventName_)
       return;