Change extension event bindings to auto-detach events when the window unloads.

Also added code to DetachEvent to unregister the context when no events are
attached.  This allows us to remove contexts from our list immediately when
their frames are unloaded.
Review URL: http://codereview.chromium.org/79057

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

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;