Fix V8EventListener leak.

This replaces Feng's CL; its very similar, but a bit different :-)

Hopefully addresses the style concerns.

Verified this cleans the leak on page cycler and restores perf.

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

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

5 files changed, 73 insertions, 46 deletions

diff --git a/webkit/port/bindings/v8/V8XMLHttpRequestCustom.cpp b/webkit/port/bindings/v8/V8XMLHttpRequestCustom.cpp
index 0c2cda0..ce4c84d 100644
--- a/webkit/port/bindings/v8/V8XMLHttpRequestCustom.cpp
+++ b/webkit/port/bindings/v8/V8XMLHttpRequestCustom.cpp
@@ -135,7 +135,8 @@ ACCESSOR_SETTER(XMLHttpRequestOnabort) {
     if (!proxy)
       return;
 
-    EventListener* listener = proxy->FindOrCreateXHREventListener(value, false);
+    RefPtr<EventListener> listener =
+      proxy->FindOrCreateXHREventListener(value, false);
     if (listener) {
       imp->setOnAbortListener(listener);
       CreateHiddenXHRDependency(info.Holder(), value);
@@ -148,7 +149,7 @@ ACCESSOR_GETTER(XMLHttpRequestOnerror) {
   XMLHttpRequest* imp = V8Proxy::ToNativeObject<XMLHttpRequest>(
       V8ClassIndex::XMLHTTPREQUEST, info.Holder());
   if (imp->onErrorListener()) {
-    V8XHREventListener* listener =
+    RefPtr<V8XHREventListener> listener =
         static_cast<V8XHREventListener*>(imp->onErrorListener());
     v8::Local<v8::Object> v8_listener = listener->GetListenerObject();
     return v8_listener;
@@ -175,7 +176,8 @@ ACCESSOR_SETTER(XMLHttpRequestOnerror) {
     if (!proxy)
       return;
 
-    EventListener* listener = proxy->FindOrCreateXHREventListener(value, false);
+    RefPtr<EventListener> listener =
+      proxy->FindOrCreateXHREventListener(value, false);
     if (listener) {
       imp->setOnErrorListener(listener);
       CreateHiddenXHRDependency(info.Holder(), value);
@@ -215,9 +217,10 @@ ACCESSOR_SETTER(XMLHttpRequestOnload)
     if (!proxy)
       return;
 
-    EventListener* listener = proxy->FindOrCreateXHREventListener(value, false);
+    RefPtr<EventListener> listener =
+      proxy->FindOrCreateXHREventListener(value, false);
     if (listener) {
-      imp->setOnload(listener);
+      imp->setOnload(listener.get());
       CreateHiddenXHRDependency(info.Holder(), value);
     }
   }
@@ -255,7 +258,8 @@ ACCESSOR_SETTER(XMLHttpRequestOnloadstart) {
     if (!proxy)
       return;
 
-    EventListener* listener = proxy->FindOrCreateXHREventListener(value, false);
+    RefPtr<EventListener> listener =
+      proxy->FindOrCreateXHREventListener(value, false);
     if (listener) {
       imp->setOnLoadStartListener(listener);
       CreateHiddenXHRDependency(info.Holder(), value);
@@ -295,7 +299,8 @@ ACCESSOR_SETTER(XMLHttpRequestOnprogress) {
     if (!proxy)
       return;
 
-    EventListener* listener = proxy->FindOrCreateXHREventListener(value, false);
+    RefPtr<EventListener> listener =
+      proxy->FindOrCreateXHREventListener(value, false);
     if (listener) {
       imp->setOnProgressListener(listener);
       CreateHiddenXHRDependency(info.Holder(), value);
@@ -336,9 +341,10 @@ ACCESSOR_SETTER(XMLHttpRequestOnreadystatechange)
     if (!proxy)
       return;
 
-    EventListener* listener = proxy->FindOrCreateXHREventListener(value, false);
+    RefPtr<EventListener> listener =
+      proxy->FindOrCreateXHREventListener(value, false);
     if (listener) {
-      imp->setOnreadystatechange(listener);
+      imp->setOnreadystatechange(listener.get());
       CreateHiddenXHRDependency(info.Holder(), value);
     }
   }
@@ -354,7 +360,8 @@ CALLBACK_FUNC_DECL(XMLHttpRequestAddEventListener)
   if (!proxy)
     return v8::Undefined();
 
-  EventListener* listener = proxy->FindOrCreateXHREventListener(args[1], false);
+  RefPtr<EventListener> listener =
+      proxy->FindOrCreateXHREventListener(args[1], false);
   if (listener) {
     String type = ToWebCoreString(args[0]);
     bool useCapture = args[2]->BooleanValue();
@@ -374,13 +381,13 @@ CALLBACK_FUNC_DECL(XMLHttpRequestRemoveEventListener) {
   if (!proxy)
     return v8::Undefined();  // probably leaked
 
-  EventListener* listener =
+  RefPtr<EventListener> listener =
     proxy->FindXHREventListener(args[1], false);
 
   if (listener) {
     String type = ToWebCoreString(args[0]);
     bool useCapture = args[2]->BooleanValue();
-    imp->removeEventListener(type, listener, useCapture);
+    imp->removeEventListener(type, listener.get(), useCapture);
 
     RemoveHiddenXHRDependency(args.Holder(), args[1]);
   }
@@ -566,7 +573,8 @@ ACCESSOR_SETTER(XMLHttpRequestUploadOnabort) {
     if (!proxy)
       return;
 
-    EventListener* listener = proxy->FindOrCreateXHREventListener(value, false);
+    RefPtr<EventListener> listener =
+      proxy->FindOrCreateXHREventListener(value, false);
     if (listener) {
       imp->setOnAbortListener(listener);
       CreateHiddenXHRDependency(info.Holder(), value);
@@ -607,7 +615,8 @@ ACCESSOR_SETTER(XMLHttpRequestUploadOnerror) {
     if (!proxy)
       return;
 
-    EventListener* listener = proxy->FindOrCreateXHREventListener(value, false);
+    RefPtr<EventListener> listener =
+      proxy->FindOrCreateXHREventListener(value, false);
     if (listener) {
       imp->setOnErrorListener(listener);
       CreateHiddenXHRDependency(info.Holder(), value);
@@ -648,7 +657,8 @@ ACCESSOR_SETTER(XMLHttpRequestUploadOnload) {
     if (!proxy)
       return;
 
-    EventListener* listener = proxy->FindOrCreateXHREventListener(value, false);
+    RefPtr<EventListener> listener =
+      proxy->FindOrCreateXHREventListener(value, false);
     if (listener) {
       imp->setOnLoadListener(listener);
       CreateHiddenXHRDependency(info.Holder(), value);
@@ -689,7 +699,8 @@ ACCESSOR_SETTER(XMLHttpRequestUploadOnloadstart) {
     if (!proxy)
       return;
 
-    EventListener* listener = proxy->FindOrCreateXHREventListener(value, false);
+    RefPtr<EventListener> listener =
+      proxy->FindOrCreateXHREventListener(value, false);
     if (listener) {
       imp->setOnLoadStartListener(listener);
       CreateHiddenXHRDependency(info.Holder(), value);
@@ -730,7 +741,8 @@ ACCESSOR_SETTER(XMLHttpRequestUploadOnprogress) {
     if (!proxy)
       return;
 
-    EventListener* listener = proxy->FindOrCreateXHREventListener(value, false);
+    RefPtr<EventListener> listener =
+      proxy->FindOrCreateXHREventListener(value, false);
     if (listener) {
       imp->setOnProgressListener(listener);
       CreateHiddenXHRDependency(info.Holder(), value);
@@ -748,7 +760,8 @@ CALLBACK_FUNC_DECL(XMLHttpRequestUploadAddEventListener) {
   if (!proxy)
     return v8::Undefined();
 
-  EventListener* listener = proxy->FindOrCreateXHREventListener(args[1], false);
+  RefPtr<EventListener> listener =
+      proxy->FindOrCreateXHREventListener(args[1], false);
   if (listener) {
     String type = ToWebCoreString(args[0]);
     bool useCapture = args[2]->BooleanValue();
@@ -769,13 +782,13 @@ CALLBACK_FUNC_DECL(XMLHttpRequestUploadRemoveEventListener) {
   if (!proxy)
     return v8::Undefined();  // probably leaked
 
-  EventListener* listener =
+  RefPtr<EventListener> listener =
     proxy->FindXHREventListener(args[1], false);
 
   if (listener) {
     String type = ToWebCoreString(args[0]);
     bool useCapture = args[2]->BooleanValue();
-    imp->removeEventListener(type, listener, useCapture);
+    imp->removeEventListener(type, listener.get(), useCapture);
 
     RemoveHiddenXHRDependency(args.Holder(), args[1]);
   }
diff --git a/webkit/port/bindings/v8/v8_custom.cpp b/webkit/port/bindings/v8/v8_custom.cpp
index 9e70dd6..3986295 100644
--- a/webkit/port/bindings/v8/v8_custom.cpp
+++ b/webkit/port/bindings/v8/v8_custom.cpp
@@ -776,7 +776,7 @@ CALLBACK_FUNC_DECL(DOMWindowAddEventListener) {
   if (!proxy)
     return v8::Undefined();
 
-  EventListener* listener =
+  RefPtr<EventListener> listener =
     proxy->FindOrCreateV8EventListener(args[1], false);
 
   if (listener) {
@@ -811,13 +811,13 @@ CALLBACK_FUNC_DECL(DOMWindowRemoveEventListener) {
   if (!proxy)
     return v8::Undefined();
 
-  EventListener* listener =
+  RefPtr<EventListener> listener =
     proxy->FindV8EventListener(args[1], false);
 
   if (listener) {
     String event_type = ToWebCoreString(args[0]);
     bool useCapture = args[2]->BooleanValue();
-    doc->removeWindowEventListener(event_type, listener, useCapture);
+    doc->removeWindowEventListener(event_type, listener.get(), useCapture);
   }
 
   return v8::Undefined();
@@ -2938,7 +2938,7 @@ CALLBACK_FUNC_DECL(EventTargetNodeAddEventListener) {
   if (!proxy)
     return v8::Undefined();
 
-  EventListener* listener =
+  RefPtr<EventListener> listener =
     proxy->FindOrCreateV8EventListener(args[1], false);
   if (listener) {
     String type = ToWebCoreString(args[0]);
@@ -2960,12 +2960,12 @@ CALLBACK_FUNC_DECL(EventTargetNodeRemoveEventListener) {
   if (!proxy)
     return v8::Undefined();
 
-  EventListener* listener =
+  RefPtr<EventListener> listener =
     proxy->FindV8EventListener(args[1], false);
   if (listener) {
     String type = ToWebCoreString(args[0]);
     bool useCapture = args[2]->BooleanValue();
-    node->removeEventListener(type, listener, useCapture);
+    node->removeEventListener(type, listener.get(), useCapture);
   }
 
   return v8::Undefined();
@@ -3160,7 +3160,7 @@ ACCESSOR_SETTER(DOMWindowEventHandler) {
     if (!proxy)
       return;
 
-    EventListener* listener =
+    RefPtr<EventListener> listener =
       proxy->FindOrCreateV8EventListener(value, true);
     if (listener) {
       doc->setHTMLWindowEventListener(event_type, listener);
@@ -3214,7 +3214,7 @@ ACCESSOR_SETTER(ElementEventHandler) {
     if (!proxy)
       return;
 
-    EventListener* listener =
+    RefPtr<EventListener> listener =
       proxy->FindOrCreateV8EventListener(value, true);
     if (listener) {
       node->setHTMLEventListener(event_type, listener);
diff --git a/webkit/port/bindings/v8/v8_events.h b/webkit/port/bindings/v8/v8_events.h
index d49be1a..72e077a 100644
--- a/webkit/port/bindings/v8/v8_events.h
+++ b/webkit/port/bindings/v8/v8_events.h
@@ -81,6 +81,11 @@ class V8AbstractEventListener : public EventListener {
 // that can handle the event.
 class V8EventListener : public V8AbstractEventListener {
  public:
+  static PassRefPtr<V8EventListener> create(Frame* frame, 
+      v8::Local<v8::Object> listener, bool html) {
+    return adoptRef(new V8EventListener(frame, listener, html));
+  }
+
   V8EventListener(Frame* frame, v8::Local<v8::Object> listener, bool html);
   virtual ~V8EventListener();
   virtual bool isHTMLEventListener() const { return m_html; }
@@ -101,6 +106,10 @@ class V8EventListener : public V8AbstractEventListener {
 // It keeps JS listener week.
 class V8XHREventListener : public V8EventListener {
  public:
+  static PassRefPtr<V8XHREventListener> create(Frame* frame, 
+      v8::Local<v8::Object> listener, bool html) {
+    return adoptRef(new V8XHREventListener(frame, listener, html));
+  }
   V8XHREventListener(Frame* frame, v8::Local<v8::Object> listener, bool html);
   virtual ~V8XHREventListener();
 };
@@ -111,6 +120,10 @@ class V8XHREventListener : public V8EventListener {
 // A V8LazyEventListener is always a HTML event handler.
 class V8LazyEventListener : public V8AbstractEventListener {
  public:
+  static PassRefPtr<V8LazyEventListener> create(Frame* frame, 
+      const String& code, const String& func_name) {
+    return adoptRef(new V8LazyEventListener(frame, code, func_name));
+  }
   V8LazyEventListener(Frame *frame, const String& code,
                       const String& func_name);
   virtual ~V8LazyEventListener();
diff --git a/webkit/port/bindings/v8/v8_proxy.cpp b/webkit/port/bindings/v8/v8_proxy.cpp
index 4e3623a..715b5ba 100644
--- a/webkit/port/bindings/v8/v8_proxy.cpp
+++ b/webkit/port/bindings/v8/v8_proxy.cpp
@@ -779,17 +779,18 @@ void V8Proxy::SetJSWrapperForDOMNode(Node* node, v8::Persistent<v8::Object> wrap
     dom_node_map().set(node, wrapper);
 }
 
-PassRefPtr<EventListener> V8Proxy::createHTMLEventHandler(const String& functionName,
+PassRefPtr<EventListener> V8Proxy::createHTMLEventHandler(
+                                               const String& functionName,
                                                const String& code, Node* node)
 {
-    return adoptRef(new V8LazyEventListener(m_frame, code, functionName));
+    return V8LazyEventListener::create(m_frame, code, functionName);
 }
 
 #if ENABLE(SVG)
 PassRefPtr<EventListener> V8Proxy::createSVGEventHandler(const String& functionName,
                                               const String& code, Node* node)
 {
-    return adoptRef(new V8LazyEventListener(m_frame, code, functionName));
+    return V8LazyEventListener::create(m_frame, code, functionName);
 }
 #endif
 
@@ -822,13 +823,13 @@ static V8EventListener* FindEventListenerInList(V8EventListenerList& list,
 }
 
 // Find an existing wrapper for a JS event listener in the map.
-V8EventListener* V8Proxy::FindV8EventListener(v8::Local<v8::Value> listener,
+PassRefPtr<V8EventListener> V8Proxy::FindV8EventListener(v8::Local<v8::Value> listener,
                                               bool html)
 {
     return FindEventListenerInList(m_event_listeners, listener, html);
 }
 
-V8EventListener* V8Proxy::FindOrCreateV8EventListener(v8::Local<v8::Value> obj, bool html)
+PassRefPtr<V8EventListener> V8Proxy::FindOrCreateV8EventListener(v8::Local<v8::Value> obj, bool html)
 {
   ASSERT(v8::Context::InContext());
 
@@ -841,9 +842,9 @@ V8EventListener* V8Proxy::FindOrCreateV8EventListener(v8::Local<v8::Value> obj,
       return wrapper;
 
   // Create a new one, and add to cache.
-  V8EventListener* new_listener =
-    new V8EventListener(m_frame, v8::Local<v8::Object>::Cast(obj), html);
-  m_event_listeners.push_back(new_listener);
+  RefPtr<V8EventListener> new_listener =
+    V8EventListener::create(m_frame, v8::Local<v8::Object>::Cast(obj), html);
+  m_event_listeners.push_back(new_listener.get());
 
   return new_listener;
 }
@@ -866,13 +867,13 @@ V8EventListener* V8Proxy::FindOrCreateV8EventListener(v8::Local<v8::Value> obj,
 // The persistent reference is made weak in the constructor
 // of V8XHREventListener.
 
-V8EventListener* V8Proxy::FindXHREventListener(v8::Local<v8::Value> listener,
+PassRefPtr<V8EventListener> V8Proxy::FindXHREventListener(v8::Local<v8::Value> listener,
                                                bool html) {
   return FindEventListenerInList(m_xhr_listeners, listener, html);
 }
 
 
-V8EventListener*
+PassRefPtr<V8EventListener>
 V8Proxy::FindOrCreateXHREventListener(v8::Local<v8::Value> obj,
                                       bool html) {
   ASSERT(v8::Context::InContext());
@@ -884,11 +885,11 @@ V8Proxy::FindOrCreateXHREventListener(v8::Local<v8::Value> obj,
   if (wrapper) return wrapper;
 
   // Create a new one, and add to cache.
-  V8EventListener* new_listener =
-    new V8XHREventListener(m_frame, v8::Local<v8::Object>::Cast(obj), html);
-  m_xhr_listeners.push_back(new_listener);
+  RefPtr<V8EventListener> new_listener =
+    V8XHREventListener::create(m_frame, v8::Local<v8::Object>::Cast(obj), html);
+  m_xhr_listeners.push_back(new_listener.get());
 
-  return new_listener;
+  return new_listener.release();
 }
 
 
diff --git a/webkit/port/bindings/v8/v8_proxy.h b/webkit/port/bindings/v8/v8_proxy.h
index bd2de40..da1bc7c 100644
--- a/webkit/port/bindings/v8/v8_proxy.h
+++ b/webkit/port/bindings/v8/v8_proxy.h
@@ -189,14 +189,14 @@ class V8Proxy {
   void clearDocumentWrapper();
 
   // Find/Create/Remove event listener wrappers.
-  V8EventListener* FindV8EventListener(v8::Local<v8::Value> listener,
+  PassRefPtr<V8EventListener> FindV8EventListener(v8::Local<v8::Value> listener,
                                        bool html);
-  V8EventListener* FindOrCreateV8EventListener(v8::Local<v8::Value> listener,
+  PassRefPtr<V8EventListener> FindOrCreateV8EventListener(v8::Local<v8::Value> listener,
                                                bool html);
 
-  V8EventListener* FindXHREventListener(v8::Local<v8::Value> listener,
+  PassRefPtr<V8EventListener> FindXHREventListener(v8::Local<v8::Value> listener,
                                         bool html);
-  V8EventListener* FindOrCreateXHREventListener(v8::Local<v8::Value> listener,
+  PassRefPtr<V8EventListener> FindOrCreateXHREventListener(v8::Local<v8::Value> listener,
                                                 bool html);
 
   void RemoveV8EventListener(V8EventListener* listener);