Changeset 199017 in webkit


Ignore:
Timestamp:
Apr 4, 2016 1:01:38 PM (8 years ago)
Author:
Chris Dumez
Message:

Regression(r196145): Crash in getOwnPropertyDescriptor on http://www.history.com/shows/vikings
https://bugs.webkit.org/show_bug.cgi?id=156136
<rdar://problem/25410767>

Reviewed by Ryosuke Niwa.

Source/JavaScriptCore:

Add a few more identifiers for using in the generated bindings.

  • runtime/CommonIdentifiers.h:

Source/WebCore:

The page was crashing when doing the following:
Object.getOwnPropertyDescriptor(window, "indexedDB")

getOwnPropertyDescriptor() expected getDirect() to return a CustomGetterSetter for
CustomAccessors but it was not the case for window.indexedDB. The reason was that
window.indexedDB was a special property, which is not part of the static table but
returned by GetOwnPropertySlot() if IndexedDB feature is enabled. This weirdness
was due to our bindings generator not having proper support for [EnabledAtRuntime]
properties on Window.

This patch adds support for [EnabledAtRuntime] properties on Window by omitting
these properties from the static property table and then setting them at runtime
in JSDOMWindow::finishCreation() if the corresponding feature is enabled.
window.indexedDB now looks like a regular property when IndexedDB is enabled
and getOwnPropertyDescriptor() works as expected for this property.

Test: storage/indexeddb/indexeddb-getownpropertyDescriptor.html

  • Modules/indexeddb/DOMWindowIndexedDatabase.cpp:

(WebCore::DOMWindowIndexedDatabase::indexedDB):

  • Modules/indexeddb/DOMWindowIndexedDatabase.h:

The generated bindings pass DOMWindow by reference instead of pointer so update
the implementation accordingly.

  • Modules/indexeddb/DOMWindowIndexedDatabase.idl:

Add 'indexedDB' and 'webkitIndexedDB' properties and mark them as
[EnabledAtRuntime]. Now that the bindings generator correctly handles
[EnabledAtRuntime] properties on the Window, there is no need to
custom-handle them in JSDOMWindowCustom.

  • bindings/js/JSDOMWindowCustom.cpp:

Drop custom handling for 'indexedDB' and 'webkitIndexedDB' properties
in getOwnPropertySlot(). The generated bindings code now makes sure to
only set those properties on the Window if IndexedDB is enabled so we
can let the regular code path look up those properties.

  • bindings/scripts/CodeGeneratorJS.pm:

(GetJSCAttributesForAttribute):
(GenerateHeader):
(GeneratePropertiesHashTable):
(GenerateImplementation):
Add support for [EnabledAtRuntime] properties on DOMWindow. For such
properties, we do the following:

  1. Omit them from the static property table
  2. In JSDOMWindow::finishCreation(), dynamically add those properties at runtime if the corresponding feature is enabled.

Note that this works for constructors as well.

  • inspector/InspectorIndexedDBAgent.cpp:

(WebCore::assertIDBFactory):
Pass Window by reference instead of pointer.

LayoutTests:

Add a layout test to confirm that calling Object.getOwnPropertyDescriptor(window, "indexedDB")
does not crash and works as expected.

  • storage/indexeddb/indexeddb-getownpropertyDescriptor-expected.txt: Added.
  • storage/indexeddb/indexeddb-getownpropertyDescriptor.html: Added.
Location:
trunk
Files:
2 added
10 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r199013 r199017  
     12016-04-04  Chris Dumez  <cdumez@apple.com>
     2
     3        Regression(r196145): Crash in getOwnPropertyDescriptor on http://www.history.com/shows/vikings
     4        https://bugs.webkit.org/show_bug.cgi?id=156136
     5        <rdar://problem/25410767>
     6
     7        Reviewed by Ryosuke Niwa.
     8
     9        Add a layout test to confirm that calling Object.getOwnPropertyDescriptor(window, "indexedDB")
     10        does not crash and works as expected.
     11
     12        * storage/indexeddb/indexeddb-getownpropertyDescriptor-expected.txt: Added.
     13        * storage/indexeddb/indexeddb-getownpropertyDescriptor.html: Added.
     14
    1152016-04-04  Ryan Haddad  <ryanhaddad@apple.com>
    216
  • trunk/Source/JavaScriptCore/ChangeLog

    r199016 r199017  
     12016-04-04  Chris Dumez  <cdumez@apple.com>
     2
     3        Regression(r196145): Crash in getOwnPropertyDescriptor on http://www.history.com/shows/vikings
     4        https://bugs.webkit.org/show_bug.cgi?id=156136
     5        <rdar://problem/25410767>
     6
     7        Reviewed by Ryosuke Niwa.
     8
     9        Add a few more identifiers for using in the generated bindings.
     10
     11        * runtime/CommonIdentifiers.h:
     12
    1132016-04-04  Geoffrey Garen  <ggaren@apple.com>
    214
  • trunk/Source/JavaScriptCore/runtime/CommonIdentifiers.h

    r198981 r199017  
    2929// ways without repeating the list.
    3030#define JSC_COMMON_IDENTIFIERS_EACH_PROPERTY_NAME(macro) \
     31    macro(AnimationTimeline) \
    3132    macro(Array) \
    3233    macro(ArrayBuffer) \
    3334    macro(ArrayIterator) \
     35    macro(Audio) \
    3436    macro(BYTES_PER_ELEMENT) \
    3537    macro(Boolean) \
     
    3739    macro(Date) \
    3840    macro(DateTimeFormat) \
     41    macro(DocumentTimeline) \
    3942    macro(Error) \
    4043    macro(EvalError) \
    4144    macro(Function) \
     45    macro(Gamepad) \
     46    macro(GamepadButton) \
     47    macro(GamepadEvent) \
    4248    macro(GeneratorFunction) \
     49    macro(HTMLAudioElement) \
     50    macro(HTMLSlotElement) \
     51    macro(IDBCursor) \
     52    macro(IDBCursorWithValue) \
     53    macro(IDBDatabase) \
     54    macro(IDBFactory) \
     55    macro(IDBIndex) \
     56    macro(IDBKeyRange) \
     57    macro(IDBObjectStore) \
     58    macro(IDBOpenDBRequest) \
     59    macro(IDBRequest) \
     60    macro(IDBTransaction) \
     61    macro(IDBVersionChangeEvent) \
    4362    macro(Infinity) \
    4463    macro(Intl) \
     
    6079    macro(Set)\
    6180    macro(SetIterator)\
     81    macro(ShadowRoot) \
    6282    macro(String) \
    6383    macro(Symbol) \
     
    6888    macro(WeakMap)\
    6989    macro(WeakSet)\
     90    macro(WebSocket) \
    7091    macro(__defineGetter__) \
    7192    macro(__defineSetter__) \
     
    219240    macro(values) \
    220241    macro(webkit) \
     242    macro(webkitIDBCursor) \
     243    macro(webkitIDBDatabase) \
     244    macro(webkitIDBFactory) \
     245    macro(webkitIDBIndex) \
     246    macro(webkitIDBKeyRange) \
     247    macro(webkitIDBObjectStore) \
     248    macro(webkitIDBRequest) \
     249    macro(webkitIDBTransaction) \
    221250    macro(webkitIndexedDB) \
    222251    macro(weekday) \
  • trunk/Source/WebCore/ChangeLog

    r199015 r199017  
     12016-04-04  Chris Dumez  <cdumez@apple.com>
     2
     3        Regression(r196145): Crash in getOwnPropertyDescriptor on http://www.history.com/shows/vikings
     4        https://bugs.webkit.org/show_bug.cgi?id=156136
     5        <rdar://problem/25410767>
     6
     7        Reviewed by Ryosuke Niwa.
     8
     9        The page was crashing when doing the following:
     10        Object.getOwnPropertyDescriptor(window, "indexedDB")
     11
     12        getOwnPropertyDescriptor() expected getDirect() to return a CustomGetterSetter for
     13        CustomAccessors but it was not the case for window.indexedDB. The reason was that
     14        window.indexedDB was a special property, which is not part of the static table but
     15        returned by GetOwnPropertySlot() if IndexedDB feature is enabled. This weirdness
     16        was due to our bindings generator not having proper support for [EnabledAtRuntime]
     17        properties on Window.
     18
     19        This patch adds support for [EnabledAtRuntime] properties on Window by omitting
     20        these properties from the static property table and then setting them at runtime
     21        in JSDOMWindow::finishCreation() if the corresponding feature is enabled.
     22        window.indexedDB now looks like a regular property when IndexedDB is enabled
     23        and getOwnPropertyDescriptor() works as expected for this property.
     24
     25        Test: storage/indexeddb/indexeddb-getownpropertyDescriptor.html
     26
     27        * Modules/indexeddb/DOMWindowIndexedDatabase.cpp:
     28        (WebCore::DOMWindowIndexedDatabase::indexedDB):
     29        * Modules/indexeddb/DOMWindowIndexedDatabase.h:
     30        The generated bindings pass DOMWindow by reference instead of pointer so update
     31        the implementation accordingly.
     32
     33        * Modules/indexeddb/DOMWindowIndexedDatabase.idl:
     34        Add 'indexedDB' and 'webkitIndexedDB' properties and mark them as
     35        [EnabledAtRuntime]. Now that the bindings generator correctly handles
     36        [EnabledAtRuntime] properties on the Window, there is no need to
     37        custom-handle them in JSDOMWindowCustom.
     38
     39        * bindings/js/JSDOMWindowCustom.cpp:
     40        Drop custom handling for 'indexedDB' and 'webkitIndexedDB' properties
     41        in getOwnPropertySlot(). The generated bindings code now makes sure to
     42        only set those properties on the Window if IndexedDB is enabled so we
     43        can let the regular code path look up those properties.
     44
     45        * bindings/scripts/CodeGeneratorJS.pm:
     46        (GetJSCAttributesForAttribute):
     47        (GenerateHeader):
     48        (GeneratePropertiesHashTable):
     49        (GenerateImplementation):
     50        Add support for [EnabledAtRuntime] properties on DOMWindow. For such
     51        properties, we do the following:
     52        1. Omit them from the static property table
     53        2. In JSDOMWindow::finishCreation(), dynamically add those properties
     54           at runtime if the corresponding feature is enabled.
     55
     56        Note that this works for constructors as well.
     57
     58        * inspector/InspectorIndexedDBAgent.cpp:
     59        (WebCore::assertIDBFactory):
     60        Pass Window by reference instead of pointer.
     61
    1622016-04-04  Myles C. Maxfield  <mmaxfield@apple.com>
    263
  • trunk/Source/WebCore/Modules/indexeddb/DOMWindowIndexedDatabase.cpp

    r198762 r199017  
    9494}
    9595
    96 IDBFactory* DOMWindowIndexedDatabase::indexedDB(DOMWindow* window)
     96IDBFactory* DOMWindowIndexedDatabase::indexedDB(DOMWindow& window)
    9797{
    98     return from(window)->indexedDB();
     98    return from(&window)->indexedDB();
    9999}
    100100
  • trunk/Source/WebCore/Modules/indexeddb/DOMWindowIndexedDatabase.h

    r197563 r199017  
    4545    static DOMWindowIndexedDatabase* from(DOMWindow*);
    4646
    47     static IDBFactory* indexedDB(DOMWindow*);
     47    static IDBFactory* indexedDB(DOMWindow&);
    4848
    4949    void disconnectFrameForDocumentSuspension() override;
  • trunk/Source/WebCore/Modules/indexeddb/DOMWindowIndexedDatabase.idl

    r185322 r199017  
    2828    Conditional=INDEXED_DATABASE,
    2929] partial interface DOMWindow {
    30     // This space is intentionally left blank.
     30    [EnabledAtRuntime=IndexedDB] readonly attribute IDBFactory indexedDB;
     31    [EnabledAtRuntime=IndexedDB, ImplementedAs=indexedDB] readonly attribute IDBFactory webkitIndexedDB;
    3132};
    3233
  • trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp

    r198023 r199017  
    7777#endif
    7878
    79 #if ENABLE(INDEXED_DATABASE)
    80 static EncodedJSValue jsDOMWindowIndexedDB(ExecState* exec, EncodedJSValue thisValue, PropertyName)
    81 {
    82     UNUSED_PARAM(exec);
    83     auto* castedThis = toJSDOMWindow(JSValue::decode(thisValue));
    84     if (!RuntimeEnabledFeatures::sharedFeatures().indexedDBEnabled())
    85         return JSValue::encode(jsUndefined());
    86     if (!castedThis || !BindingSecurity::shouldAllowAccessToDOMWindow(exec, castedThis->wrapped()))
    87         return JSValue::encode(jsUndefined());
    88     auto& impl = castedThis->wrapped();
    89     JSValue result = toJS(exec, castedThis->globalObject(), WTF::getPtr(DOMWindowIndexedDatabase::indexedDB(&impl)));
    90     return JSValue::encode(result);
    91 }
    92 #endif
    93 
    9479static bool jsDOMWindowGetOwnPropertySlotRestrictedAccess(JSDOMWindow* thisObject, Frame* frame, ExecState* exec, PropertyName propertyName, PropertySlot& slot, String& errorMessage)
    9580{
     
    288273        return true;
    289274
    290 #if ENABLE(INDEXED_DATABASE)
    291     // FIXME: With generated JS bindings built on static property tables there is no way to
    292     // completely remove a generated property at runtime. So to completely disable IndexedDB
    293     // at runtime we have to not generate these accessors and have to handle them specially here.
    294     // Once https://webkit.org/b/145669 is resolved, they can once again be auto generated.
    295     if (RuntimeEnabledFeatures::sharedFeatures().indexedDBEnabled() && (propertyName == exec->propertyNames().indexedDB || propertyName == exec->propertyNames().webkitIndexedDB)) {
    296         slot.setCustom(thisObject, DontDelete | ReadOnly | CustomAccessor, jsDOMWindowIndexedDB);
    297         return true;
    298     }
    299 #endif
    300275#if ENABLE(USER_MESSAGE_HANDLERS)
    301276    if (propertyName == exec->propertyNames().webkit && thisObject->wrapped().shouldHaveWebKitNamespaceForWorld(thisObject->world())) {
  • trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm

    r199012 r199017  
    732732
    733733    return 0;
     734}
     735
     736sub GetJSCAttributesForAttribute
     737{
     738    my $interface = shift;
     739    my $attribute = shift;
     740
     741    my @specials = ();
     742    push(@specials, "DontDelete") if IsUnforgeable($interface, $attribute);
     743
     744    # As per Web IDL specification, constructor properties on the ECMAScript global object should not be enumerable.
     745    my $is_global_constructor = $attribute->signature->type =~ /Constructor$/;
     746    push(@specials, "DontEnum") if ($attribute->signature->extendedAttributes->{"NotEnumerable"} || $is_global_constructor);
     747    push(@specials, "ReadOnly") if IsReadonly($attribute);
     748    push(@specials, "CustomAccessor") unless $is_global_constructor or IsJSBuiltin($interface, $attribute);
     749    push(@specials, "Accessor | Builtin") if  IsJSBuiltin($interface, $attribute);
     750    return (@specials > 0) ? join(" | ", @specials) : "0";
    734751}
    735752
     
    12311248    if ($interfaceName eq "DOMWindow") {
    12321249        push(@headerContent, "    $className(JSC::VM&, JSC::Structure*, Ref<$implType>&&, JSDOMWindowShell*);\n");
     1250        push(@headerContent, "    void finishCreation(JSC::VM&, JSDOMWindowShell*);\n");
    12331251    } elsif ($codeGenerator->InheritsInterface($interface, "WorkerGlobalScope")) {
    12341252        push(@headerContent, "    $className(JSC::VM&, JSC::Structure*, Ref<$implType>&&);\n");
     
    13931411        next if ($attribute->isStatic);
    13941412        next if AttributeShouldBeOnInstance($interface, $attribute) != $isInstance;
     1413
     1414        # DOMWindow adds RuntimeEnabled attributes after creation so do not add them to the static table.
     1415        if ($interfaceName eq "DOMWindow" && $attribute->signature->extendedAttributes->{"EnabledAtRuntime"}) {
     1416            $propertyCount -= 1;
     1417            next;
     1418        }
     1419
    13951420        my $name = $attribute->signature->name;
    13961421        push(@$hashKeys, $name);
    13971422
    1398         my @specials = ();
    1399         push(@specials, "DontDelete") if IsUnforgeable($interface, $attribute);
    1400 
    1401         # As per Web IDL specification, constructor properties on the ECMAScript global object should not be enumerable.
    1402         my $is_global_constructor = $attribute->signature->type =~ /Constructor$/;
    1403         push(@specials, "DontEnum") if ($attribute->signature->extendedAttributes->{"NotEnumerable"} || $is_global_constructor);
    1404         push(@specials, "ReadOnly") if IsReadonly($attribute);
    1405         push(@specials, "CustomAccessor") unless $is_global_constructor or IsJSBuiltin($interface, $attribute);
    1406         push(@specials, "Accessor | Builtin") if  IsJSBuiltin($interface, $attribute);
    1407         my $special = (@specials > 0) ? join(" | ", @specials) : "0";
     1423        my $special = GetJSCAttributesForAttribute($interface, $attribute);
    14081424        push(@$hashSpecials, $special);
    14091425
     
    21752191        push(@implContent, "    : $parentClassName(vm, structure, WTFMove(impl), shell)\n");
    21762192        push(@implContent, "{\n");
     2193        push(@implContent, "}\n\n");
     2194
     2195        push(@implContent, "void ${className}::finishCreation(VM& vm, JSDOMWindowShell* shell)\n");
     2196        push(@implContent, "{\n");
     2197        push(@implContent, "    Base::finishCreation(vm, shell);\n\n");
     2198        # Support for RuntimeEnabled attributes on DOMWindow.
     2199        foreach my $attribute (@{$interface->attributes}) {
     2200            next unless $attribute->signature->extendedAttributes->{"EnabledAtRuntime"};
     2201
     2202            AddToImplIncludes("RuntimeEnabledFeatures.h");
     2203            my $conditionalString = $codeGenerator->GenerateConditionalString($attribute->signature);
     2204            push(@implContent, "#if ${conditionalString}\n") if $conditionalString;
     2205            my $enable_function = GetRuntimeEnableFunctionName($attribute->signature);
     2206            my $attributeName = $attribute->signature->name;
     2207            push(@implContent, "    if (${enable_function}()) {\n");
     2208            my $getter = GetAttributeGetterName($interfaceName, $className, $interface, $attribute);
     2209            my $setter = IsReadonly($attribute) ? "nullptr" : GetAttributeSetterName($interfaceName, $className, $interface, $attribute);
     2210            push(@implContent, "        auto* customGetterSetter = CustomGetterSetter::create(vm, $getter, $setter);\n");
     2211            my $jscAttributes = GetJSCAttributesForAttribute($interface, $attribute);
     2212            push(@implContent, "        putDirectCustomAccessor(vm, vm.propertyNames->$attributeName, customGetterSetter, attributesForStructure($jscAttributes));\n");
     2213            push(@implContent, "    }\n");
     2214            push(@implContent, "#endif\n") if $conditionalString;
     2215        }
    21772216        push(@implContent, "}\n\n");
    21782217    } elsif ($codeGenerator->InheritsInterface($interface, "WorkerGlobalScope")) {
     
    23592398            # Global constructors can be disabled at runtime.
    23602399            if ($attribute->signature->type =~ /Constructor$/) {
    2361                 if ($attribute->signature->extendedAttributes->{"EnabledAtRuntime"}) {
    2362                     AddToImplIncludes("RuntimeEnabledFeatures.h");
    2363                     my $enable_function = GetRuntimeEnableFunctionName($attribute->signature);
    2364                     push(@implContent, "    if (!${enable_function}())\n");
    2365                     push(@implContent, "        return JSValue::encode(jsUndefined());\n");
    2366                 } elsif ($attribute->signature->extendedAttributes->{"EnabledBySetting"}) {
     2400                if ($attribute->signature->extendedAttributes->{"EnabledBySetting"}) {
    23672401                    AddToImplIncludes("Frame.h");
    23682402                    AddToImplIncludes("Settings.h");
  • trunk/Source/WebCore/inspector/InspectorIndexedDBAgent.cpp

    r198762 r199017  
    498498    }
    499499
    500     IDBFactory* idbFactory = DOMWindowIndexedDatabase::indexedDB(domWindow);
     500    IDBFactory* idbFactory = DOMWindowIndexedDatabase::indexedDB(*domWindow);
    501501    if (!idbFactory)
    502502        errorString = ASCIILiteral("No IndexedDB factory for given frame found");
Note: See TracChangeset for help on using the changeset viewer.