Changeset 220601 in webkit


Ignore:
Timestamp:
Aug 11, 2017 9:56:37 AM (7 years ago)
Author:
weinig@apple.com
Message:

WTF::Function does not allow for reference / non-default constructible return types
https://bugs.webkit.org/show_bug.cgi?id=175244
Source/JavaScriptCore:

Reviewed by Chris Dumez.

  • runtime/ArrayBuffer.cpp:

(JSC::ArrayBufferContents::transferTo):
Call reset(), rather than clear() to avoid the call to destroy() in clear(). The
destroy call needed to be a no-op anyway, since the data is being moved.

Source/WebCore:

Reviewed by Chris Dumez.

  • bindings/js/JSCustomElementInterface.h:

(WebCore::JSCustomElementInterface::invokeCallback):
Update the default value for the addArguments parameter to be an empty lambda, rather than
default initialization, which leads to a null WTF::Function. This allows us to remove support
for calling null WTF::Function. No change in behavior.

Source/WebKit:

Reviewed by Chris Dumez.

  • UIProcess/WebResourceLoadStatisticsStore.h:

Update the default value for the updateCookiePartitioningForDomainsHandler parameter to be an
empty lambda, rather than default initialization, which leads to a null WTF::Function. This allows
us to remove support for calling null WTF::Function. No change in behavior.

Source/WTF:

Reviewed by Chris Dumez.

When Function, then NoncopyableFunction, was templatized to allow non-void return values
in r201493, it maintained the behavior of being callable even if the Function was null.
To accomplish this, when null, the default construction of the return parameter was used.
This means Function can't be used with return types that are not default constructible,
such as reference types and Ref.

This behavior of returning something when null is surprising, as this is not how normal
functions behave, and not very useful. Instead, we now assert that the function is not
null when being called.

  • wtf/Function.h:

(WTF::Function operator(...)):
Instead of allowing a null callable wrapper by returning the default construction of
the return type, assert that the wrapper is there when calling a Function.

Tools:

<rdar://problem/33801582>

Reviewed by Chris Dumez.

  • TestWebKitAPI/Tests/WTF/Function.cpp:

(TestWebKitAPI::TEST):

Location:
trunk
Files:
10 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/ChangeLog

    r220600 r220601  
     12017-08-10  Sam Weinig  <sam@webkit.org>
     2
     3        WTF::Function does not allow for reference / non-default constructible return types
     4        https://bugs.webkit.org/show_bug.cgi?id=175244
     5
     6        Reviewed by Chris Dumez.
     7
     8        * runtime/ArrayBuffer.cpp:
     9        (JSC::ArrayBufferContents::transferTo):
     10        Call reset(), rather than clear() to avoid the call to destroy() in clear(). The
     11        destroy call needed to be a no-op anyway, since the data is being moved.
     12
    1132017-08-11  Mark Lam  <mark.lam@apple.com>
    214
  • trunk/Source/JavaScriptCore/runtime/ArrayBuffer.cpp

    r220499 r220601  
    133133    other.m_destructor = WTFMove(m_destructor);
    134134    other.m_shared = m_shared;
    135     clear();
     135    reset();
    136136}
    137137
  • trunk/Source/WTF/ChangeLog

    r220579 r220601  
     12017-08-10  Sam Weinig  <sam@webkit.org>
     2
     3        WTF::Function does not allow for reference / non-default constructible return types
     4        https://bugs.webkit.org/show_bug.cgi?id=175244
     5
     6        Reviewed by Chris Dumez.
     7
     8        When Function, then NoncopyableFunction, was templatized to allow non-void return values
     9        in r201493, it maintained the behavior of being callable even if the Function was null.
     10        To accomplish this, when null, the default construction of the return parameter was used.
     11        This means Function can't be used with return types that are not default constructible,
     12        such as reference types and Ref.
     13
     14        This behavior of returning something when null is surprising, as this is not how normal
     15        functions behave, and not very useful. Instead, we now assert that the function is not
     16        null when being called.
     17
     18        * wtf/Function.h:
     19        (WTF::Function operator(...)):
     20        Instead of allowing a null callable wrapper by returning the default construction of
     21        the return type, assert that the wrapper is there when calling a Function.
     22
    1232017-08-10  Mark Lam  <mark.lam@apple.com>
    224
  • trunk/Source/WTF/wtf/Function.h

    r220499 r220601  
    5353    Out operator()(In... in) const
    5454    {
    55         if (m_callableWrapper)
    56             return m_callableWrapper->call(std::forward<In>(in)...);
    57         return Out();
     55        ASSERT(m_callableWrapper);
     56        return m_callableWrapper->call(std::forward<In>(in)...);
    5857    }
    5958
  • trunk/Source/WebCore/ChangeLog

    r220594 r220601  
     12017-08-10  Sam Weinig  <sam@webkit.org>
     2
     3        WTF::Function does not allow for reference / non-default constructible return types
     4        https://bugs.webkit.org/show_bug.cgi?id=175244
     5
     6        Reviewed by Chris Dumez.
     7
     8        * bindings/js/JSCustomElementInterface.h:
     9        (WebCore::JSCustomElementInterface::invokeCallback):
     10        Update the default value for the addArguments parameter to be an empty lambda, rather than
     11        default initialization, which leads to a null WTF::Function. This allows us to remove support
     12        for calling null WTF::Function. No change in behavior.
     13
    1142017-08-11  Antti Koivisto  <antti@apple.com>
    215
  • trunk/Source/WebCore/bindings/js/JSCustomElementInterface.h

    r220499 r220601  
    9595    RefPtr<Element> tryToConstructCustomElement(Document&, const AtomicString&);
    9696
    97     void invokeCallback(Element&, JSC::JSObject* callback, const WTF::Function<void(JSC::ExecState*, JSDOMGlobalObject*, JSC::MarkedArgumentBuffer&)>& addArguments = { });
     97    void invokeCallback(Element&, JSC::JSObject* callback, const WTF::Function<void(JSC::ExecState*, JSDOMGlobalObject*, JSC::MarkedArgumentBuffer&)>& addArguments = [](JSC::ExecState*, JSDOMGlobalObject*, JSC::MarkedArgumentBuffer&) { });
    9898
    9999    QualifiedName m_name;
  • trunk/Source/WebKit/ChangeLog

    r220585 r220601  
     12017-08-10  Sam Weinig  <sam@webkit.org>
     2
     3        WTF::Function does not allow for reference / non-default constructible return types
     4        https://bugs.webkit.org/show_bug.cgi?id=175244
     5
     6        Reviewed by Chris Dumez.
     7
     8        * UIProcess/WebResourceLoadStatisticsStore.h:
     9        Update the default value for the updateCookiePartitioningForDomainsHandler parameter to be an
     10        empty lambda, rather than default initialization, which leads to a null WTF::Function. This allows
     11        us to remove support for calling null WTF::Function. No change in behavior.
     12
    1132017-08-11  Adrian Perez de Castro  <aperez@igalia.com>
    214
  • trunk/Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.h

    r220571 r220601  
    6161public:
    6262    using UpdateCookiePartitioningForDomainsHandler = WTF::Function<void(const Vector<String>& domainsToRemove, const Vector<String>& domainsToAdd, ShouldClearFirst)>;
    63     static Ref<WebResourceLoadStatisticsStore> create(const String& resourceLoadStatisticsDirectory, Function<void (const String&)>&& testingCallback, UpdateCookiePartitioningForDomainsHandler&& updateCookiePartitioningForDomainsHandler = { })
     63    static Ref<WebResourceLoadStatisticsStore> create(const String& resourceLoadStatisticsDirectory, Function<void (const String&)>&& testingCallback, UpdateCookiePartitioningForDomainsHandler&& updateCookiePartitioningForDomainsHandler = [](const Vector<String>&, const Vector<String>&, ShouldClearFirst) { })
    6464    {
    6565        return adoptRef(*new WebResourceLoadStatisticsStore(resourceLoadStatisticsDirectory, WTFMove(testingCallback), WTFMove(updateCookiePartitioningForDomainsHandler)));
  • trunk/Tools/ChangeLog

    r220583 r220601  
     12017-08-11  Sam Weinig  <sam@webkit.org>
     2
     3        WTF::Function does not allow for reference / non-default constructible return types
     4        https://bugs.webkit.org/show_bug.cgi?id=175244
     5        <rdar://problem/33801582>
     6
     7        Reviewed by Chris Dumez.
     8
     9        * TestWebKitAPI/Tests/WTF/Function.cpp:
     10        (TestWebKitAPI::TEST):
     11
    1122017-08-11  Carlos Garcia Campos  <cgarcia@igalia.com>
    213
  • trunk/Tools/TestWebKitAPI/Tests/WTF/Function.cpp

    r220499 r220601  
    142142    Function<unsigned()> a;
    143143    EXPECT_FALSE(static_cast<bool>(a));
    144     EXPECT_EQ(0U, a());
    145144
    146145    a = [] {
     
    152151    a = nullptr;
    153152    EXPECT_FALSE(static_cast<bool>(a));
    154     EXPECT_EQ(0U, a());
    155153
    156154    a = MoveOnly { 2 };
     
    162160    EXPECT_EQ(2U, b());
    163161    EXPECT_FALSE(static_cast<bool>(a));
    164     EXPECT_EQ(0U, a());
    165162
    166163    a = MoveOnly { 3 };
     
    169166    EXPECT_EQ(3U, c());
    170167    EXPECT_FALSE(static_cast<bool>(a));
    171     EXPECT_EQ(0U, a());
    172168
    173169    b = WTFMove(c);
     
    175171    EXPECT_EQ(3U, b());
    176172    EXPECT_FALSE(static_cast<bool>(c));
    177     EXPECT_EQ(0U, c());
    178 
    179     Function<unsigned()> d = nullptr;
    180     EXPECT_FALSE(static_cast<bool>(d));
    181     EXPECT_EQ(0U, d());
    182173}
    183174
     
    223214
    224215    a = FunctionDestructionChecker(a);
    225     a = nullptr;
    226     EXPECT_TRUE(static_cast<bool>(FunctionDestructionChecker::functionAsBool));
    227     EXPECT_TRUE(static_cast<bool>(FunctionDestructionChecker::functionResult));
    228     EXPECT_FALSE(FunctionDestructionChecker::functionAsBool.value());
    229     EXPECT_EQ(0U, FunctionDestructionChecker::functionResult.value());
    230     FunctionDestructionChecker::functionAsBool = std::nullopt;
    231     FunctionDestructionChecker::functionResult = std::nullopt;
    232 
    233     a = FunctionDestructionChecker(a);
    234216    a = MoveOnly { 2 };
    235217    EXPECT_TRUE(static_cast<bool>(FunctionDestructionChecker::functionAsBool));
Note: See TracChangeset for help on using the changeset viewer.