WTF::Function does not allow for reference / non-default constructible return types

https://bugs.webkit.org/show_bug.cgi?id=175244

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

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@220477 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index ca22a1aa96e94bc5d189fd0afe35c22b66245044..900bbf4397059c499f4585450657247b817c74db 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -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 Sam Weinig.
+
+        * 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  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         [iOS DnD] ENABLE_DRAG_SUPPORT should be turned off for iOS 10 and enabled by default

diff --git a/Source/JavaScriptCore/runtime/ArrayBuffer.cpp b/Source/JavaScriptCore/runtime/ArrayBuffer.cpp
index ccf988e2a06061bde4e6bf4adeaae16fdee37465..8d82026a90eedd1404a94693f121fe0cea144fed 100644 (file)
--- a/Source/JavaScriptCore/runtime/ArrayBuffer.cpp
+++ b/Source/JavaScriptCore/runtime/ArrayBuffer.cpp
@@ -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)

diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
index fb5f19fb7271bbf0c4c9e62a85d4437c2d6f4f11..afe4bf2da91a73e13ac92b9190b3f33c7703090e 100644 (file)
--- a/Source/WTF/ChangeLog
+++ b/Source/WTF/ChangeLog
@@ -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 Sam Weinig.
+
+        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-09  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r220457.

diff --git a/Source/WTF/wtf/Function.h b/Source/WTF/wtf/Function.h
index bd5b05c4a7e29b35289facbc57a8801d0b40e9d4..22789afd85bcecb7ecaea4fcf1477a00e2541dd2 100644 (file)
--- a/Source/WTF/wtf/Function.h
+++ b/Source/WTF/wtf/Function.h
@@ -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; }

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 09c555ecd27b8e3340ef22e20d76fadc2461b7ca..b6f033fdcea9bfb28187bfdf922fa4be0719a652 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -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 Sam Weinig.
+
+        * 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  Brady Eidson  <beidson@apple.com>
 
         Teach ScriptExecutionContexts about their SessionID.

diff --git a/Source/WebCore/bindings/js/JSCustomElementInterface.h b/Source/WebCore/bindings/js/JSCustomElementInterface.h
index ec84590bf1e454d946889ff942dbb5b0abdfea65..f06f322fd95ced0ab118cca30be38fbd94dcce01 100644 (file)
--- a/Source/WebCore/bindings/js/JSCustomElementInterface.h
+++ b/Source/WebCore/bindings/js/JSCustomElementInterface.h
@@ -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;

diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
index e5b4ad0b1c145bb99af318e2ef43873caa76fbe9..c3f1116c22f9c139c4f9ad96f652d1964bbd84e3 100644 (file)
--- a/Source/WebKit/ChangeLog
+++ b/Source/WebKit/ChangeLog
@@ -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 Sam Weinig.
+
+        * 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-09  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         [iOS DnD] ENABLE_DRAG_SUPPORT should be turned off for iOS 10 and enabled by default

diff --git a/Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.h b/Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.h
index 521bd32cdc1c4058b03ef60cad4cc655f1c8fbfd..47826ed10eff9745c2da2b18e0df3e5e2cbd8356 100644 (file)
--- a/Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.h
+++ b/Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.h
@@ -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)));
     }