[webkit-reviews] review granted: [Bug 35245] [V8] Clean up V8DOMWrapper::getEventListener() : [Attachment 49224] patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Feb 22 10:46:35 PST 2010


Dimitri Glazkov (Google) <dglazkov at chromium.org> has granted Nate Chapin
<japhet at chromium.org>'s request for review:
Bug 35245: [V8] Clean up V8DOMWrapper::getEventListener()
https://bugs.webkit.org/show_bug.cgi?id=35245

Attachment 49224: patch
https://bugs.webkit.org/attachment.cgi?id=49224&action=review

------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
Awesome, with comments:

> +  static v8::Handle<v8::Value> ${functionName}EventListenerCallback(const
v8::Arguments& args)
> +  {
> +    INC_STATS("DOM.${implClassName}.${functionName}EventListener()");
> +    RefPtr<EventListener> listener = V8DOMWrapper::getEventListener(args[1],
false, ListenerFind${lookupType});
> +    if (listener) {
> +	
V8${implClassName}::toNative(args.Holder())->${functionName}EventListener(v8Val
ueToAtomicWebCoreString(args[0]), listener${passRefPtrHandling},
args[2]->BooleanValue());
> +	 ${hiddenDependencyAction}HiddenDependency(args.Holder(), args[1],
V8${implClassName}::cacheIndex);
> +    }
> +    return v8::Undefined();
> +  }

Can haz proper 4-space indent here? I know we seem to generate 2-spacers, but
that should change too.

> +    if (WorkerContextExecutionProxy::retrieve())
> +	   return
V8EventListenerList::findOrCreateWrapper<V8WorkerContextEventListener>(value,
isAttribute);

Can we not get rid of this using your new fancy trick? Or is this already using
it?


More information about the webkit-reviews mailing list