Changeset 128102 in webkit


Ignore:
Timestamp:
Sep 10, 2012 1:54:43 PM (12 years ago)
Author:
abarth@webkit.org
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.
Location:
trunk/Source/WebCore
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r128098 r128102  
     12012-09-10  Adam Barth  <abarth@webkit.org>
     2
     3        [V8] setNamedHiddenWindowReference doesn't need to be a special case
     4        https://bugs.webkit.org/show_bug.cgi?id=96198
     5
     6        Reviewed by Nate Chapin.
     7
     8        Prior to this patch, the DOMWindow cached its properties on the outter
     9        global object rather than on the Holder of the properties. (We cache
     10        properties to prevent their DOM wrappers from being garbage collected
     11        too early.) There doesn't seem to be any reason why DOMWindow need to
     12        be special-cased in this regard. We can just cache the properities on
     13        their Holders, as usual.
     14
     15        * bindings/scripts/CodeGeneratorV8.pm:
     16        (GenerateNormalAttrGetter):
     17        * bindings/v8/V8DOMWrapper.cpp:
     18        * bindings/v8/V8DOMWrapper.h:
     19        (V8DOMWrapper):
     20        * bindings/v8/custom/V8LocationCustom.cpp:
     21        * page/Location.idl:
     22            - Previously, Location had a custom toV8 function so that
     23              document.location would cache its wrapper in the same place as
     24              window.location. However, that's no longer necessary as the
     25              DOMWindow now holds its Document in a hidden property, which
     26              means the DOMWindow keeps the cached location property on
     27              Document alive anyway.
     28
    1292012-09-10  Anders Carlsson  <andersca@apple.com>
    230

  • trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm

    r128094 r128102  
    985985    # garbage-collected prematurely when their lifetime is strongly tied to their owner. We accomplish this by inserting a reference to
    986986    # the newly created wrapper into an internal field of the holder object.
    987     if (!IsNodeSubType($dataNode) && $attrName ne "self" && (IsWrapperType($returnType) && ($attribute->type =~ /^readonly/ || $attribute->signature->extendedAttributes->{"Replaceable"})
     987    if (!IsNodeSubType($dataNode) && $attrName ne "self" && (IsWrapperType($returnType) && ($attribute->type =~ /^readonly/ || $attribute->signature->extendedAttributes->{"Replaceable"} || $attrName eq "location")
    988988        && $returnType ne "EventTarget" && $returnType ne "SerializedScriptValue" && $returnType ne "DOMWindow"
    989989        && $returnType ne "MessagePortArray"
    … …  
    10101010        push(@implContentDecls, "        wrapper = toV8(result.get(), info.Holder(), info.GetIsolate());\n");
    10111011        push(@implContentDecls, "        if (!wrapper.IsEmpty())\n");
    1012         if ($dataNode->name eq "DOMWindow") {
    1013             AddToImplIncludes("Frame.h");
    1014             push(@implContentDecls, "            V8DOMWrapper::setNamedHiddenWindowReference(imp->frame(), \"${attrName}\", wrapper);\n");
    1015         } else {
    1016             push(@implContentDecls, "            V8DOMWrapper::setNamedHiddenReference(info.Holder(), \"${attrName}\", wrapper);\n");
    1017         }
     1012        push(@implContentDecls, "            V8DOMWrapper::setNamedHiddenReference(info.Holder(), \"${attrName}\", wrapper);\n");
    10181013        push(@implContentDecls, "    }\n");
    10191014        push(@implContentDecls, "    return wrapper;\n");

  • trunk/Source/WebCore/bindings/v8/V8DOMWrapper.cpp

    r127972 r128102  
    122122}
    123123
    124 void V8DOMWrapper::setNamedHiddenWindowReference(Frame* frame, const char* name, v8::Handle<v8::Value> jsObject)
    125 {
    126     // Get DOMWindow
    127     if (!frame)
    128         return; // Object might be detached from window
    129     v8::Handle<v8::Context> context = frame->script()->currentWorldContext();
    130     if (context.IsEmpty())
    131         return;
    132 
    133     v8::Handle<v8::Object> global = context->Global();
    134     // Look for real DOM wrapper.
    135     global = V8DOMWrapper::lookupDOMWrapper(V8DOMWindow::GetTemplate(), global);
    136     ASSERT(!global.IsEmpty());
    137 
    138     setNamedHiddenReference(global, name, jsObject);
    139 }
    140 
    141124WrapperTypeInfo* V8DOMWrapper::domWrapperType(v8::Handle<v8::Object> object)
    142125{

  • trunk/Source/WebCore/bindings/v8/V8DOMWrapper.h

    r127997 r128102  
    111111        static bool isWrapperOfType(v8::Handle<v8::Value>, WrapperTypeInfo*);
    112112
    113         // Proper object lifetime support.
    114         //
    115         // Helper functions to make sure the child object stays alive
    116         // while the parent is alive. Using the name more than once
    117         // overwrites previous references making it possible to free
    118         // old children.
    119113        static void setNamedHiddenReference(v8::Handle<v8::Object> parent, const char* name, v8::Handle<v8::Value> child);
    120         static void setNamedHiddenWindowReference(Frame*, const char*, v8::Handle<v8::Value>);
    121114
    122115        static v8::Local<v8::Object> instantiateV8Object(WrapperTypeInfo*, void*);

  • trunk/Source/WebCore/bindings/v8/custom/V8LocationCustom.cpp

    r127946 r128102  
    271271}
    272272
    273 v8::Handle<v8::Value> toV8(Location* impl, v8::Handle<v8::Object> creationContext, v8::Isolate* isolate)
    274 {
    275     if (!impl)
    276         return v8NullWithCheck(isolate);
    277     v8::Handle<v8::Object> wrapper = getDOMObjectMap().get(impl);
    278     if (wrapper.IsEmpty()) {
    279         wrapper = V8Location::wrap(impl, creationContext, isolate);
    280         if (!wrapper.IsEmpty())
    281             V8DOMWrapper::setNamedHiddenWindowReference(impl->frame(), "location", wrapper);
    282     }
    283     return wrapper;
    284 }
    285 
    286273}  // namespace WebCore

  • trunk/Source/WebCore/page/Location.idl

    r113945 r128102  
    4141        JSCustomNamedGetterOnPrototype,
    4242        JSCustomDefineOwnPropertyOnPrototype,
    43         OmitConstructor,
    44         V8CustomToJSObject
     43        OmitConstructor
    4544    ] Location {
    4645#if !defined(LANGUAGE_CPP) || !LANGUAGE_CPP
Note: See TracChangeset for help on using the changeset viewer.