WTF::Function does not allow for reference / non-default constructible return types
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 Aug 2017 15:59:01 +0000 (15:59 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 Aug 2017 15:59:01 +0000 (15:59 +0000)
https://bugs.webkit.org/show_bug.cgi?id=175244

Patch by Sam Weinig <sam@webkit.org> on 2017-08-09
Reviewed by Chris Dumez.

Source/JavaScriptCore:

* 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:

* 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:

* 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:

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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@220457 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/ArrayBuffer.cpp
Source/WTF/ChangeLog
Source/WTF/wtf/Function.h
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSCustomElementInterface.h
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.h

index a8a8e5b689d4dd4baa9b808cb3b74355aace1af1..d8f63173c69c7c27b9513020359bfd06a8460d45 100644 (file)
@@ -1,3 +1,15 @@
+2017-08-09  Sam Weinig  <sam@webkit.org>
+
+        WTF::Function does not allow for reference / non-default constructible return types
+        https://bugs.webkit.org/show_bug.cgi?id=175244
+
+        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.
+
 2017-08-09  Oleksandr Skachkov  <gskachkov@gmail.com>
 
         REGRESSION: 2 test262/test/language/statements/async-function failures
index ccf988e2a06061bde4e6bf4adeaae16fdee37465..8d82026a90eedd1404a94693f121fe0cea144fed 100644 (file)
@@ -132,7 +132,7 @@ void ArrayBufferContents::transferTo(ArrayBufferContents& other)
     other.m_sizeInBytes = m_sizeInBytes;
     other.m_destructor = WTFMove(m_destructor);
     other.m_shared = m_shared;
-    clear();
+    reset();
 }
 
 void ArrayBufferContents::copyTo(ArrayBufferContents& other)
index 99625405bf62309b1d5b33c2f82d927c0141880c..d7dc7af1019492cfebf292e485691e9af9e1217b 100644 (file)
@@ -1,3 +1,25 @@
+2017-08-09  Sam Weinig  <sam@webkit.org>
+
+        WTF::Function does not allow for reference / non-default constructible return types
+        https://bugs.webkit.org/show_bug.cgi?id=175244
+
+        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.
+
 2017-08-08  Filip Pizlo  <fpizlo@apple.com>
 
         Baseline JIT should do caging
index bd5b05c4a7e29b35289facbc57a8801d0b40e9d4..22789afd85bcecb7ecaea4fcf1477a00e2541dd2 100644 (file)
@@ -52,9 +52,8 @@ public:
 
     Out operator()(In... in) const
     {
-        if (m_callableWrapper)
-            return m_callableWrapper->call(std::forward<In>(in)...);
-        return Out();
+        ASSERT(m_callableWrapper);
+        return m_callableWrapper->call(std::forward<In>(in)...);
     }
 
     explicit operator bool() const { return !!m_callableWrapper; }
index 88fa46ebe7bb1d63672f32aa86eff085ad692378..2dd60d3604e38fae465e19855dfccee93ac0af2d 100644 (file)
@@ -1,3 +1,16 @@
+2017-08-09  Sam Weinig  <sam@webkit.org>
+
+        WTF::Function does not allow for reference / non-default constructible return types
+        https://bugs.webkit.org/show_bug.cgi?id=175244
+
+        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.
+
 2017-08-09  Andy Estes  <aestes@apple.com>
 
         [QuickLook] Use case-insensitive comparison of preview MIME types
index ec84590bf1e454d946889ff942dbb5b0abdfea65..f06f322fd95ced0ab118cca30be38fbd94dcce01 100644 (file)
@@ -94,7 +94,7 @@ private:
 
     RefPtr<Element> tryToConstructCustomElement(Document&, const AtomicString&);
 
-    void invokeCallback(Element&, JSC::JSObject* callback, const WTF::Function<void(JSC::ExecState*, JSDOMGlobalObject*, JSC::MarkedArgumentBuffer&)>& addArguments = { });
+    void invokeCallback(Element&, JSC::JSObject* callback, const WTF::Function<void(JSC::ExecState*, JSDOMGlobalObject*, JSC::MarkedArgumentBuffer&)>& addArguments = [](JSC::ExecState*, JSDOMGlobalObject*, JSC::MarkedArgumentBuffer&) { });
 
     QualifiedName m_name;
     JSC::Weak<JSC::JSObject> m_constructor;
index 6a7ac634d3057f39a15dc01d97e2e7471957dd25..b8336521809322fd60bb256a9ca604c8e791d323 100644 (file)
@@ -1,3 +1,15 @@
+2017-08-09  Sam Weinig  <sam@webkit.org>
+
+        WTF::Function does not allow for reference / non-default constructible return types
+        https://bugs.webkit.org/show_bug.cgi?id=175244
+
+        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.
+
 2017-08-08  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         Unreviewed, rolling out r220393.
index 521bd32cdc1c4058b03ef60cad4cc655f1c8fbfd..47826ed10eff9745c2da2b18e0df3e5e2cbd8356 100644 (file)
@@ -60,7 +60,7 @@ enum class ShouldClearFirst;
 class WebResourceLoadStatisticsStore final : public IPC::Connection::WorkQueueMessageReceiver {
 public:
     using UpdateCookiePartitioningForDomainsHandler = WTF::Function<void(const Vector<String>& domainsToRemove, const Vector<String>& domainsToAdd, ShouldClearFirst)>;
-    static Ref<WebResourceLoadStatisticsStore> create(const String& resourceLoadStatisticsDirectory, Function<void (const String&)>&& testingCallback, UpdateCookiePartitioningForDomainsHandler&& updateCookiePartitioningForDomainsHandler = { })
+    static Ref<WebResourceLoadStatisticsStore> create(const String& resourceLoadStatisticsDirectory, Function<void (const String&)>&& testingCallback, UpdateCookiePartitioningForDomainsHandler&& updateCookiePartitioningForDomainsHandler = [](const Vector<String>&, const Vector<String>&, ShouldClearFirst) { })
     {
         return adoptRef(*new WebResourceLoadStatisticsStore(resourceLoadStatisticsDirectory, WTFMove(testingCallback), WTFMove(updateCookiePartitioningForDomainsHandler)));
     }