Bug 25496 – Switch V8EventListenerList to a hashtable implementation

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Antony Sargent]

This avoids some tricky issues with event listener removal in the current implmentation and has slightly better performance.

http://codereview.chromium.org/79059

[Antony Sargent]

Created attachment 29925 [details]
patch

[Antony Sargent]

Created attachment 29933 [details]
patch v2

A few fixes to comply with WebKit style guidelines.

[Antony Sargent]

I'll be uploading a new version in a little while with one more small change based on offline discussion with dglazkov.

[Antony Sargent]

Created attachment 29950 [details]
patch v3

Changed the iterator from being a nested class to it's own standalone class.

[Dimitri Glazkov (Google)]

Comment on attachment 29950 [details]
patch v3

You did good! Just a few comments below.

> +        No new functionality so no new tests.
> +
> +        * bindings/v8/V8EventListenerList.cpp:
> +        (WebCore::V8EventListenerListIterator::V8EventListenerListIterator):
> +        (WebCore::V8EventListenerListIterator::~V8EventListenerListIterator):
> +        (WebCore::V8EventListenerListIterator::operator++):
> +        (WebCore::V8EventListenerListIterator::operator==):
> +        (WebCore::V8EventListenerListIterator::operator!=):
> +        (WebCore::V8EventListenerListIterator::operator*):
> +        (WebCore::V8EventListenerListIterator::seekToEnd):
> +        (WebCore::V8EventListenerList::V8EventListenerList):
> +        (WebCore::V8EventListenerList::~V8EventListenerList):
> +        (WebCore::V8EventListenerList::begin):
> +        (WebCore::V8EventListenerList::end):
> +        (WebCore::getKey):
> +        (WebCore::V8EventListenerList::add):
> +        (WebCore::V8EventListenerList::remove):
> +        (WebCore::V8EventListenerList::clear):
> +        (WebCore::V8EventListenerList::find):

No need to leave all these hanging. Can just say:

 +        * bindings/v8/V8EventListenerList.cpp: Added V8EventListenerListIterator.

> +int getKey(v8::Local<v8::Object> object)

static int getKey(...

[Antony Sargent]

Created attachment 29995 [details]
patch v4

New Patch in response to Dmitri's last comments. Contains less verbose ChangeLog comment, and switched getKey function to static.

[Eric Seidel (no email)]

Dimitri - Would you like to land this, or should I?

[Eric Seidel (no email)]

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/bindings/v8/V8EventListenerList.cpp
	M	WebCore/bindings/v8/V8EventListenerList.h
	M	WebCore/bindings/v8/WorkerContextExecutionProxy.cpp
Committed r43277