Attachment 346903 Details for Bug 188469 – Patch

[patch] Patch

bug-188469-20180810101715.patch (text/plain), 15.38 KB, created by Ali Juma on 2018-08-10 07:17:16 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Ali Juma

Created: 2018-08-10 07:17:16 PDT

Size: 15.38 KB

patch

obsolete

>Subversion Revision: 234732
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 0ef337b84376a224203bf99faa7e21dedcbadcde..2ffe86fe41c97fc7436d9f2e52a282432c7555d6 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,28 @@
>+2018-08-10  Ali Juma  <ajuma@chromium.org>
>+
>+        [IntersectionObserver] Implement rootMargin parsing
>+        https://bugs.webkit.org/show_bug.cgi?id=188469
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Parse IntersectionObserver's rootMargin argument and throw an exception if it's invalid.
>+        Change the stored rootMargin from a String to a LengthBox so that future patches can
>+        use this value in intersection logic.
>+
>+        Tested by: imported/w3c/web-platform-tests/intersection-observer/observer-attributes.html
>+                   imported/w3c/web-platform-tests/intersection-observer/observer-exceptions.html
>+                   intersection-observer/intersection-observer-interface.html
>+
>+        * page/IntersectionObserver.cpp:
>+        (WebCore::parseRootMargin):
>+        (WebCore::IntersectionObserver::create):
>+        (WebCore::IntersectionObserver::IntersectionObserver):
>+        (WebCore::IntersectionObserver::rootMargin const):
>+        * page/IntersectionObserver.h:
>+        (WebCore::IntersectionObserver::create): Deleted.
>+        (WebCore::IntersectionObserver::rootMargin const): Deleted.
>+        * page/IntersectionObserver.idl:
>+
> 2018-08-09  Ali Juma  <ajuma@chromium.org>
> 
>         Update IDL for IntersectionObserverEntry and IntersectionObserverEntryInit
>diff --git a/Source/WebCore/page/IntersectionObserver.cpp b/Source/WebCore/page/IntersectionObserver.cpp
>index 07fa8a889c4845b29a1d7a823b2665c5ffae56c4..fa7a2cb422d213bcda46afea0396da933850d11a 100644
>--- a/Source/WebCore/page/IntersectionObserver.cpp
>+++ b/Source/WebCore/page/IntersectionObserver.cpp
>@@ -28,6 +28,9 @@
> #if ENABLE(INTERSECTION_OBSERVER)
> #include "IntersectionObserver.h"
> 
>+#include "CSSParserTokenRange.h"
>+#include "CSSPropertyParserHelpers.h"
>+#include "CSSTokenizer.h"
> #include "Element.h"
> #include "IntersectionObserverCallback.h"
> #include "IntersectionObserverEntry.h"
>@@ -35,9 +38,61 @@
> 
> namespace WebCore {
> 
>-IntersectionObserver::IntersectionObserver(Ref<IntersectionObserverCallback>&& callback, Init&& init)
>+static ExceptionOr<LengthBox> parseRootMargin(String& rootMargin)
>+{
>+    CSSTokenizer tokenizer(rootMargin);
>+    auto tokenRange = tokenizer.tokenRange();
>+    Vector<Length, 4> margins;
>+    while (!tokenRange.atEnd()) {
>+        if (margins.size() == 4)
>+            return Exception { SyntaxError, "Failed to construct 'IntersectionObserver': Extra text found at the end of rootMargin." };
>+        RefPtr<CSSPrimitiveValue> parsedValue = CSSPropertyParserHelpers::consumeLengthOrPercent(tokenRange, HTMLStandardMode, ValueRangeAll);
>+        if (!parsedValue || parsedValue->isCalculated())
>+            return Exception { SyntaxError, "Failed to construct 'IntersectionObserver': rootMargin must be specified in pixels or percent." };
>+        if (parsedValue->isPercentage())
>+            margins.append(Length(parsedValue->doubleValue(), Percent));
>+        else if (parsedValue->isPx())
>+            margins.append(Length(parsedValue->intValue(), Fixed));
>+        else
>+            return Exception { SyntaxError, "Failed to construct 'IntersectionObserver': rootMargin must be specified in pixels or percent." };
>+    }
>+    switch (margins.size()) {
>+    case 0:
>+        for (unsigned i = 0; i < 4; ++i)
>+            margins.append(Length());
>+        break;
>+    case 1:
>+        for (unsigned i = 0; i < 3; ++i)
>+            margins.append(margins[0]);
>+        break;
>+    case 2:
>+        margins.append(margins[0]);
>+        margins.append(margins[1]);
>+        break;
>+    case 3:
>+        margins.append(margins[1]);
>+        break;
>+    case 4:
>+        break;
>+    default:
>+        ASSERT_NOT_REACHED();
>+    }
>+
>+    return LengthBox(WTFMove(margins[0]), WTFMove(margins[1]), WTFMove(margins[2]), WTFMove(margins[3]));
>+}
>+
>+ExceptionOr<Ref<IntersectionObserver>> IntersectionObserver::create(Ref<IntersectionObserverCallback>&& callback, IntersectionObserver::Init&& init)
>+{
>+    auto rootMarginOrException = parseRootMargin(init.rootMargin);
>+    if (rootMarginOrException.hasException())
>+        return rootMarginOrException.releaseException();
>+
>+    return adoptRef(*new IntersectionObserver(WTFMove(callback), WTFMove(init), rootMarginOrException.releaseReturnValue()));
>+}
>+
>+IntersectionObserver::IntersectionObserver(Ref<IntersectionObserverCallback>&& callback, Init&& init, LengthBox&& parsedRootMargin)
>     : m_root(init.root)
>-    , m_rootMargin(WTFMove(init.rootMargin))
>+    , m_rootMargin(WTFMove(parsedRootMargin))
>     , m_callback(WTFMove(callback))
> {
>     if (WTF::holds_alternative<double>(init.threshold))
>@@ -46,6 +101,23 @@ IntersectionObserver::IntersectionObserver(Ref<IntersectionObserverCallback>&& c
>         m_thresholds = WTF::get<Vector<double>>(WTFMove(init.threshold));
> }
> 
>+String IntersectionObserver::rootMargin() const
>+{
>+    StringBuilder stringBuilder;
>+    PhysicalBoxSide sides[4] = { PhysicalBoxSide::Top, PhysicalBoxSide::Right, PhysicalBoxSide::Bottom, PhysicalBoxSide::Left };
>+    for (auto side : sides) {
>+        auto& length = m_rootMargin.at(side);
>+        stringBuilder.appendNumber(length.intValue());
>+        if (length.type() == Percent)
>+            stringBuilder.append('%');
>+        else
>+            stringBuilder.append("px", 2);
>+        if (side != PhysicalBoxSide::Left)
>+            stringBuilder.append(' ');
>+    }
>+    return stringBuilder.toString();
>+}
>+
> void IntersectionObserver::observe(Element&)
> {
> }
>diff --git a/Source/WebCore/page/IntersectionObserver.h b/Source/WebCore/page/IntersectionObserver.h
>index 13d58f3cfc55edecd52e98542844018cd2fb431a..1d3f01201aad081e7c52d81786db36b82f65ccaa 100644
>--- a/Source/WebCore/page/IntersectionObserver.h
>+++ b/Source/WebCore/page/IntersectionObserver.h
>@@ -29,6 +29,7 @@
> 
> #include "IntersectionObserverCallback.h"
> #include "IntersectionObserverEntry.h"
>+#include "LengthBox.h"
> #include <wtf/RefCounted.h>
> #include <wtf/Variant.h>
> #include <wtf/text/WTFString.h>
>@@ -45,13 +46,10 @@ public:
>         Variant<double, Vector<double>> threshold;
>     };
> 
>-    static Ref<IntersectionObserver> create(Ref<IntersectionObserverCallback>&& callback, Init&& init)
>-    {
>-        return adoptRef(*new IntersectionObserver(WTFMove(callback), WTFMove(init)));
>-    }
>+    static ExceptionOr<Ref<IntersectionObserver>> create(Ref<IntersectionObserverCallback>&&, Init&&);
>     
>     Element* root() const { return m_root.get(); }
>-    String rootMargin() const { return m_rootMargin; }
>+    String rootMargin() const;
>     const Vector<double>& thresholds() const { return m_thresholds; }
> 
>     void observe(Element&);
>@@ -61,10 +59,10 @@ public:
>     Vector<RefPtr<IntersectionObserverEntry>> takeRecords();
> 
> private:
>-    IntersectionObserver(Ref<IntersectionObserverCallback>&&, Init&&);
>+    IntersectionObserver(Ref<IntersectionObserverCallback>&&, Init&&, LengthBox&& parsedRootMargin);
>     
>     RefPtr<Element> m_root;
>-    String m_rootMargin;
>+    LengthBox m_rootMargin;
>     Vector<double> m_thresholds;
>     Ref<IntersectionObserverCallback> m_callback;
> };
>diff --git a/Source/WebCore/page/IntersectionObserver.idl b/Source/WebCore/page/IntersectionObserver.idl
>index 757de6ee1f02f4f1705e011658b64abfe4528e3b..e6dbabf6c0204e910978507a476e111e8de74b15 100644
>--- a/Source/WebCore/page/IntersectionObserver.idl
>+++ b/Source/WebCore/page/IntersectionObserver.idl
>@@ -27,6 +27,7 @@
> 
> [
>     Conditional=INTERSECTION_OBSERVER,
>+    ConstructorMayThrowException,
>     Constructor(IntersectionObserverCallback callback, optional IntersectionObserverInit options),
>     ImplementationLacksVTable,
>     EnabledAtRuntime=IntersectionObserver
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index d9a2077a4a4c29b6f7cebe0c8c019b26a92d43b7..3663a666f3ca923d68a34af7d4443539064773cb 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,13 @@
>+2018-08-10  Ali Juma  <ajuma@chromium.org>
>+
>+        [IntersectionObserver] Implement rootMargin parsing
>+        https://bugs.webkit.org/show_bug.cgi?id=188469
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * intersection-observer/intersection-observer-interface-expected.txt:
>+        * intersection-observer/intersection-observer-interface.html:
>+
> 2018-08-09  Ali Juma  <ajuma@chromium.org>
> 
>         Update IDL for IntersectionObserverEntry and IntersectionObserverEntryInit
>diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog
>index 06ee6d0641c13930235f38f8f616a3440a9c60a7..a76947883e0f206f023584adf1f10db76c6fa0b1 100644
>--- a/LayoutTests/imported/w3c/ChangeLog
>+++ b/LayoutTests/imported/w3c/ChangeLog
>@@ -1,3 +1,15 @@
>+2018-08-10  Ali Juma  <ajuma@chromium.org>
>+
>+        [IntersectionObserver] Implement rootMargin parsing
>+        https://bugs.webkit.org/show_bug.cgi?id=188469
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Update expectations for newly passing test cases.
>+
>+        * web-platform-tests/intersection-observer/observer-attributes-expected.txt:
>+        * web-platform-tests/intersection-observer/observer-exceptions-expected.txt:
>+
> 2018-08-09  Ali Juma  <ajuma@chromium.org>
> 
>         Import WPTs for IntersectionObserver
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/intersection-observer/observer-attributes-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/intersection-observer/observer-attributes-expected.txt
>index 2efd44b4eee0f77504a8cce8bd0b2479108bfd44..9461517f99f603fd8cec27c24ccd7fb322df9f93 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/intersection-observer/observer-attributes-expected.txt
>+++ b/LayoutTests/imported/w3c/web-platform-tests/intersection-observer/observer-attributes-expected.txt
>@@ -2,8 +2,8 @@
> PASS Observer attribute getters. 
> PASS observer.root 
> PASS observer.thresholds 
>-FAIL observer.rootMargin assert_equals: expected "0px 0px 0px 0px" but got "0px"
>+PASS observer.rootMargin 
> PASS set observer.root 
> PASS set observer.thresholds 
>-FAIL set observer.rootMargin assert_equals: expected "10% 20px 10% 20px" but got "10% 20px"
>+PASS set observer.rootMargin 
> 
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/intersection-observer/observer-exceptions-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/intersection-observer/observer-exceptions-expected.txt
>index be3d45748031fea3f889081f61aa1f1023bdf304..69880cc7471a6be0299eee54e3dda1b2ef2d784b 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/intersection-observer/observer-exceptions-expected.txt
>+++ b/LayoutTests/imported/w3c/web-platform-tests/intersection-observer/observer-exceptions-expected.txt
>@@ -3,23 +3,11 @@ FAIL IntersectionObserver constructor with { threshold: [1.1] } assert_throws: f
>     new IntersectionObserver(e => {}, {threshold: [1.1]})
>   }" did not throw
> PASS IntersectionObserver constructor with { threshold: ["foo"] } 
>-FAIL IntersectionObserver constructor witth { rootMargin: "1" } assert_throws: function "function () {
>-    new IntersectionObserver(e => {}, {rootMargin: "1"})
>-  }" did not throw
>-FAIL IntersectionObserver constructor with { rootMargin: "2em" } assert_throws: function "function () {
>-    new IntersectionObserver(e => {}, {rootMargin: "2em"})
>-  }" did not throw
>-FAIL IntersectionObserver constructor with { rootMargin: "auto" } assert_throws: function "function () {
>-    new IntersectionObserver(e => {}, {rootMargin: "auto"})
>-  }" did not throw
>-FAIL IntersectionObserver constructor with { rootMargin: "calc(1px + 2px)" } assert_throws: function "function () {
>-    new IntersectionObserver(e => {}, {rootMargin: "calc(1px + 2px)"})
>-  }" did not throw
>-FAIL IntersectionObserver constructor with { rootMargin: "1px !important" } assert_throws: function "function () {
>-    new IntersectionObserver(e => {}, {rootMargin: "1px !important"})
>-  }" did not throw
>-FAIL IntersectionObserver constructor with { rootMargin: "1px 1px 1px 1px 1px" } assert_throws: function "function () {
>-    new IntersectionObserver(e => {}, {rootMargin: "1px 1px 1px 1px 1px"})
>-  }" did not throw
>+PASS IntersectionObserver constructor witth { rootMargin: "1" } 
>+PASS IntersectionObserver constructor with { rootMargin: "2em" } 
>+PASS IntersectionObserver constructor with { rootMargin: "auto" } 
>+PASS IntersectionObserver constructor with { rootMargin: "calc(1px + 2px)" } 
>+PASS IntersectionObserver constructor with { rootMargin: "1px !important" } 
>+PASS IntersectionObserver constructor with { rootMargin: "1px 1px 1px 1px 1px" } 
> PASS IntersectionObserver.observe("foo") 
> 
>diff --git a/LayoutTests/intersection-observer/intersection-observer-interface-expected.txt b/LayoutTests/intersection-observer/intersection-observer-interface-expected.txt
>index b806a8ae02c1bb39bdf836628cc334b6fcd16353..16b615092954dd49d3b58709f62a5d6eee81bc78 100644
>--- a/LayoutTests/intersection-observer/intersection-observer-interface-expected.txt
>+++ b/LayoutTests/intersection-observer/intersection-observer-interface-expected.txt
>@@ -3,7 +3,10 @@ PASS Constructor0
> PASS DefaultRootMargin 
> PASS DefaultRoot 
> PASS DefaultThresholds 
>-PASS ExplicitRootMargin 
>+PASS ExplicitOneArgumentRootMargin 
>+PASS ExplicitTwoArgumentRootMargin 
>+PASS ExplicitThreeArgumentRootMargin 
>+PASS ExplicitFourArgumentRootMargin 
> PASS ExplicitRoot 
> PASS ExplicitThreshold 
> PASS ExplicitThresholds 
>diff --git a/LayoutTests/intersection-observer/intersection-observer-interface.html b/LayoutTests/intersection-observer/intersection-observer-interface.html
>index 4a299060655677c00976c0e78b517e0ab41b471b..39fa59e69e0d00a602c0727e323c8ef56d679740 100644
>--- a/LayoutTests/intersection-observer/intersection-observer-interface.html
>+++ b/LayoutTests/intersection-observer/intersection-observer-interface.html
>@@ -15,7 +15,7 @@
>     },'Constructor0');
>     test(function() {
>         var observer = new IntersectionObserver(function() {});
>-        assert_equals(observer.rootMargin, '0px');
>+        assert_equals(observer.rootMargin, '0px 0px 0px 0px');
>     },'DefaultRootMargin');
>     test(function() {
>         var observer = new IntersectionObserver(function() {});
>@@ -26,9 +26,21 @@
>         assert_array_equals(observer.thresholds, [0]);
>     },'DefaultThresholds');
>     test(function() {
>-        var observer = new IntersectionObserver(function() {}, { rootMargin: '33em 10px -120px 3pt' });
>-        assert_equals(observer.rootMargin, '33em 10px -120px 3pt');
>-    },'ExplicitRootMargin');
>+        var observer = new IntersectionObserver(function() {}, { rootMargin: '33%' });
>+        assert_equals(observer.rootMargin, '33% 33% 33% 33%');
>+    },'ExplicitOneArgumentRootMargin');
>+    test(function() {
>+        var observer = new IntersectionObserver(function() {}, { rootMargin: '33% 10px' });
>+        assert_equals(observer.rootMargin, '33% 10px 33% 10px');
>+    },'ExplicitTwoArgumentRootMargin');
>+    test(function() {
>+        var observer = new IntersectionObserver(function() {}, { rootMargin: '33% 10px -120px' });
>+        assert_equals(observer.rootMargin, '33% 10px -120px 10px');
>+    },'ExplicitThreeArgumentRootMargin');
>+    test(function() {
>+        var observer = new IntersectionObserver(function() {}, { rootMargin: '33% 10px -120px 3%' });
>+        assert_equals(observer.rootMargin, '33% 10px -120px 3%');
>+    },'ExplicitFourArgumentRootMargin');
>     test(function() {
>         var observer = new IntersectionObserver(function() {}, { root: document.body });
>         assert_equals(observer.root, document.body);

Actions: View | Formatted Diff | Diff

Attachments on bug 188469: 346903 | 347087