Commit ca341fc0 authored by dglazkov@chromium.org's avatar dglazkov@chromium.org
Browse files

2009-12-03 Dimitri Glazkov <dglazkov@chromium.org>

        Reviewed by Adam Barth.

        [V8] Attributes and NamedNodeMaps aren't tracked correctly and may be prematurely garbage-collected.
        https://bugs.webkit.org/show_bug.cgi?id=32094

        Covered by existing test: LayoutTests/fast/dom/Attr/access-after-element-destruction.html

        * bindings/v8/DOMObjectsInclude.h:
        * bindings/v8/V8DOMWrapper.cpp:
        (WebCore::V8DOMWrapper::getTemplate):
        (WebCore::V8DOMWrapper::convertToV8Object):
        (WebCore::V8DOMWrapper::convertNamedNodeMapToV8Object):
        * bindings/v8/V8DOMWrapper.h:
        * bindings/v8/V8GCController.cpp:
        (WebCore::ObjectGrouperVisitor::visitDOMWrapper):
        * bindings/v8/custom/V8CustomBinding.h:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@51638 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent ad9927f8
2009-12-03 Dimitri Glazkov <dglazkov@chromium.org>
Reviewed by Adam Barth.
[V8] Attributes and NamedNodeMaps aren't tracked correctly and may be prematurely garbage-collected.
https://bugs.webkit.org/show_bug.cgi?id=32094
Covered by existing test: LayoutTests/fast/dom/Attr/access-after-element-destruction.html
* bindings/v8/DOMObjectsInclude.h:
* bindings/v8/V8DOMWrapper.cpp:
(WebCore::V8DOMWrapper::getTemplate):
(WebCore::V8DOMWrapper::convertToV8Object):
(WebCore::V8DOMWrapper::convertNamedNodeMapToV8Object):
* bindings/v8/V8DOMWrapper.h:
* bindings/v8/V8GCController.cpp:
(WebCore::ObjectGrouperVisitor::visitDOMWrapper):
* bindings/v8/custom/V8CustomBinding.h:
2009-12-03 Pavel Feldman <pfeldman@chromium.org>
 
Reviewed by Timothy Hatcher.
......@@ -31,6 +31,7 @@
#ifndef DOMObjectsInclude_h
#define DOMObjectsInclude_h
#include "Attr.h"
#include "BarInfo.h"
#include "BeforeLoadEvent.h"
#include "WebGLActiveInfo.h"
......
......@@ -366,10 +366,15 @@ v8::Persistent<v8::FunctionTemplate> V8DOMWrapper::getTemplate(V8ClassIndex::V8W
case V8ClassIndex::MIMETYPEARRAY:
setCollectionIndexedAndNamedGetters<MimeTypeArray, MimeType>(descriptor, V8ClassIndex::MIMETYPE);
break;
case V8ClassIndex::NAMEDNODEMAP:
descriptor->InstanceTemplate()->SetNamedPropertyHandler(USE_NAMED_PROPERTY_GETTER(NamedNodeMap));
descriptor->InstanceTemplate()->SetIndexedPropertyHandler(USE_INDEXED_PROPERTY_GETTER(NamedNodeMap), 0, 0, 0, collectionIndexedPropertyEnumerator<NamedNodeMap>, v8::Integer::New(V8ClassIndex::NODE));
case V8ClassIndex::NAMEDNODEMAP: {
// We add an extra internal field to hold a reference to the owner node.
v8::Local<v8::ObjectTemplate> instanceTemplate = descriptor->InstanceTemplate();
ASSERT(instanceTemplate->InternalFieldCount() == V8Custom::kDefaultWrapperInternalFieldCount);
instanceTemplate->SetInternalFieldCount(V8Custom::kNamedNodeMapInternalFieldCount);
instanceTemplate->SetNamedPropertyHandler(USE_NAMED_PROPERTY_GETTER(NamedNodeMap));
instanceTemplate->SetIndexedPropertyHandler(USE_INDEXED_PROPERTY_GETTER(NamedNodeMap), 0, 0, 0, collectionIndexedPropertyEnumerator<NamedNodeMap>, v8::Integer::New(V8ClassIndex::NODE));
break;
}
#if ENABLE(DOM_STORAGE)
case V8ClassIndex::STORAGE:
descriptor->InstanceTemplate()->SetNamedPropertyHandler(USE_NAMED_PROPERTY_GETTER(Storage), USE_NAMED_PROPERTY_SETTER(Storage), 0, USE_NAMED_PROPERTY_DELETER(Storage), V8Custom::v8StorageNamedPropertyEnumerator);
......@@ -698,6 +703,8 @@ v8::Handle<v8::Value> V8DOMWrapper::convertToV8Object(V8ClassIndex::V8WrapperTyp
return convertStyleSheetToV8Object(static_cast<StyleSheet*>(impl));
case V8ClassIndex::DOMWINDOW:
return convertWindowToV8Object(static_cast<DOMWindow*>(impl));
case V8ClassIndex::NAMEDNODEMAP:
return convertNamedNodeMapToV8Object(static_cast<NamedNodeMap*>(impl));
#if ENABLE(SVG)
SVG_NONNODE_TYPES(MAKE_CASE)
if (type == V8ClassIndex::SVGELEMENTINSTANCE)
......@@ -1736,4 +1743,30 @@ v8::Handle<v8::Value> V8DOMWrapper::convertWindowToV8Object(DOMWindow* window)
return global;
}
v8::Handle<v8::Value> V8DOMWrapper::convertNamedNodeMapToV8Object(NamedNodeMap* map)
{
if (!map)
return v8::Null();
v8::Handle<v8::Object> wrapper = getDOMObjectMap().get(map);
if (!wrapper.IsEmpty())
return wrapper;
v8::Handle<v8::Object> result = instantiateV8Object(V8ClassIndex::NAMEDNODEMAP, V8ClassIndex::NAMEDNODEMAP, map);
if (result.IsEmpty())
return result;
// Only update the DOM object map if the result is non-empty.
map->ref();
setJSWrapperForDOMObject(map, v8::Persistent<v8::Object>::New(result));
// Add a hidden reference from named node map to its owner node.
if (Element* element = map->element()) {
v8::Handle<v8::Object> owner = v8::Handle<v8::Object>::Cast(convertNodeToV8Object(element));
result->SetInternalField(V8Custom::kNamedNodeMapOwnerNodeIndex, owner);
}
return result;
}
} // namespace WebCore
......@@ -292,6 +292,7 @@ namespace WebCore {
// Returns the JS wrapper of a window object, initializes the environment
// of the window frame if needed.
static v8::Handle<v8::Value> convertWindowToV8Object(DOMWindow*);
static v8::Handle<v8::Value> convertNamedNodeMapToV8Object(NamedNodeMap*);
#if ENABLE(SVG)
static v8::Handle<v8::Value> convertSVGElementInstanceToV8Object(SVGElementInstance*);
......
......@@ -262,14 +262,21 @@ public:
groupId = reinterpret_cast<uintptr_t>(node->document());
else {
Node* root = node;
while (root->parent())
root = root->parent();
// If the node is alone in its DOM tree (doesn't have a parent or any
// children) then the group will be filtered out later anyway.
if (root == node && !node->hasChildNodes())
return;
if (node->isAttributeNode()) {
root = static_cast<Attr*>(node)->ownerElement();
// If the attribute has no element, no need to put it in the group,
// because it'll always be a group of 1.
if (!root)
return;
} else {
while (root->parent())
root = root->parent();
// If the node is alone in its DOM tree (doesn't have a parent or any
// children) then the group will be filtered out later anyway.
if (root == node && !node->hasChildNodes() && !node->hasAttributes())
return;
}
groupId = reinterpret_cast<uintptr_t>(root);
}
m_grouper.append(GrouperItem(groupId, node, wrapper));
......
......@@ -173,6 +173,8 @@ namespace WebCore {
static const int kStyleSheetOwnerNodeIndex = kDefaultWrapperInternalFieldCount + 0;
static const int kStyleSheetInternalFieldCount = kDefaultWrapperInternalFieldCount + 1;
static const int kNamedNodeMapOwnerNodeIndex = kDefaultWrapperInternalFieldCount + 0;
static const int kNamedNodeMapInternalFieldCount = kDefaultWrapperInternalFieldCount + 1;
#if ENABLE(OFFLINE_WEB_APPLICATIONS)
static const int kDOMApplicationCacheCacheIndex = kDefaultWrapperInternalFieldCount + 0;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment