Bug 24951 – Upstream V8DOMMap for v8 bindings.

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Jian Li]

Upstream V8DOMMap for v8 bindings.

[Jian Li]

Created attachment 29096 [details]
Proposed Patch

[Darin Fisher (:fishd, Google)]

Comment on attachment 29096 [details]
Proposed Patch

> +++ b/WebCore/ChangeLog
...
> +        * bindings/v8/V8DOMMap.cpp: Added.
> +        (WebCore::domThreadMap):
> +        (WebCore::domThreadMapMutex):
> +        (WebCore::ThreadSpecificDOMData::):
> +        (WebCore::ThreadSpecificDOMData::InternalDOMWrapperMap::InternalDOMWrapperMap):

nit: when adding a new file to the repository, please delete these lines
that prepare-ChangeLog adds for each added method.


> +++ b/WebCore/bindings/v8/V8DOMMap.cpp

> +// DOM binding algorithm:
> +//
> +// There are two kinds of DOM objects:
> +// 1. DOM tree nodes, such as Document, HTMLElement, ...
> +//    there classes implements TreeShared<T> interface;

nit: perhaps this would be better written as: "these classes implement the TreeShared<T> interface" ?


> +//   Handles from DOM objects to JS wrappers are always weak,
> +// so JS wrappers of non-node object cannot create a cycle.

nit: I think this comment is referring to Peerable, which no longer exists.  It might
be good to check with the V8 guys.


> +// This encapsulates thread-specific DOM data for the main thread. All the maps in it are static.
> +// This is because we are unable to rely on WTF::ThreadSpecificThreadExit to do the cleanup since the place that tears down the main thread can not call any WTF functions.

nit: it might be nice to add a line break to the second line of the comment so that each line of
the comment block is approx the same width.


otherwise LGTM

[Jian Li]

Created attachment 29130 [details]
Proposed Patch

Here is the new patch that fixed all Darin's comment. Please use this to land.

[Dmitry Titov]

Landed: http://trac.webkit.org/changeset/42135