- Revision
- 128102
- Author
- [email protected]
- Date
- 2012-09-10 13:54:43 -0700 (Mon, 10 Sep 2012)
Log Message
[V8] setNamedHiddenWindowReference doesn't need to be a special case
https://bugs.webkit.org/show_bug.cgi?id=96198
Reviewed by Nate Chapin.
Prior to this patch, the DOMWindow cached its properties on the outter
global object rather than on the Holder of the properties. (We cache
properties to prevent their DOM wrappers from being garbage collected
too early.) There doesn't seem to be any reason why DOMWindow need to
be special-cased in this regard. We can just cache the properities on
their Holders, as usual.
* bindings/scripts/CodeGeneratorV8.pm:
(GenerateNormalAttrGetter):
* bindings/v8/V8DOMWrapper.cpp:
* bindings/v8/V8DOMWrapper.h:
(V8DOMWrapper):
* bindings/v8/custom/V8LocationCustom.cpp:
* page/Location.idl:
- Previously, Location had a custom toV8 function so that
document.location would cache its wrapper in the same place as
window.location. However, that's no longer necessary as the
DOMWindow now holds its Document in a hidden property, which
means the DOMWindow keeps the cached location property on
Document alive anyway.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (128101 => 128102)
--- trunk/Source/WebCore/ChangeLog 2012-09-10 20:52:04 UTC (rev 128101)
+++ trunk/Source/WebCore/ChangeLog 2012-09-10 20:54:43 UTC (rev 128102)
@@ -1,3 +1,31 @@
+2012-09-10 Adam Barth <[email protected]>
+
+ [V8] setNamedHiddenWindowReference doesn't need to be a special case
+ https://bugs.webkit.org/show_bug.cgi?id=96198
+
+ Reviewed by Nate Chapin.
+
+ Prior to this patch, the DOMWindow cached its properties on the outter
+ global object rather than on the Holder of the properties. (We cache
+ properties to prevent their DOM wrappers from being garbage collected
+ too early.) There doesn't seem to be any reason why DOMWindow need to
+ be special-cased in this regard. We can just cache the properities on
+ their Holders, as usual.
+
+ * bindings/scripts/CodeGeneratorV8.pm:
+ (GenerateNormalAttrGetter):
+ * bindings/v8/V8DOMWrapper.cpp:
+ * bindings/v8/V8DOMWrapper.h:
+ (V8DOMWrapper):
+ * bindings/v8/custom/V8LocationCustom.cpp:
+ * page/Location.idl:
+ - Previously, Location had a custom toV8 function so that
+ document.location would cache its wrapper in the same place as
+ window.location. However, that's no longer necessary as the
+ DOMWindow now holds its Document in a hidden property, which
+ means the DOMWindow keeps the cached location property on
+ Document alive anyway.
+
2012-09-10 Anders Carlsson <[email protected]>
Silence a warning when running update-webkit-localizable-strings
Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm (128101 => 128102)
--- trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm 2012-09-10 20:52:04 UTC (rev 128101)
+++ trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm 2012-09-10 20:54:43 UTC (rev 128102)
@@ -984,7 +984,7 @@
# Special case for readonly or Replaceable attributes (with a few exceptions). This attempts to ensure that JS wrappers don't get
# garbage-collected prematurely when their lifetime is strongly tied to their owner. We accomplish this by inserting a reference to
# the newly created wrapper into an internal field of the holder object.
- if (!IsNodeSubType($dataNode) && $attrName ne "self" && (IsWrapperType($returnType) && ($attribute->type =~ /^readonly/ || $attribute->signature->extendedAttributes->{"Replaceable"})
+ if (!IsNodeSubType($dataNode) && $attrName ne "self" && (IsWrapperType($returnType) && ($attribute->type =~ /^readonly/ || $attribute->signature->extendedAttributes->{"Replaceable"} || $attrName eq "location")
&& $returnType ne "EventTarget" && $returnType ne "SerializedScriptValue" && $returnType ne "DOMWindow"
&& $returnType ne "MessagePortArray"
&& $returnType !~ /SVG/ && $returnType !~ /HTML/ && !IsDOMNodeType($returnType))) {
@@ -1009,12 +1009,7 @@
push(@implContentDecls, " if (wrapper.IsEmpty()) {\n");
push(@implContentDecls, " wrapper = toV8(result.get(), info.Holder(), info.GetIsolate());\n");
push(@implContentDecls, " if (!wrapper.IsEmpty())\n");
- if ($dataNode->name eq "DOMWindow") {
- AddToImplIncludes("Frame.h");
- push(@implContentDecls, " V8DOMWrapper::setNamedHiddenWindowReference(imp->frame(), \"${attrName}\", wrapper);\n");
- } else {
- push(@implContentDecls, " V8DOMWrapper::setNamedHiddenReference(info.Holder(), \"${attrName}\", wrapper);\n");
- }
+ push(@implContentDecls, " V8DOMWrapper::setNamedHiddenReference(info.Holder(), \"${attrName}\", wrapper);\n");
push(@implContentDecls, " }\n");
push(@implContentDecls, " return wrapper;\n");
push(@implContentDecls, "}\n\n");
Modified: trunk/Source/WebCore/bindings/v8/V8DOMWrapper.cpp (128101 => 128102)
--- trunk/Source/WebCore/bindings/v8/V8DOMWrapper.cpp 2012-09-10 20:52:04 UTC (rev 128101)
+++ trunk/Source/WebCore/bindings/v8/V8DOMWrapper.cpp 2012-09-10 20:54:43 UTC (rev 128102)
@@ -121,23 +121,6 @@
parent->SetHiddenValue(V8HiddenPropertyName::hiddenReferenceName(name), child);
}
-void V8DOMWrapper::setNamedHiddenWindowReference(Frame* frame, const char* name, v8::Handle<v8::Value> jsObject)
-{
- // Get DOMWindow
- if (!frame)
- return; // Object might be detached from window
- v8::Handle<v8::Context> context = frame->script()->currentWorldContext();
- if (context.IsEmpty())
- return;
-
- v8::Handle<v8::Object> global = context->Global();
- // Look for real DOM wrapper.
- global = V8DOMWrapper::lookupDOMWrapper(V8DOMWindow::GetTemplate(), global);
- ASSERT(!global.IsEmpty());
-
- setNamedHiddenReference(global, name, jsObject);
-}
-
WrapperTypeInfo* V8DOMWrapper::domWrapperType(v8::Handle<v8::Object> object)
{
ASSERT(V8DOMWrapper::maybeDOMWrapper(object));
Modified: trunk/Source/WebCore/bindings/v8/V8DOMWrapper.h (128101 => 128102)
--- trunk/Source/WebCore/bindings/v8/V8DOMWrapper.h 2012-09-10 20:52:04 UTC (rev 128101)
+++ trunk/Source/WebCore/bindings/v8/V8DOMWrapper.h 2012-09-10 20:54:43 UTC (rev 128102)
@@ -110,14 +110,7 @@
// Check whether a V8 value is a wrapper of type |classType|.
static bool isWrapperOfType(v8::Handle<v8::Value>, WrapperTypeInfo*);
- // Proper object lifetime support.
- //
- // Helper functions to make sure the child object stays alive
- // while the parent is alive. Using the name more than once
- // overwrites previous references making it possible to free
- // old children.
static void setNamedHiddenReference(v8::Handle<v8::Object> parent, const char* name, v8::Handle<v8::Value> child);
- static void setNamedHiddenWindowReference(Frame*, const char*, v8::Handle<v8::Value>);
static v8::Local<v8::Object> instantiateV8Object(WrapperTypeInfo*, void*);
Modified: trunk/Source/WebCore/bindings/v8/custom/V8LocationCustom.cpp (128101 => 128102)
--- trunk/Source/WebCore/bindings/v8/custom/V8LocationCustom.cpp 2012-09-10 20:52:04 UTC (rev 128101)
+++ trunk/Source/WebCore/bindings/v8/custom/V8LocationCustom.cpp 2012-09-10 20:54:43 UTC (rev 128102)
@@ -270,17 +270,4 @@
return BindingSecurity::shouldAllowAccessToFrame(BindingState::instance(), imp->frame(), DoNotReportSecurityError);
}
-v8::Handle<v8::Value> toV8(Location* impl, v8::Handle<v8::Object> creationContext, v8::Isolate* isolate)
-{
- if (!impl)
- return v8NullWithCheck(isolate);
- v8::Handle<v8::Object> wrapper = getDOMObjectMap().get(impl);
- if (wrapper.IsEmpty()) {
- wrapper = V8Location::wrap(impl, creationContext, isolate);
- if (!wrapper.IsEmpty())
- V8DOMWrapper::setNamedHiddenWindowReference(impl->frame(), "location", wrapper);
- }
- return wrapper;
-}
-
} // namespace WebCore
Modified: trunk/Source/WebCore/page/Location.idl (128101 => 128102)
--- trunk/Source/WebCore/page/Location.idl 2012-09-10 20:52:04 UTC (rev 128101)
+++ trunk/Source/WebCore/page/Location.idl 2012-09-10 20:54:43 UTC (rev 128102)
@@ -40,8 +40,7 @@
JSCustomDefineOwnProperty,
JSCustomNamedGetterOnPrototype,
JSCustomDefineOwnPropertyOnPrototype,
- OmitConstructor,
- V8CustomToJSObject
+ OmitConstructor
] Location {
#if !defined(LANGUAGE_CPP) || !LANGUAGE_CPP
attribute [DoNotCheckSecurityOnSetter, CustomSetter, V8Unforgeable] DOMString href;
