From 9cec718466e9dec2fc36d7cc3951aee737df5f64 Mon Sep 17 00:00:00 2001
From: "aa@chromium.org" <aa@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Mon, 14 Sep 2009 19:08:41 +0000
Subject: Fix bug where content scripts can get GC'd if they don't attach to
 any DOM events.

BUG=17410
TEST=none

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@26136 0039d316-1c4b-4281-b951-d872f2087c98
---
 chrome/renderer/extensions/bindings_utils.cc |  9 +++++++++
 chrome/renderer/extensions/bindings_utils.h  | 10 +++++++++-
 chrome/renderer/extensions/event_bindings.cc | 22 ++++++++++++++++------
 3 files changed, 34 insertions(+), 7 deletions(-)

(limited to 'chrome')

diff --git a/chrome/renderer/extensions/bindings_utils.cc b/chrome/renderer/extensions/bindings_utils.cc
index d96f8d7..ddccf2c 100644
--- a/chrome/renderer/extensions/bindings_utils.cc
+++ b/chrome/renderer/extensions/bindings_utils.cc
@@ -71,6 +71,15 @@ ContextList GetContextsForExtension(const std::string& extension_id) {
   return contexts;
 }
 
+ContextInfo* GetInfoForCurrentContext() {
+  v8::Local<v8::Context> context = v8::Context::GetCurrent();
+  ContextList::iterator context_iter = FindContext(context);
+  if (context_iter == GetContexts().end())
+    return NULL;
+  else
+    return context_iter->get();
+}
+
 ContextList::iterator FindContext(v8::Handle<v8::Context> context) {
   ContextList& all_contexts = GetContexts();
 
diff --git a/chrome/renderer/extensions/bindings_utils.h b/chrome/renderer/extensions/bindings_utils.h
index 7354c72..b0b3f08 100644
--- a/chrome/renderer/extensions/bindings_utils.h
+++ b/chrome/renderer/extensions/bindings_utils.h
@@ -68,12 +68,17 @@ struct ContextInfo {
   // a valid pointer, and is used for comparisons only.  Do not dereference.
   RenderView* render_view;
 
+  // A count of the number of events that are listening in this context. When
+  // this is zero, |context| will be a weak handle.
+  int num_connected_events;
+
   ContextInfo(v8::Persistent<v8::Context> context,
               const std::string& extension_id,
               v8::Persistent<v8::Context> parent_context,
               RenderView* render_view)
       : context(context), extension_id(extension_id),
-        parent_context(parent_context), render_view(render_view) {}
+        parent_context(parent_context), render_view(render_view),
+        num_connected_events(0) {}
 };
 typedef std::list< linked_ptr<ContextInfo> > ContextList;
 
@@ -86,6 +91,9 @@ ContextList GetContextsForExtension(const std::string& extension_id);
 // Returns the ContextInfo item that has the given context.
 ContextList::iterator FindContext(v8::Handle<v8::Context> context);
 
+// Returns the ContextInfo for the current v8 context.
+ContextInfo* GetInfoForCurrentContext();
+
 // Contains info relevant to a pending API request.
 struct PendingRequest {
  public :
diff --git a/chrome/renderer/extensions/event_bindings.cc b/chrome/renderer/extensions/event_bindings.cc
index 416ab2c..c30c364 100644
--- a/chrome/renderer/extensions/event_bindings.cc
+++ b/chrome/renderer/extensions/event_bindings.cc
@@ -22,12 +22,16 @@ using bindings_utils::CallFunctionInContext;
 using bindings_utils::ContextInfo;
 using bindings_utils::ContextList;
 using bindings_utils::GetContexts;
+using bindings_utils::GetInfoForCurrentContext;
 using bindings_utils::GetStringResource;
 using bindings_utils::ExtensionBase;
 using bindings_utils::GetPendingRequestMap;
 using bindings_utils::PendingRequestMap;
 using WebKit::WebFrame;
 
+static void ContextWeakReferenceCallback(v8::Persistent<v8::Value> context,
+                                         void*);
+
 namespace {
 
 // Keep a local cache of RenderThread so that we can mock it out for unit tests.
@@ -79,19 +83,18 @@ class ExtensionImpl : public ExtensionBase {
       std::string event_name(*v8::String::AsciiValue(args[0]));
       bool has_permission =
           ExtensionProcessBindings::CurrentContextHasPermission(event_name);
-#if EXTENSION_TIME_TO_BREAK_API
-      bool allow_api = has_permission;
-#else
-      bool allow_api = true;
-#endif
 
       // Increment the count even if the caller doesn't have permission, so that
       // refcounts stay balanced.
-      if (EventIncrementListenerCount(event_name) == 1 && allow_api) {
+      if (EventIncrementListenerCount(event_name) == 1 && has_permission) {
         EventBindings::GetRenderThread()->Send(
             new ViewHostMsg_ExtensionAddListener(event_name));
       }
 
+      ContextInfo* current_context_info = GetInfoForCurrentContext();
+      if (++current_context_info->num_connected_events == 1)
+        current_context_info->context.ClearWeak();
+
       if (!has_permission) {
         return ExtensionProcessBindings::ThrowPermissionDeniedException(
             event_name);
@@ -112,6 +115,13 @@ class ExtensionImpl : public ExtensionBase {
         EventBindings::GetRenderThread()->Send(
           new ViewHostMsg_ExtensionRemoveListener(event_name));
       }
+
+      ContextInfo* current_context_info = GetInfoForCurrentContext();
+      if (current_context_info &&
+          --current_context_info->num_connected_events == 0) {
+        current_context_info->context.MakeWeak(NULL,
+            &ContextWeakReferenceCallback);
+      }
     }
 
     return v8::Undefined();
-- 
cgit v1.1