Title: [212405] trunk
Revision
212405
Author
wei...@apple.com
Date
2017-02-15 15:38:18 -0800 (Wed, 15 Feb 2017)

Log Message

[WebIDL] Remove custom conversion from FontFace code by using a Variant
https://bugs.webkit.org/show_bug.cgi?id=168384

Reviewed by Alex Christensen.

Source/WebCore:

Match the font face spec and use a union rather than any in the FontFace constructor.

Test: Added additional cases to fast/text/font-face-_javascript_.html.

* css/FontFace.cpp:
(WebCore::FontFace::create):
* css/FontFace.h:
* css/FontFace.idl:

LayoutTests:

* fast/text/font-face-_javascript_.html:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (212404 => 212405)


--- trunk/LayoutTests/ChangeLog	2017-02-15 23:37:41 UTC (rev 212404)
+++ trunk/LayoutTests/ChangeLog	2017-02-15 23:38:18 UTC (rev 212405)
@@ -1,3 +1,12 @@
+2017-02-15  Sam Weinig  <s...@webkit.org>
+
+        [WebIDL] Remove custom conversion from FontFace code by using a Variant
+        https://bugs.webkit.org/show_bug.cgi?id=168384
+
+        Reviewed by Alex Christensen.
+
+        * fast/text/font-face-_javascript_.html:
+
 2017-02-15  Jer Noble  <jer.no...@apple.com>
 
         Disabled Media Sources should render black/silence

Modified: trunk/LayoutTests/fast/text/font-face-_javascript_.html (212404 => 212405)


--- trunk/LayoutTests/fast/text/font-face-_javascript_.html	2017-02-15 23:37:41 UTC (rev 212404)
+++ trunk/LayoutTests/fast/text/font-face-_javascript_.html	2017-02-15 23:38:18 UTC (rev 212405)
@@ -55,7 +55,7 @@
 everything.featureSettings = "'smcp'";
 shouldBeEqualToString("everything.featureSettings", "'smcp' 1");
 
-/*shouldNotThrow("new FontFace('family_name', 'url(\\'asdf\\')', {})");
+shouldNotThrow("new FontFace('family_name', 'url(\\'asdf\\')', {})");
 var newArrayBuffer = new ArrayBuffer(100);
 shouldNotThrow("new FontFace('family_name', newArrayBuffer, {})");
 shouldNotThrow("new FontFace('family_name', new DataView(newArrayBuffer), {})");
@@ -62,7 +62,10 @@
 shouldNotThrow("new FontFace('family_name', new Uint8Array(newArrayBuffer), {})");
 shouldThrow("new FontFace('family_name', 5, {})");
 shouldThrow("new FontFace('family_name', {}, {})");
-shouldThrow("new FontFace('family_name', new Array(), {})");*/
+shouldThrow("new FontFace('family_name', new Array(), {})");
+shouldThrow("new FontFace('family_name', { toString: function() { return 5; } }, {})");
+shouldThrow("new FontFace('family_name', { toString: function() { throw \"Error\"; } }, {})");
+shouldBeEqualToString("new FontFace('family_name', { toString: function() { return \"url(\'asdf\')\"; } }, {}).family", "family_name");
 
 shouldBeEqualToString("new FontFace('family_name', 'url(\\'asdf\\')', {}).status", "unloaded");
 

Modified: trunk/Source/WebCore/ChangeLog (212404 => 212405)


--- trunk/Source/WebCore/ChangeLog	2017-02-15 23:37:41 UTC (rev 212404)
+++ trunk/Source/WebCore/ChangeLog	2017-02-15 23:38:18 UTC (rev 212405)
@@ -1,3 +1,19 @@
+2017-02-15  Sam Weinig  <s...@webkit.org>
+
+        [WebIDL] Remove custom conversion from FontFace code by using a Variant
+        https://bugs.webkit.org/show_bug.cgi?id=168384
+
+        Reviewed by Alex Christensen.
+
+        Match the font face spec and use a union rather than any in the FontFace constructor.
+
+        Test: Added additional cases to fast/text/font-face-_javascript_.html.
+
+        * css/FontFace.cpp:
+        (WebCore::FontFace::create):
+        * css/FontFace.h:
+        * css/FontFace.idl:
+
 2017-02-15  Jer Noble  <jer.no...@apple.com>
 
         Disabled Media Sources should render black/silence

Modified: trunk/Source/WebCore/css/FontFace.cpp (212404 => 212405)


--- trunk/Source/WebCore/css/FontFace.cpp	2017-02-15 23:37:41 UTC (rev 212404)
+++ trunk/Source/WebCore/css/FontFace.cpp	2017-02-15 23:38:18 UTC (rev 212405)
@@ -36,6 +36,9 @@
 #include "FontVariantBuilder.h"
 #include "JSFontFace.h"
 #include "StyleProperties.h"
+#include <runtime/ArrayBuffer.h>
+#include <runtime/ArrayBufferView.h>
+#include <runtime/JSCInlines.h>
 
 namespace WebCore {
 
@@ -46,9 +49,8 @@
     return false;
 }
 
-ExceptionOr<Ref<FontFace>> FontFace::create(JSC::ExecState& state, Document& document, const String& family, JSC::JSValue source, const Descriptors& descriptors)
+ExceptionOr<Ref<FontFace>> FontFace::create(Document& document, const String& family, Source&& source, const Descriptors& descriptors)
 {
-    JSC::VM& vm = state.vm();
     auto result = adoptRef(*new FontFace(document.fontSelector()));
 
     bool dataRequiresAsynchronousLoading = true;
@@ -57,18 +59,28 @@
     if (setFamilyResult.hasException())
         return setFamilyResult.releaseException();
 
-    if (source.isString()) {
-        auto value = FontFace::parseString(asString(source)->value(&state), CSSPropertySrc);
-        if (!is<CSSValueList>(value.get()))
-            return Exception { SYNTAX_ERR };
-        CSSFontFace::appendSources(result->backing(), downcast<CSSValueList>(*value), &document, false);
-    } else if (auto arrayBufferView = toUnsharedArrayBufferView(vm, source))
-        dataRequiresAsynchronousLoading = populateFontFaceWithArrayBuffer(result->backing(), arrayBufferView.releaseNonNull());
-    else if (auto arrayBuffer = toUnsharedArrayBuffer(vm, source)) {
-        auto arrayBufferView = JSC::Uint8Array::create(arrayBuffer, 0, arrayBuffer->byteLength());
-        dataRequiresAsynchronousLoading = populateFontFaceWithArrayBuffer(result->backing(), arrayBufferView.releaseNonNull());
-    }
+    auto sourceConversionResult = WTF::switchOn(source,
+        [&] (String& string) -> ExceptionOr<void> {
+            auto value = FontFace::parseString(string, CSSPropertySrc);
+            if (!is<CSSValueList>(value.get()))
+                return Exception { SYNTAX_ERR };
+            CSSFontFace::appendSources(result->backing(), downcast<CSSValueList>(*value), &document, false);
+            return { };
+        },
+        [&] (RefPtr<ArrayBufferView>& arrayBufferView) -> ExceptionOr<void> {
+            dataRequiresAsynchronousLoading = populateFontFaceWithArrayBuffer(result->backing(), arrayBufferView.releaseNonNull());
+            return { };
+        },
+        [&] (RefPtr<ArrayBuffer>& arrayBuffer) -> ExceptionOr<void> {
+            auto arrayBufferView = JSC::Uint8Array::create(arrayBuffer, 0, arrayBuffer->byteLength());
+            dataRequiresAsynchronousLoading = populateFontFaceWithArrayBuffer(result->backing(), arrayBufferView.releaseNonNull());
+            return { };
+        }
+    );
 
+    if (sourceConversionResult.hasException())
+        return sourceConversionResult.releaseException();
+
     // These ternaries match the default strings inside the FontFaceDescriptors dictionary inside FontFace.idl.
     auto setStyleResult = result->setStyle(descriptors.style.isEmpty() ? ASCIILiteral("normal") : descriptors.style);
     if (setStyleResult.hasException())

Modified: trunk/Source/WebCore/css/FontFace.h (212404 => 212405)


--- trunk/Source/WebCore/css/FontFace.h	2017-02-15 23:37:41 UTC (rev 212404)
+++ trunk/Source/WebCore/css/FontFace.h	2017-02-15 23:38:18 UTC (rev 212405)
@@ -28,8 +28,14 @@
 #include "CSSFontFace.h"
 #include "CSSPropertyNames.h"
 #include "JSDOMPromise.h"
+#include <wtf/Variant.h>
 #include <wtf/WeakPtr.h>
 
+namespace JSC {
+class ArrayBuffer;
+class ArrayBufferView;
+}
+
 namespace WebCore {
 
 class FontFace final : public RefCounted<FontFace>, private CSSFontFace::Client {
@@ -42,7 +48,9 @@
         String variant;
         String featureSettings;
     };
-    static ExceptionOr<Ref<FontFace>> create(JSC::ExecState&, Document&, const String& family, JSC::JSValue source, const Descriptors&);
+    
+    using Source = Variant<String, RefPtr<JSC::ArrayBuffer>, RefPtr<JSC::ArrayBufferView>>;
+    static ExceptionOr<Ref<FontFace>> create(Document&, const String& family, Source&&, const Descriptors&);
     static Ref<FontFace> create(CSSFontFace&);
     virtual ~FontFace();
 

Modified: trunk/Source/WebCore/css/FontFace.idl (212404 => 212405)


--- trunk/Source/WebCore/css/FontFace.idl	2017-02-15 23:37:41 UTC (rev 212404)
+++ trunk/Source/WebCore/css/FontFace.idl	2017-02-15 23:38:18 UTC (rev 212405)
@@ -23,6 +23,8 @@
  * THE POSSIBILITY OF SUCH DAMAGE.
  */
 
+typedef (ArrayBuffer or ArrayBufferView) BinaryData;
+
 enum FontFaceLoadStatus {
     "unloaded",
     "loading",
@@ -40,9 +42,9 @@
 };
 
 [
-    ConstructorCallWith=Document&ScriptState,
+    ConstructorCallWith=Document,
     ConstructorMayThrowException,
-    Constructor(DOMString family, any source, optional FontFaceDescriptors descriptors)
+    Constructor(DOMString family, (DOMString or BinaryData) source, optional FontFaceDescriptors descriptors)
 ] interface FontFace {
     [SetterMayThrowException] attribute DOMString family;
     [SetterMayThrowException] attribute DOMString style;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to