Changeset 266983 in webkit


Ignore:
Timestamp:
Sep 12, 2020 4:11:38 PM (4 years ago)
Author:
weinig@apple.com
Message:

[WebIDL] Remove need for [MayThrowException] on named deleters
​https://bugs.webkit.org/show_bug.cgi?id=216429

Reviewed by Darin Adler.

Deduce implementation potentially thowing by introspecting the return
type of the deleters implementation. This allows us to remove another
use of [MayThrowException].

  • bindings/js/JSDOMAbstractOperations.h:

(WebCore::performLegacyPlatformObjectDeleteOperation):
Add helper function to house shared implementation of the various
combinations of return types allowed.

  • bindings/scripts/CodeGeneratorJS.pm:

(GenerateDeletePropertyCommon):
Convert the commented expectation that unnamed deleters return bool
or ExceptionOr<bool> into a static assertion and use the new helper
function to simplify code generation.

  • storage/Storage.idl:

Remove now unneeded [MayThrowException].

  • bindings/scripts/test/JS/JSTestNamedDeleterNoIdentifier.cpp:
  • bindings/scripts/test/JS/JSTestNamedDeleterThrowingException.cpp:
  • bindings/scripts/test/JS/JSTestNamedDeleterWithIdentifier.cpp:
  • bindings/scripts/test/JS/JSTestNamedDeleterWithIndexedGetter.cpp:
  • bindings/scripts/test/TestNamedDeleterThrowingException.idl:

Update tests/expectations.

Location:
trunk/Source/WebCore
Files:
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r266982 r266983  
     12020-09-12  Sam Weinig  <weinig@apple.com>
     2
     3        [WebIDL] Remove need for [MayThrowException] on named deleters
     4        https://bugs.webkit.org/show_bug.cgi?id=216429
     5
     6        Reviewed by Darin Adler.
     7       
     8        Deduce implementation potentially thowing by introspecting the return
     9        type of the deleters implementation. This allows us to remove another
     10        use of [MayThrowException].
     11
     12        * bindings/js/JSDOMAbstractOperations.h:
     13        (WebCore::performLegacyPlatformObjectDeleteOperation):
     14        Add helper function to house shared implementation of the various
     15        combinations of return types allowed. 
     16       
     17        * bindings/scripts/CodeGeneratorJS.pm:
     18        (GenerateDeletePropertyCommon):
     19        Convert the commented expectation that unnamed deleters return bool
     20        or ExceptionOr<bool> into a static assertion and use the new helper
     21        function to simplify code generation.
     22
     23        * storage/Storage.idl:
     24        Remove now unneeded [MayThrowException].
     25
     26        * bindings/scripts/test/JS/JSTestNamedDeleterNoIdentifier.cpp:
     27        * bindings/scripts/test/JS/JSTestNamedDeleterThrowingException.cpp:
     28        * bindings/scripts/test/JS/JSTestNamedDeleterWithIdentifier.cpp:
     29        * bindings/scripts/test/JS/JSTestNamedDeleterWithIndexedGetter.cpp:
     30        * bindings/scripts/test/TestNamedDeleterThrowingException.idl:
     31        Update tests/expectations.
     32
    1332020-09-12  Myles C. Maxfield  <mmaxfield@apple.com>
    234

  • trunk/Source/WebCore/bindings/js/JSDOMAbstractOperations.h

    r266676 r266983  
    2727
    2828#include "JSDOMConvertStrings.h"
     29#include "JSDOMExceptionHandling.h"
    2930
    3031namespace WebCore {
    … …  
    118119}
    119120
     121// This implements steps 2.2 through 2.5 of https://heycam.github.io/webidl/#legacy-platform-object-delete.
     122template<typename Functor> bool performLegacyPlatformObjectDeleteOperation(JSC::JSGlobalObject& lexicalGlobalObject, Functor&& functor)
     123{
     124    using ReturnType = std::invoke_result_t<Functor>;
     125
     126    if constexpr (IsExceptionOr<ReturnType>::value) {
     127        auto result = functor();
     128        if (result.hasException()) {
     129            auto throwScope = DECLARE_THROW_SCOPE(JSC::getVM(&lexicalGlobalObject));
     130            propagateException(lexicalGlobalObject, throwScope, result.releaseException());
     131            return true;
     132        }
     133       
     134        if constexpr (std::is_same_v<ReturnType, ExceptionOr<bool>>)
     135            return result.releaseReturnValue();
     136        return true;
     137    }
     138
     139    if constexpr (std::is_same_v<ReturnType, bool>)
     140        return functor();
     141   
     142    functor();
     143    return true;
    120144}
     145
     146}

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

    r266954 r266983  
    12751275    my $functionCall = "impl." . $functionImplementationName . "(propertyNameToString(propertyName))";
    12761276
    1277     # NOTE: We expect the implementation function of named deleters without an identifier to
    1278     #       return either bool or ExceptionOr<bool>. the implementation function of named deleters
    1279     #       with an identifier have no restriction, but if the return value of the operation is
    1280     #       boolean, we return that value, otherwise it is ignored (as per section 4.2).
    1281 
    1282     if ($operation->extendedAttributes->{MayThrowException}) {
    1283         push(@$outputArray, "        auto result = ${functionCall};\n");
    1284         push(@$outputArray, "        if (result.hasException()) {\n");
    1285         push(@$outputArray, "            auto throwScope = DECLARE_THROW_SCOPE(JSC::getVM(lexicalGlobalObject));\n");
    1286         push(@$outputArray, "            propagateException(*lexicalGlobalObject, throwScope, result.releaseException());\n");
    1287         push(@$outputArray, "            return true;\n");
    1288         push(@$outputArray, "        }\n\n");
    1289 
    1290         if (!$operation->name || $operation->name && $operation->type->name eq "boolean") {
    1291             push(@$outputArray, "        return result.releaseReturnValue();\n");
    1292         } else {
    1293             push(@$outputArray, "        return true;\n");
    1294         }
    1295     } else {
    1296         if (!$operation->name || $operation->name && $operation->type->name eq "boolean") {
    1297             push(@$outputArray, "        return ${functionCall};\n");
    1298         } else {
    1299             push(@$outputArray, "        ${functionCall};\n");
    1300             push(@$outputArray, "        return true;\n");
    1301         }
    1302     }
    1303 
     1277    # NOTE: We require the implementation function of named deleters without an identifier to
     1278    #       return either bool or ExceptionOr<bool>.
     1279    if (!$operation->name) {
     1280        push(@$outputArray, "        using ReturnType = decltype($functionCall);\n");
     1281        push(@$outputArray, "        static_assert(std::is_same_v<ReturnType, ExceptionOr<bool>> || std::is_same_v<ReturnType, bool>, \"The implementation of named deleters without an identifer must return either bool or ExceptionOr<bool>.\");\n");
     1282    }
     1283
     1284    push(@$outputArray, "        return performLegacyPlatformObjectDeleteOperation(*lexicalGlobalObject, [&] { return $functionCall; });\n");
    13041285    push(@$outputArray, "    }\n");
    13051286}

  • trunk/Source/WebCore/bindings/scripts/test/JS/JSTestNamedDeleterNoIdentifier.cpp

    r266662 r266983  
    207207    auto& impl = thisObject.wrapped();
    208208    if (isVisibleNamedProperty<LegacyOverrideBuiltIns::No>(*lexicalGlobalObject, thisObject, propertyName)) {
    209         return impl.deleteNamedProperty(propertyNameToString(propertyName));
     209        using ReturnType = decltype(impl.deleteNamedProperty(propertyNameToString(propertyName)));
     210        static_assert(std::is_same_v<ReturnType, ExceptionOr<bool>> || std::is_same_v<ReturnType, bool>, "The implementation of named deleters without an identifer must return either bool or ExceptionOr<bool>.");
     211        return performLegacyPlatformObjectDeleteOperation(*lexicalGlobalObject, [&] { return impl.deleteNamedProperty(propertyNameToString(propertyName)); });
    210212    }
    211213    return JSObject::deleteProperty(cell, lexicalGlobalObject, propertyName, slot);
    … …  
    219221    auto propertyName = Identifier::from(vm, index);
    220222    if (isVisibleNamedProperty<LegacyOverrideBuiltIns::No>(*lexicalGlobalObject, thisObject, propertyName)) {
    221         return impl.deleteNamedProperty(propertyNameToString(propertyName));
     223        using ReturnType = decltype(impl.deleteNamedProperty(propertyNameToString(propertyName)));
     224        static_assert(std::is_same_v<ReturnType, ExceptionOr<bool>> || std::is_same_v<ReturnType, bool>, "The implementation of named deleters without an identifer must return either bool or ExceptionOr<bool>.");
     225        return performLegacyPlatformObjectDeleteOperation(*lexicalGlobalObject, [&] { return impl.deleteNamedProperty(propertyNameToString(propertyName)); });
    222226    }
    223227    return JSObject::deletePropertyByIndex(cell, lexicalGlobalObject, index);

  • trunk/Source/WebCore/bindings/scripts/test/JS/JSTestNamedDeleterThrowingException.cpp

    r266662 r266983  
    207207    auto& impl = thisObject.wrapped();
    208208    if (isVisibleNamedProperty<LegacyOverrideBuiltIns::No>(*lexicalGlobalObject, thisObject, propertyName)) {
    209         auto result = impl.deleteNamedProperty(propertyNameToString(propertyName));
    210         if (result.hasException()) {
    211             auto throwScope = DECLARE_THROW_SCOPE(JSC::getVM(lexicalGlobalObject));
    212             propagateException(*lexicalGlobalObject, throwScope, result.releaseException());
    213             return true;
    214         }
    215 
    216         return result.releaseReturnValue();
     209        using ReturnType = decltype(impl.deleteNamedProperty(propertyNameToString(propertyName)));
     210        static_assert(std::is_same_v<ReturnType, ExceptionOr<bool>> || std::is_same_v<ReturnType, bool>, "The implementation of named deleters without an identifer must return either bool or ExceptionOr<bool>.");
     211        return performLegacyPlatformObjectDeleteOperation(*lexicalGlobalObject, [&] { return impl.deleteNamedProperty(propertyNameToString(propertyName)); });
    217212    }
    218213    return JSObject::deleteProperty(cell, lexicalGlobalObject, propertyName, slot);
    … …  
    226221    auto propertyName = Identifier::from(vm, index);
    227222    if (isVisibleNamedProperty<LegacyOverrideBuiltIns::No>(*lexicalGlobalObject, thisObject, propertyName)) {
    228         auto result = impl.deleteNamedProperty(propertyNameToString(propertyName));
    229         if (result.hasException()) {
    230             auto throwScope = DECLARE_THROW_SCOPE(JSC::getVM(lexicalGlobalObject));
    231             propagateException(*lexicalGlobalObject, throwScope, result.releaseException());
    232             return true;
    233         }
    234 
    235         return result.releaseReturnValue();
     223        using ReturnType = decltype(impl.deleteNamedProperty(propertyNameToString(propertyName)));
     224        static_assert(std::is_same_v<ReturnType, ExceptionOr<bool>> || std::is_same_v<ReturnType, bool>, "The implementation of named deleters without an identifer must return either bool or ExceptionOr<bool>.");
     225        return performLegacyPlatformObjectDeleteOperation(*lexicalGlobalObject, [&] { return impl.deleteNamedProperty(propertyNameToString(propertyName)); });
    236226    }
    237227    return JSObject::deletePropertyByIndex(cell, lexicalGlobalObject, index);

  • trunk/Source/WebCore/bindings/scripts/test/JS/JSTestNamedDeleterWithIdentifier.cpp

    r266662 r266983  
    213213    auto& impl = thisObject.wrapped();
    214214    if (isVisibleNamedProperty<LegacyOverrideBuiltIns::No>(*lexicalGlobalObject, thisObject, propertyName)) {
    215         impl.namedDeleter(propertyNameToString(propertyName));
    216         return true;
     215        return performLegacyPlatformObjectDeleteOperation(*lexicalGlobalObject, [&] { return impl.namedDeleter(propertyNameToString(propertyName)); });
    217216    }
    218217    return JSObject::deleteProperty(cell, lexicalGlobalObject, propertyName, slot);
    … …  
    226225    auto propertyName = Identifier::from(vm, index);
    227226    if (isVisibleNamedProperty<LegacyOverrideBuiltIns::No>(*lexicalGlobalObject, thisObject, propertyName)) {
    228         impl.namedDeleter(propertyNameToString(propertyName));
    229         return true;
     227        return performLegacyPlatformObjectDeleteOperation(*lexicalGlobalObject, [&] { return impl.namedDeleter(propertyNameToString(propertyName)); });
    230228    }
    231229    return JSObject::deletePropertyByIndex(cell, lexicalGlobalObject, index);

  • trunk/Source/WebCore/bindings/scripts/test/JS/JSTestNamedDeleterWithIndexedGetter.cpp

    r266662 r266983  
    228228        return !impl.isSupportedPropertyIndex(index.value());
    229229    if (isVisibleNamedProperty<LegacyOverrideBuiltIns::No>(*lexicalGlobalObject, thisObject, propertyName)) {
    230         return impl.deleteNamedProperty(propertyNameToString(propertyName));
     230        using ReturnType = decltype(impl.deleteNamedProperty(propertyNameToString(propertyName)));
     231        static_assert(std::is_same_v<ReturnType, ExceptionOr<bool>> || std::is_same_v<ReturnType, bool>, "The implementation of named deleters without an identifer must return either bool or ExceptionOr<bool>.");
     232        return performLegacyPlatformObjectDeleteOperation(*lexicalGlobalObject, [&] { return impl.deleteNamedProperty(propertyNameToString(propertyName)); });
    231233    }
    232234    return JSObject::deleteProperty(cell, lexicalGlobalObject, propertyName, slot);

  • trunk/Source/WebCore/bindings/scripts/test/TestNamedDeleterThrowingException.idl

    r266311 r266983  
    2525
    2626interface TestNamedDeleterThrowingException {
    27     [MayThrowException] deleter undefined (DOMString name);
     27    deleter undefined (DOMString name);
    2828
    2929    // NOTE: Named deleters require the presence of a named getter as well.

  • trunk/Source/WebCore/storage/Storage.idl

    r266311 r266983  
    3232    getter DOMString? getItem(DOMString key);
    3333    [MayThrowException] setter undefined setItem(DOMString key, DOMString data);
    34     [MayThrowException] deleter undefined removeItem(DOMString key);
     34    deleter undefined removeItem(DOMString key);
    3535    [MayThrowException] undefined clear();
    3636};
Note: See TracChangeset for help on using the changeset viewer.