[WebIDL] Remove JS builtin bindings for FetchHeaders
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 29 Jul 2017 02:35:56 +0000 (02:35 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 29 Jul 2017 02:35:56 +0000 (02:35 +0000)
https://bugs.webkit.org/show_bug.cgi?id=174905

Patch by Sam Weinig <sam@webkit.org> on 2017-07-28
Reviewed by Alex Christensen.

LayoutTests/imported/w3c:

* web-platform-tests/fetch/api/headers/headers-basic-expected.txt:
* web-platform-tests/fetch/api/headers/headers-record-expected.txt:
Update results for more passing tests.

Source/WebCore:

* CMakeLists.txt:
* DerivedSources.make:
* WebCore.xcodeproj/project.pbxproj:
* Modules/fetch/FetchHeaders.js: Removed.
Remove FetchHeaders.js

* Modules/fetch/FetchHeaders.cpp:
(WebCore::appendToHeaderMap):
(WebCore::FetchHeaders::create):
(WebCore::FetchHeaders::append):
* Modules/fetch/FetchHeaders.h:
(WebCore::FetchHeaders::FetchHeaders):
Add create function for generated constructor.
Add appendToHeaderMap static function which takes the functionality
from the existing append function, and makes it useable in create.

* Modules/fetch/FetchHeaders.idl:
Replace [JSBuiltinConstructor] with real constructor. Keep other builtin
attributes as they are still used by other Fetch code.

* bindings/js/JSDOMConvertRecord.h:
Fix record conversion to work with proxies by changing to use the method table
for getOwnPropertyNames, and undefined values by not excluding undefined values.

LayoutTests:

* fetch/header-constructor-is-array-expected.txt:
* fetch/header-constructor-is-array.html:
Update test to match spec. An array with out a prototype will not yield
a valid Header as it is not iterable.

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

15 files changed:
LayoutTests/ChangeLog
LayoutTests/fetch/header-constructor-is-array-expected.txt
LayoutTests/fetch/header-constructor-is-array.html
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/fetch/api/headers/headers-basic-expected.txt
LayoutTests/imported/w3c/web-platform-tests/fetch/api/headers/headers-record-expected.txt
Source/WebCore/CMakeLists.txt
Source/WebCore/ChangeLog
Source/WebCore/DerivedSources.make
Source/WebCore/Modules/fetch/FetchHeaders.cpp
Source/WebCore/Modules/fetch/FetchHeaders.h
Source/WebCore/Modules/fetch/FetchHeaders.idl
Source/WebCore/Modules/fetch/FetchHeaders.js [deleted file]
Source/WebCore/WebCore.xcodeproj/project.pbxproj
Source/WebCore/bindings/js/JSDOMConvertRecord.h

index dbf4750..7e61e95 100644 (file)
@@ -1,3 +1,15 @@
+2017-07-28  Sam Weinig  <sam@webkit.org>
+
+        [WebIDL] Remove JS builtin bindings for FetchHeaders
+        https://bugs.webkit.org/show_bug.cgi?id=174905
+
+        Reviewed by Alex Christensen.
+
+        * fetch/header-constructor-is-array-expected.txt:
+        * fetch/header-constructor-is-array.html:
+        Update test to match spec. An array with out a prototype will not yield
+        a valid Header as it is not iterable.
+
 2017-07-28  Matt Baker  <mattbaker@apple.com>
 
         Web Inspector: capture an async stack trace when web content calls addEventListener
index aa218e3..ed92465 100644 (file)
@@ -1,9 +1,9 @@
-This test should create a Headers with strange Array, but the constructor should recognize it as an Array.
+This test should create a Headers with strange Arrays.
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-PASS headers.get('hello') is "world"
+PASS headers.get('hello') is null
 PASS headers.get('hello') is "world"
 PASS successfullyParsed is true
 
index 46ab0bd..28a928d 100644 (file)
@@ -6,13 +6,13 @@
 <body>
 <iframe></iframe>
 <script>
-description("This test should create a Headers with strange Array, but the constructor should recognize it as an Array.");
+description("This test should create a Headers with strange Arrays.");
 
 {
     let array = [ [ 'hello', 'world' ] ];
     array.__proto__ = null;
     var headers = new Headers(array);
-    shouldBeEqualToString("headers.get('hello')", "world");
+    shouldBeNull("headers.get('hello')");
 }
 
 {
index 9fdeccb..1c2415a 100644 (file)
@@ -1,3 +1,14 @@
+2017-07-28  Sam Weinig  <sam@webkit.org>
+
+        [WebIDL] Remove JS builtin bindings for FetchHeaders
+        https://bugs.webkit.org/show_bug.cgi?id=174905
+
+        Reviewed by Alex Christensen.
+
+        * web-platform-tests/fetch/api/headers/headers-basic-expected.txt:
+        * web-platform-tests/fetch/api/headers/headers-record-expected.txt:
+        Update results for more passing tests.
+
 2017-07-26  Ali Juma  <ajuma@chromium.org>
 
         Implement document.elementsFromPoint
index 584a544..b0dc175 100644 (file)
@@ -7,7 +7,7 @@ PASS Create headers with 1 should throw
 PASS Create headers with sequence 
 PASS Create headers with record 
 PASS Create headers with existing headers 
-FAIL Create headers with existing headers with custom iterator assert_equals: expected (string) "test" but got (object) null
+PASS Create headers with existing headers with custom iterator 
 PASS Check append method 
 PASS Check set method 
 PASS Check has method 
index 05e8957..688df19 100644 (file)
@@ -2,14 +2,14 @@
 PASS Passing nothing to Headers constructor 
 PASS Passing undefined to Headers constructor 
 PASS Passing null to Headers constructor 
-FAIL Basic operation with one property assert_equals: expected 4 but got 3
-FAIL Basic operation with one property and a proto assert_equals: expected 4 but got 3
-FAIL Correct operation ordering with two properties assert_equals: expected 6 but got 4
-FAIL Correct operation ordering with two properties one of which has an invalid name assert_equals: expected 5 but got 4
-FAIL Correct operation ordering with two properties one of which has an invalid value assert_equals: expected 4 but got 3
-FAIL Correct operation ordering with non-enumerable properties assert_equals: expected 6 but got 5
-FAIL Correct operation ordering with undefined descriptors assert_equals: expected 6 but got 5
-FAIL Correct operation ordering with repeated keys assert_equals: expected 9 but got 6
+PASS Basic operation with one property 
+PASS Basic operation with one property and a proto 
+PASS Correct operation ordering with two properties 
+PASS Correct operation ordering with two properties one of which has an invalid name 
+PASS Correct operation ordering with two properties one of which has an invalid value 
+PASS Correct operation ordering with non-enumerable properties 
+PASS Correct operation ordering with undefined descriptors 
+FAIL Correct operation ordering with repeated keys assert_equals: expected "3" but got "1, 3"
 FAIL Basic operation with Symbol keys assert_throws: function "function () { var h = new Headers(proxy); }" did not throw
-FAIL Operation with non-enumerable Symbol keys assert_equals: expected 9 but got 6
+FAIL Operation with non-enumerable Symbol keys assert_equals: expected 9 but got 8
 
index 434362f..7410151 100644 (file)
@@ -3824,7 +3824,6 @@ add_dependencies(WebCoreTestSupportBindings WebCoreDerivedSources)
 
 set(WebCore_BUILTINS_SOURCES
     ${WEBCORE_DIR}/Modules/fetch/DOMWindowFetch.js
-    ${WEBCORE_DIR}/Modules/fetch/FetchHeaders.js
     ${WEBCORE_DIR}/Modules/fetch/FetchInternals.js
     ${WEBCORE_DIR}/Modules/fetch/FetchRequest.js
     ${WEBCORE_DIR}/Modules/fetch/FetchResponse.js
index 95ec4b7..e455413 100644 (file)
@@ -1,3 +1,34 @@
+2017-07-28  Sam Weinig  <sam@webkit.org>
+
+        [WebIDL] Remove JS builtin bindings for FetchHeaders
+        https://bugs.webkit.org/show_bug.cgi?id=174905
+
+        Reviewed by Alex Christensen.
+
+        * CMakeLists.txt:
+        * DerivedSources.make:
+        * WebCore.xcodeproj/project.pbxproj:
+        * Modules/fetch/FetchHeaders.js: Removed.
+        Remove FetchHeaders.js
+
+        * Modules/fetch/FetchHeaders.cpp:
+        (WebCore::appendToHeaderMap):
+        (WebCore::FetchHeaders::create):
+        (WebCore::FetchHeaders::append):
+        * Modules/fetch/FetchHeaders.h:
+        (WebCore::FetchHeaders::FetchHeaders):
+        Add create function for generated constructor.
+        Add appendToHeaderMap static function which takes the functionality
+        from the existing append function, and makes it useable in create.
+
+        * Modules/fetch/FetchHeaders.idl:
+        Replace [JSBuiltinConstructor] with real constructor. Keep other builtin
+        attributes as they are still used by other Fetch code.
+
+        * bindings/js/JSDOMConvertRecord.h:
+        Fix record conversion to work with proxies by changing to use the method table
+        for getOwnPropertyNames, and undefined values by not excluding undefined values. 
+
 2017-07-28  Matt Baker  <mattbaker@apple.com>
 
         Web Inspector: capture an async stack trace when web content calls addEventListener
index f055227..736a7fc 100644 (file)
@@ -1382,7 +1382,6 @@ CommandLineAPIModuleSource.h : CommandLineAPIModuleSource.js
 
 WebCore_BUILTINS_SOURCES = \
     ${WebCore}/Modules/fetch/DOMWindowFetch.js \
-    $(WebCore)/Modules/fetch/FetchHeaders.js \
     $(WebCore)/Modules/fetch/FetchInternals.js \
     $(WebCore)/Modules/fetch/FetchRequest.js \
     $(WebCore)/Modules/fetch/FetchResponse.js \
index 75e5712..46fa014 100644 (file)
@@ -50,18 +50,52 @@ static ExceptionOr<bool> canWriteHeader(const String& name, const String& value,
     return true;
 }
 
-ExceptionOr<void> FetchHeaders::append(const String& name, const String& value)
+static ExceptionOr<void> appendToHeaderMap(const String& name, const String& value, HTTPHeaderMap& headers, FetchHeaders::Guard guard)
 {
     String normalizedValue = stripLeadingAndTrailingHTTPSpaces(value);
-    auto canWriteResult = canWriteHeader(name, normalizedValue, m_guard);
+    auto canWriteResult = canWriteHeader(name, normalizedValue, guard);
     if (canWriteResult.hasException())
         return canWriteResult.releaseException();
     if (!canWriteResult.releaseReturnValue())
         return { };
-    m_headers.add(name, normalizedValue);
+    headers.add(name, normalizedValue);
     return { };
 }
 
+
+ExceptionOr<Ref<FetchHeaders>> FetchHeaders::create(std::optional<HeadersInit>&& headersInit)
+{
+    HTTPHeaderMap headers;
+
+    if (headersInit) {
+        if (WTF::holds_alternative<Vector<Vector<String>>>(*headersInit)) {
+            auto& sequence = WTF::get<Vector<Vector<String>>>(*headersInit);
+            for (auto& header : sequence) {
+                if (header.size() != 2)
+                    return Exception { TypeError, "Header sub-sequence must contain exactly two items" };
+                auto result = appendToHeaderMap(header[0], header[1], headers, Guard::None);
+                if (result.hasException())
+                    return result.releaseException();
+            }
+        } else {
+            auto& record = WTF::get<Vector<WTF::KeyValuePair<String, String>>>(*headersInit);
+            for (auto& header : record) {
+                auto result = appendToHeaderMap(header.key, header.value, headers, Guard::None);
+                if (result.hasException())
+                    return result.releaseException();
+            }
+        }
+    }
+
+    return adoptRef(*new FetchHeaders { Guard::None, WTFMove(headers) });
+}
+
+
+ExceptionOr<void> FetchHeaders::append(const String& name, const String& value)
+{
+    return appendToHeaderMap(name, value, m_headers, m_guard);
+}
+
 ExceptionOr<void> FetchHeaders::remove(const String& name)
 {
     auto canWriteResult = canWriteHeader(name, { }, m_guard);
index 85ab515..2e52cd5 100644 (file)
@@ -50,6 +50,9 @@ public:
 
     using Init = Variant<Vector<Vector<String>>, Vector<WTF::KeyValuePair<String, String>>>;
 
+    using HeadersInit = Variant<Vector<Vector<String>>, Vector<WTF::KeyValuePair<String, String>>>;
+    static ExceptionOr<Ref<FetchHeaders>> create(std::optional<HeadersInit>&&);
+
     static Ref<FetchHeaders> create(Guard guard = Guard::None) { return adoptRef(*new FetchHeaders { guard }); }
     static Ref<FetchHeaders> create(const FetchHeaders& headers) { return adoptRef(*new FetchHeaders { headers }); }
 
@@ -82,7 +85,12 @@ public:
     void setGuard(Guard);
 
 private:
-    FetchHeaders(Guard guard) : m_guard(guard) { }
+    FetchHeaders(Guard guard, HTTPHeaderMap&& headers = { })
+        : m_guard(guard)
+        , m_headers(WTFMove(headers))
+    {
+    }
+
     FetchHeaders(const FetchHeaders&);
 
     Guard m_guard;
@@ -90,8 +98,7 @@ private:
 };
 
 inline FetchHeaders::FetchHeaders(const FetchHeaders& other)
-    : RefCounted()
-    , m_guard(other.m_guard)
+    : m_guard(other.m_guard)
     , m_headers(other.m_headers)
 {
 }
index 4fbaf05..c875ca2 100644 (file)
@@ -34,7 +34,8 @@ typedef (sequence<sequence<ByteString>> or record<ByteString, ByteString>) Heade
     Exposed=(Window,Worker),
     ImplementationLacksVTable,
     InterfaceName=Headers,
-    JSBuiltinConstructor,
+    Constructor(optional HeadersInit init),
+    ConstructorMayThrowException,
     PrivateIdentifier,
     PublicIdentifier,
 ] interface FetchHeaders {
diff --git a/Source/WebCore/Modules/fetch/FetchHeaders.js b/Source/WebCore/Modules/fetch/FetchHeaders.js
deleted file mode 100644 (file)
index e8c2d75..0000000
+++ /dev/null
@@ -1,41 +0,0 @@
-/*
- * Copyright (C) 2016 Canon Inc.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in the
- *    documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY CANON INC. ``AS IS'' AND ANY
- * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
- * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL CANON INC. OR
- * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
- * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
- * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
- * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
- * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-// @conditional=ENABLE(FETCH_API)
-
-function initializeFetchHeaders(headersInit)
-{
-    "use strict";
-
-    if (headersInit === @undefined)
-        return this;
-
-    if (!@isObject(headersInit))
-        @throwTypeError("headersInit must be an object");
-
-    @fillFetchHeaders(this, headersInit);
-
-    return this;
-}
index fc8d7b8..0fce103 100644 (file)
                41F54F821C50C4F600338488 /* FetchHeaders.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = FetchHeaders.cpp; sourceTree = "<group>"; };
                41F54F831C50C4F600338488 /* FetchHeaders.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = FetchHeaders.h; sourceTree = "<group>"; };
                41F54F841C50C4F600338488 /* FetchHeaders.idl */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text; path = FetchHeaders.idl; sourceTree = "<group>"; };
-               41F54F851C50C4F600338488 /* FetchHeaders.js */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.javascript; path = FetchHeaders.js; sourceTree = "<group>"; };
                41F54F871C50C4F600338488 /* FetchRequest.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = FetchRequest.cpp; sourceTree = "<group>"; };
                41F54F881C50C4F600338488 /* FetchRequest.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = FetchRequest.h; sourceTree = "<group>"; };
                41F54F891C50C4F600338488 /* FetchRequest.idl */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text; path = FetchRequest.idl; sourceTree = "<group>"; };
                                41F54F821C50C4F600338488 /* FetchHeaders.cpp */,
                                41F54F831C50C4F600338488 /* FetchHeaders.h */,
                                41F54F841C50C4F600338488 /* FetchHeaders.idl */,
-                               41F54F851C50C4F600338488 /* FetchHeaders.js */,
                                41CF8BE61D46222C00707DC9 /* FetchInternals.js */,
                                4147E2B41C89912600A7E715 /* FetchLoader.cpp */,
                                4147E2B51C89912600A7E715 /* FetchLoader.h */,
index 4451d8b..d790524 100644 (file)
@@ -86,7 +86,8 @@ template<typename K, typename V> struct Converter<IDLRecord<K, V>> : DefaultConv
     
         // 4. Let keys be ? O.[[OwnPropertyKeys]]().
         JSC::PropertyNameArray keys(&vm, JSC::PropertyNameMode::Strings);
-        object->getOwnPropertyNames(object, &state, keys, JSC::EnumerationMode());
+        object->methodTable()->getOwnPropertyNames(object, &state, keys, JSC::EnumerationMode());
+
         RETURN_IF_EXCEPTION(scope, { });
 
         // 5. Repeat, for each element key of keys in List order:
@@ -96,15 +97,12 @@ template<typename K, typename V> struct Converter<IDLRecord<K, V>> : DefaultConv
             bool didGetDescriptor = object->getOwnPropertyDescriptor(&state, key, descriptor);
             RETURN_IF_EXCEPTION(scope, { });
 
-            if (!didGetDescriptor)
-                continue;
-
             // 2. If desc is not undefined and desc.[[Enumerable]] is true:
-            
+
             // FIXME: Do we need to check for enumerable / undefined, or is this handled by the default
             // enumeration mode?
 
-            if (!descriptor.value().isUndefined() && descriptor.enumerable()) {
+            if (didGetDescriptor && descriptor.enumerable()) {
                 // 1. Let typedKey be key converted to an IDL value of type K.
                 auto typedKey = Detail::IdentifierConverter<K>::convert(state, key);