First parameter to Window.getComputedStyle() should be mandatory and non-nullable
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 Jul 2016 22:56:15 +0000 (22:56 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 Jul 2016 22:56:15 +0000 (22:56 +0000)
https://bugs.webkit.org/show_bug.cgi?id=160097

Reviewed by Ryosuke Niwa.

Source/WebCore:

First parameter to Window.getComputedStyle() should be mandatory and
non-nullable:
- https://drafts.csswg.org/cssom/#extensions-to-the-window-interface

Firefox and Chrome agree with the specification.

Test: fast/dom/Window/getComputedStyle-missing-parameter.html

* css/CSSComputedStyleDeclaration.cpp:
(WebCore::ComputedStyleExtractor::ComputedStyleExtractor):
(WebCore::CSSComputedStyleDeclaration::CSSComputedStyleDeclaration):
(WebCore::CSSComputedStyleDeclaration::getPropertyCSSValue):
(WebCore::CSSComputedStyleDeclaration::copyProperties):
(WebCore::CSSComputedStyleDeclaration::length):
(WebCore::CSSComputedStyleDeclaration::item):
(WebCore::CSSComputedStyleDeclaration::getPropertyValue):
* css/CSSComputedStyleDeclaration.h:
* dom/Document.idl:
* inspector/InspectorCSSAgent.cpp:
(WebCore::InspectorCSSAgent::getComputedStyleForNode):
* page/DOMWindow.cpp:
(WebCore::DOMWindow::getComputedStyle):
* page/DOMWindow.h:
* page/DOMWindow.idl:
* testing/Internals.cpp:
(WebCore::Internals::computedStyleIncludingVisitedInfo):
* testing/Internals.h:
* testing/Internals.idl:

LayoutTests:

Add test coverage.

* fast/dom/Window/getComputedStyle-missing-parameter-expected.txt: Added.
* fast/dom/Window/getComputedStyle-missing-parameter.html: Added.

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

14 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/dom/Window/getComputedStyle-missing-parameter-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/Window/getComputedStyle-missing-parameter.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/css/CSSComputedStyleDeclaration.cpp
Source/WebCore/css/CSSComputedStyleDeclaration.h
Source/WebCore/dom/Document.idl
Source/WebCore/inspector/InspectorCSSAgent.cpp
Source/WebCore/page/DOMWindow.cpp
Source/WebCore/page/DOMWindow.h
Source/WebCore/page/DOMWindow.idl
Source/WebCore/testing/Internals.cpp
Source/WebCore/testing/Internals.h
Source/WebCore/testing/Internals.idl

index 853e546..0edb15c 100644 (file)
@@ -1,3 +1,15 @@
+2016-07-22  Chris Dumez  <cdumez@apple.com>
+
+        First parameter to Window.getComputedStyle() should be mandatory and non-nullable
+        https://bugs.webkit.org/show_bug.cgi?id=160097
+
+        Reviewed by Ryosuke Niwa.
+
+        Add test coverage.
+
+        * fast/dom/Window/getComputedStyle-missing-parameter-expected.txt: Added.
+        * fast/dom/Window/getComputedStyle-missing-parameter.html: Added.
+
 2016-07-22  Ryan Haddad  <ryanhaddad@apple.com>
 
         Marking webaudio/audionode-connect-order.html as a flaky crash on mac-wk1 debug
diff --git a/LayoutTests/fast/dom/Window/getComputedStyle-missing-parameter-expected.txt b/LayoutTests/fast/dom/Window/getComputedStyle-missing-parameter-expected.txt
new file mode 100644 (file)
index 0000000..724c016
--- /dev/null
@@ -0,0 +1,11 @@
+Test that the first parameter to Window.getComputedStyle() is mandatory and not nullable.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS window.getComputedStyle() threw exception TypeError: Not enough arguments.
+PASS window.getComputedStyle(null) threw exception TypeError: Argument 1 ('element') to DOMWindow.getComputedStyle must be an instance of Element.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/dom/Window/getComputedStyle-missing-parameter.html b/LayoutTests/fast/dom/Window/getComputedStyle-missing-parameter.html
new file mode 100644 (file)
index 0000000..0c49db4
--- /dev/null
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../../resources/js-test-pre.js"></script>
+<script>
+description("Test that the first parameter to Window.getComputedStyle() is mandatory and not nullable.");
+
+shouldThrow("window.getComputedStyle()", "'TypeError: Not enough arguments'");
+shouldThrow("window.getComputedStyle(null)", "'TypeError: Argument 1 (\\'element\\') to DOMWindow.getComputedStyle must be an instance of Element'");
+</script>
+<script src="../../../resources/js-test-post.js"></script>
+</body>
+</html>
index 53a9f65..70e2564 100644 (file)
@@ -1,3 +1,39 @@
+2016-07-22  Chris Dumez  <cdumez@apple.com>
+
+        First parameter to Window.getComputedStyle() should be mandatory and non-nullable
+        https://bugs.webkit.org/show_bug.cgi?id=160097
+
+        Reviewed by Ryosuke Niwa.
+
+        First parameter to Window.getComputedStyle() should be mandatory and
+        non-nullable:
+        - https://drafts.csswg.org/cssom/#extensions-to-the-window-interface
+
+        Firefox and Chrome agree with the specification.
+
+        Test: fast/dom/Window/getComputedStyle-missing-parameter.html
+
+        * css/CSSComputedStyleDeclaration.cpp:
+        (WebCore::ComputedStyleExtractor::ComputedStyleExtractor):
+        (WebCore::CSSComputedStyleDeclaration::CSSComputedStyleDeclaration):
+        (WebCore::CSSComputedStyleDeclaration::getPropertyCSSValue):
+        (WebCore::CSSComputedStyleDeclaration::copyProperties):
+        (WebCore::CSSComputedStyleDeclaration::length):
+        (WebCore::CSSComputedStyleDeclaration::item):
+        (WebCore::CSSComputedStyleDeclaration::getPropertyValue):
+        * css/CSSComputedStyleDeclaration.h:
+        * dom/Document.idl:
+        * inspector/InspectorCSSAgent.cpp:
+        (WebCore::InspectorCSSAgent::getComputedStyleForNode):
+        * page/DOMWindow.cpp:
+        (WebCore::DOMWindow::getComputedStyle):
+        * page/DOMWindow.h:
+        * page/DOMWindow.idl:
+        * testing/Internals.cpp:
+        (WebCore::Internals::computedStyleIncludingVisitedInfo):
+        * testing/Internals.h:
+        * testing/Internals.idl:
+
 2016-07-22  Brady Eidson  <beidson@apple.com>
 
         Removing IndexedDatabases that have stored blobs doesn't remove the blob files.
index 036cb46..b6c5d60 100644 (file)
@@ -1581,16 +1581,16 @@ static Ref<CSSValue> createLineBoxContainValue(unsigned lineBoxContain)
     return CSSLineBoxContainValue::create(lineBoxContain);
 }
 
-ComputedStyleExtractor::ComputedStyleExtractor(PassRefPtr<Node> node, bool allowVisitedStyle, PseudoId pseudoElementSpecifier)
-    : m_node(node)
+ComputedStyleExtractor::ComputedStyleExtractor(RefPtr<Node>&& node, bool allowVisitedStyle, PseudoId pseudoElementSpecifier)
+    : m_node(WTFMove(node))
     , m_pseudoElementSpecifier(pseudoElementSpecifier)
     , m_allowVisitedStyle(allowVisitedStyle)
 {
 }
 
 
-CSSComputedStyleDeclaration::CSSComputedStyleDeclaration(PassRefPtr<Node> n, bool allowVisitedStyle, const String& pseudoElementName)
-    : m_node(n)
+CSSComputedStyleDeclaration::CSSComputedStyleDeclaration(Element& element, bool allowVisitedStyle, const String& pseudoElementName)
+    : m_element(element)
     , m_allowVisitedStyle(allowVisitedStyle)
     , m_refCount(1)
 {
@@ -2322,12 +2322,12 @@ static StyleSelfAlignmentData resolveAlignSelfAuto(const StyleSelfAlignmentData&
 
 RefPtr<CSSValue> CSSComputedStyleDeclaration::getPropertyCSSValue(CSSPropertyID propertyID, EUpdateLayout updateLayout) const
 {
-    return ComputedStyleExtractor(m_node, m_allowVisitedStyle, m_pseudoElementSpecifier).propertyValue(propertyID, updateLayout);
+    return ComputedStyleExtractor(m_element.copyRef(), m_allowVisitedStyle, m_pseudoElementSpecifier).propertyValue(propertyID, updateLayout);
 }
 
 Ref<MutableStyleProperties> CSSComputedStyleDeclaration::copyProperties() const
 {
-    return ComputedStyleExtractor(m_node, m_allowVisitedStyle, m_pseudoElementSpecifier).copyProperties();
+    return ComputedStyleExtractor(m_element.copyRef(), m_allowVisitedStyle, m_pseudoElementSpecifier).copyProperties();
 }
 
 static inline bool nodeOrItsAncestorNeedsStyleRecalc(const Node& node)
@@ -3944,13 +3944,10 @@ String CSSComputedStyleDeclaration::getPropertyValue(CSSPropertyID propertyID) c
 
 unsigned CSSComputedStyleDeclaration::length() const
 {
-    Node* node = m_node.get();
-    if (!node)
-        return 0;
-
-    updateStyleIfNeededForNode(*node);
+    auto& element = m_element.get();
+    updateStyleIfNeededForNode(element);
 
-    auto* style = node->computedStyle(m_pseudoElementSpecifier);
+    auto* style = const_cast<Element&>(element).computedStyle(m_pseudoElementSpecifier);
     if (!style)
         return 0;
 
@@ -3965,11 +3962,8 @@ String CSSComputedStyleDeclaration::item(unsigned i) const
     if (i < numComputedProperties)
         return getPropertyNameString(computedProperties[i]);
     
-    Node* node = m_node.get();
-    if (!node)
-        return String();
-
-    auto* style = node->computedStyle(m_pseudoElementSpecifier);
+    auto& element = m_element.get();
+    auto* style = const_cast<Element&>(element).computedStyle(m_pseudoElementSpecifier);
     if (!style)
         return String();
     
@@ -4073,7 +4067,7 @@ CSSRule* CSSComputedStyleDeclaration::parentRule() const
 RefPtr<CSSValue> CSSComputedStyleDeclaration::getPropertyCSSValue(const String& propertyName)
 {
     if (isCustomPropertyName(propertyName))
-        return ComputedStyleExtractor(m_node, m_allowVisitedStyle, m_pseudoElementSpecifier).customPropertyValue(propertyName);
+        return ComputedStyleExtractor(m_element.copyRef(), m_allowVisitedStyle, m_pseudoElementSpecifier).customPropertyValue(propertyName);
 
     CSSPropertyID propertyID = cssPropertyID(propertyName);
     if (!propertyID)
@@ -4085,7 +4079,7 @@ RefPtr<CSSValue> CSSComputedStyleDeclaration::getPropertyCSSValue(const String&
 String CSSComputedStyleDeclaration::getPropertyValue(const String &propertyName)
 {
     if (isCustomPropertyName(propertyName))
-        return ComputedStyleExtractor(m_node, m_allowVisitedStyle, m_pseudoElementSpecifier).customPropertyText(propertyName);
+        return ComputedStyleExtractor(m_element.copyRef(), m_allowVisitedStyle, m_pseudoElementSpecifier).customPropertyText(propertyName);
 
     CSSPropertyID propertyID = cssPropertyID(propertyName);
     if (!propertyID)
index 03b12b5..7bb156c 100644 (file)
@@ -31,6 +31,7 @@ namespace WebCore {
 class CSSPrimitiveValue;
 class CSSValueList;
 class Color;
+class Element;
 class FilterOperations;
 class MutableStyleProperties;
 class Node;
@@ -47,7 +48,7 @@ enum AdjustPixelValuesForComputedStyle { AdjustPixelValues, DoNotAdjustPixelValu
 
 class ComputedStyleExtractor {
 public:
-    ComputedStyleExtractor(PassRefPtr<Node>, bool allowVisitedStyle = false, PseudoId = NOPSEUDO);
+    ComputedStyleExtractor(RefPtr<Node>&&, bool allowVisitedStyle = false, PseudoId = NOPSEUDO);
 
     RefPtr<CSSValue> propertyValue(CSSPropertyID, EUpdateLayout = UpdateLayout) const;
     String customPropertyText(const String& propertyName) const;
@@ -87,9 +88,9 @@ private:
 
 class CSSComputedStyleDeclaration final : public CSSStyleDeclaration {
 public:
-    static Ref<CSSComputedStyleDeclaration> create(PassRefPtr<Node> node, bool allowVisitedStyle = false, const String& pseudoElementName = String())
+    static Ref<CSSComputedStyleDeclaration> create(Element& element, bool allowVisitedStyle = false, const String& pseudoElementName = String())
     {
-        return adoptRef(*new CSSComputedStyleDeclaration(node, allowVisitedStyle, pseudoElementName));
+        return adoptRef(*new CSSComputedStyleDeclaration(element, allowVisitedStyle, pseudoElementName));
     }
     virtual ~CSSComputedStyleDeclaration();
 
@@ -99,7 +100,7 @@ public:
     String getPropertyValue(CSSPropertyID) const;
 
 private:
-    WEBCORE_EXPORT CSSComputedStyleDeclaration(PassRefPtr<Node>, bool allowVisitedStyle, const String&);
+    WEBCORE_EXPORT CSSComputedStyleDeclaration(Element&, bool allowVisitedStyle, const String&);
 
     // CSSOM functions. Don't make these public.
     CSSRule* parentRule() const override;
@@ -121,7 +122,7 @@ private:
 
     RefPtr<CSSValue> getPropertyCSSValue(CSSPropertyID, EUpdateLayout = UpdateLayout) const;
 
-    RefPtr<Node> m_node;
+    Ref<Element> m_element;
     PseudoId m_pseudoElementSpecifier;
     bool m_allowVisitedStyle;
     unsigned m_refCount;
index c0a6d45..77a899b 100644 (file)
 
 #if defined(LANGUAGE_OBJECTIVE_C) && LANGUAGE_OBJECTIVE_C
     // DOM Level 2 Style Interface
-    [ObjCLegacyUnnamedParameters, ObjCUseDefaultView] CSSStyleDeclaration getComputedStyle(Element? element,
-                                                                  DOMString pseudoElement);
+    [ObjCLegacyUnnamedParameters, ObjCUseDefaultView] CSSStyleDeclaration getComputedStyle(Element element, DOMString pseudoElement);
 
     // WebKit extension
     // FIXME: remove the first version once optional is implemented for Objective-C.
index 021dcbd..ff576cd 100644 (file)
@@ -643,7 +643,7 @@ void InspectorCSSAgent::getComputedStyleForNode(ErrorString& errorString, int no
     if (!element)
         return;
 
-    RefPtr<CSSComputedStyleDeclaration> computedStyleInfo = CSSComputedStyleDeclaration::create(element, true);
+    RefPtr<CSSComputedStyleDeclaration> computedStyleInfo = CSSComputedStyleDeclaration::create(*element, true);
     Ref<InspectorStyle> inspectorStyle = InspectorStyle::create(InspectorCSSId(), computedStyleInfo, nullptr);
     style = inspectorStyle->buildArrayForComputedStyle();
 }
index d2b0560..f42384d 100644 (file)
@@ -1396,11 +1396,8 @@ RefPtr<StyleMedia> DOMWindow::styleMedia() const
     return m_media;
 }
 
-RefPtr<CSSStyleDeclaration> DOMWindow::getComputedStyle(Element* element, const String& pseudoElt) const
+RefPtr<CSSStyleDeclaration> DOMWindow::getComputedStyle(Element& element, const String& pseudoElt) const
 {
-    if (!element)
-        return nullptr;
-
     return CSSComputedStyleDeclaration::create(element, false, pseudoElt);
 }
 
index 87a1118..f8889e7 100644 (file)
@@ -228,7 +228,7 @@ namespace WebCore {
 
         // DOM Level 2 Style Interface
 
-        RefPtr<CSSStyleDeclaration> getComputedStyle(Element*, const String& pseudoElt) const;
+        RefPtr<CSSStyleDeclaration> getComputedStyle(Element&, const String& pseudoElt) const;
 
         // WebKit extensions
 
index 6312663..7e1666f 100644 (file)
     readonly attribute StyleMedia styleMedia;
 
     // DOM Level 2 Style Interface
-    CSSStyleDeclaration getComputedStyle(optional Element? element = null, optional DOMString? pseudoElement = null);
+    [NewObject] CSSStyleDeclaration getComputedStyle(Element element, optional DOMString? pseudoElement = null);
 
     // WebKit extensions
 #if defined(LANGUAGE_JAVASCRIPT) && LANGUAGE_JAVASCRIPT
index cf19ea6..b63c377 100644 (file)
@@ -783,10 +783,10 @@ bool Internals::hasPausedImageAnimations(Element& element)
     return element.renderer() && element.renderer()->hasPausedImageAnimations();
 }
 
-RefPtr<CSSComputedStyleDeclaration> Internals::computedStyleIncludingVisitedInfo(Node& node) const
+RefPtr<CSSComputedStyleDeclaration> Internals::computedStyleIncludingVisitedInfo(Element& element) const
 {
     bool allowVisitedStyle = true;
-    return CSSComputedStyleDeclaration::create(&node, allowVisitedStyle);
+    return CSSComputedStyleDeclaration::create(element, allowVisitedStyle);
 }
 
 Node* Internals::ensureShadowRoot(Element& host, ExceptionCode& ec)
index 4dad966..72c0ec5 100644 (file)
@@ -107,7 +107,7 @@ public:
     void clearPageCache();
     unsigned pageCacheSize() const;
 
-    RefPtr<CSSComputedStyleDeclaration> computedStyleIncludingVisitedInfo(Node&) const;
+    RefPtr<CSSComputedStyleDeclaration> computedStyleIncludingVisitedInfo(Element&) const;
 
     Node* ensureShadowRoot(Element& host, ExceptionCode&);
     Node* ensureUserAgentShadowRoot(Element& host);
index 469a783..6e863e7 100644 (file)
@@ -98,7 +98,7 @@ enum UserInterfaceLayoutDirection {
     void clearPageCache();
     unsigned long pageCacheSize();
 
-    CSSStyleDeclaration computedStyleIncludingVisitedInfo(Node node);
+    CSSStyleDeclaration computedStyleIncludingVisitedInfo(Element element);
 
     [RaisesException] Node ensureShadowRoot(Element host);
     Node ensureUserAgentShadowRoot(Element host);