Attachment 114094 Details for Bug 71596 – Patch for landing

[patch] Patch for landing

bug-71596-20111108093349.patch (text/plain), 9.36 KB, created by Adam Klein on 2011-11-08 09:33:50 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Adam Klein

Created: 2011-11-08 09:33:50 PST

Size: 9.36 KB

patch

obsolete

>Subversion Revision: 99341
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 040446aaebb52cc6095c1bfc0872b9617409a012..e806855bb43bbc6768096e8e21654b11d1d53689 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,25 @@
>+2011-11-07  Adam Klein  <adamk@chromium.org>
>+
>+        WebKitMutationObserver.observe should raise a DOMException if passed invalid arguments
>+        https://bugs.webkit.org/show_bug.cgi?id=71596
>+
>+        Reviewed by Ryosuke Niwa.
>+
>+        Adds two cases where WebKitMutationObserver.observe throws an exception:
>+          - When passed a null Node*.
>+          - When passed options that don't make sense, e.g.,
>+            'attributeOldValue' but not 'attributes'.
>+
>+        * bindings/js/JSWebKitMutationObserverCustom.cpp:
>+        (WebCore::JSWebKitMutationObserver::observe):
>+        * bindings/v8/custom/V8WebKitMutationObserverCustom.cpp:
>+        (WebCore::V8WebKitMutationObserver::observeCallback):
>+        * dom/WebKitMutationObserver.cpp:
>+        (WebCore::WebKitMutationObserver::validateOptions):
>+        (WebCore::WebKitMutationObserver::observe):
>+        * dom/WebKitMutationObserver.h:
>+        * dom/WebKitMutationObserver.idl:
>+
> 2011-11-04  Stephen Chenney  <schenney@chromium.org>
> 
>         Crash in ScrollAnimator.cpp
>diff --git a/Source/WebCore/bindings/js/JSWebKitMutationObserverCustom.cpp b/Source/WebCore/bindings/js/JSWebKitMutationObserverCustom.cpp
>index cd613ff160e577723f35cf22e51b1d618b50c321..018ee424e912b035f6b35cc46b1acdde10f5075d 100644
>--- a/Source/WebCore/bindings/js/JSWebKitMutationObserverCustom.cpp
>+++ b/Source/WebCore/bindings/js/JSWebKitMutationObserverCustom.cpp
>@@ -93,7 +93,10 @@ JSValue JSWebKitMutationObserver::observe(ExecState* exec)
>     if (exec->hadException())
>         return jsUndefined();
> 
>-    impl()->observe(target, options);
>+    ExceptionCode ec = 0;
>+    impl()->observe(target, options, ec);
>+    if (ec)
>+        setDOMException(exec, ec);
>     return jsUndefined();
> }
> 
>diff --git a/Source/WebCore/bindings/v8/custom/V8WebKitMutationObserverCustom.cpp b/Source/WebCore/bindings/v8/custom/V8WebKitMutationObserverCustom.cpp
>index ef53eaf27ef1053ce70eade9d4c773cb43c34874..966dcf8838a7410a793a13f65f0ba3602390fd88 100644
>--- a/Source/WebCore/bindings/v8/custom/V8WebKitMutationObserverCustom.cpp
>+++ b/Source/WebCore/bindings/v8/custom/V8WebKitMutationObserverCustom.cpp
>@@ -103,7 +103,10 @@ v8::Handle<v8::Value> V8WebKitMutationObserver::observeCallback(const v8::Argume
>     if (optionsObject.get("characterDataOldValue", option) && option)
>         options |= WebKitMutationObserver::CharacterDataOldValue;
> 
>-    imp->observe(target, options);
>+    ExceptionCode ec = 0;
>+    imp->observe(target, options, ec);
>+    if (ec)
>+        V8Proxy::setDOMException(ec);
>     return v8::Handle<v8::Value>();
> }
> 
>diff --git a/Source/WebCore/dom/WebKitMutationObserver.cpp b/Source/WebCore/dom/WebKitMutationObserver.cpp
>index 0c7cbfc240d19e19cc1965def78a265750b6e760..23a5a8aeea6cd9760270f2586c34bd0ade431ab7 100644
>--- a/Source/WebCore/dom/WebKitMutationObserver.cpp
>+++ b/Source/WebCore/dom/WebKitMutationObserver.cpp
>@@ -34,6 +34,7 @@
> 
> #include "WebKitMutationObserver.h"
> 
>+#include "ExceptionCode.h"
> #include "MutationCallback.h"
> #include "MutationObserverRegistration.h"
> #include "MutationRecord.h"
>@@ -57,8 +58,28 @@ WebKitMutationObserver::~WebKitMutationObserver()
>     ASSERT(m_registrations.isEmpty());
> }
> 
>-void WebKitMutationObserver::observe(Node* node, MutationObserverOptions options)
>+bool WebKitMutationObserver::validateOptions(MutationObserverOptions options)
> {
>+    return (options & (Attributes | CharacterData | ChildList))
>+        && ((options & Attributes) || !(options & AttributeOldValue))
>+        // FIXME: Uncomment the line below once attributeFilter is supported.
>+        // && ((options & Attributes) || !(options & AttributeFilter))
>+        && ((options & CharacterData) || !(options & CharacterDataOldValue));
>+}
>+
>+void WebKitMutationObserver::observe(Node* node, MutationObserverOptions options, ExceptionCode& ec)
>+{
>+    if (!node) {
>+        ec = NOT_FOUND_ERR;
>+        return;
>+    }
>+
>+    if (!validateOptions(options)) {
>+        // FIXME: Revisit this once the spec specifies the exception type; SYNTAX_ERR may not be appropriate.
>+        ec = SYNTAX_ERR;
>+        return;
>+    }
>+
>     MutationObserverRegistration* registration = node->registerMutationObserver(this);
>     registration->resetObservation(options);
> }
>diff --git a/Source/WebCore/dom/WebKitMutationObserver.h b/Source/WebCore/dom/WebKitMutationObserver.h
>index fe6e6e27e09a61473ffd82e4d7aeadee77ca3db9..636e1ff4d42ce14df07a39c4cb9c0a875a6e9b55 100644
>--- a/Source/WebCore/dom/WebKitMutationObserver.h
>+++ b/Source/WebCore/dom/WebKitMutationObserver.h
>@@ -33,6 +33,7 @@
> 
> #if ENABLE(MUTATION_OBSERVERS)
> 
>+#include "ExceptionCode.h"
> #include <wtf/HashSet.h>
> #include <wtf/PassRefPtr.h>
> #include <wtf/RefCounted.h>
>@@ -73,7 +74,7 @@ public:
> 
>     ~WebKitMutationObserver();
> 
>-    void observe(Node*, MutationObserverOptions);
>+    void observe(Node*, MutationObserverOptions, ExceptionCode&);
>     void disconnect();
>     void observationStarted(MutationObserverRegistration*);
>     void observationEnded(MutationObserverRegistration*);
>@@ -83,6 +84,8 @@ private:
>     WebKitMutationObserver(PassRefPtr<MutationCallback>);
>     void deliver();
> 
>+    static bool validateOptions(MutationObserverOptions);
>+
>     RefPtr<MutationCallback> m_callback;
>     Vector<RefPtr<MutationRecord> > m_records;
>     HashSet<MutationObserverRegistration*> m_registrations;
>diff --git a/Source/WebCore/dom/WebKitMutationObserver.idl b/Source/WebCore/dom/WebKitMutationObserver.idl
>index da6e8c7eec00a0b001519f1c33456285873656a7..b54575f5a695caa9f706a7b310030756e3e902fa 100644
>--- a/Source/WebCore/dom/WebKitMutationObserver.idl
>+++ b/Source/WebCore/dom/WebKitMutationObserver.idl
>@@ -34,7 +34,8 @@ module core {
>         CanBeConstructed,
>         CustomConstructor
>     ] WebKitMutationObserver {
>-        [Custom] void observe(in Node target, in MutationObserverOptions options);
>+        [Custom] void observe(in Node target, in MutationObserverOptions options)
>+            raises(DOMException);
>         void disconnect();
>     };
> }
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index db0941f3c3247708f840c39089e963d2db9c555a..45ed6701050e8415aef83e26175cde65e2cf7209 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,13 @@
>+2011-11-07  Adam Klein  <adamk@chromium.org>
>+
>+        WebKitMutationObserver.observe should raise a DOMException if passed invalid arguments
>+        https://bugs.webkit.org/show_bug.cgi?id=71596
>+
>+        Reviewed by Ryosuke Niwa.
>+
>+        * fast/mutation/observe-exceptions-expected.txt:
>+        * fast/mutation/observe-exceptions.html:
>+
> 2011-11-04  Stephen Chenney  <schenney@chromium.org>
> 
>         Crash in ScrollAnimator.cpp
>diff --git a/LayoutTests/fast/mutation/observe-exceptions-expected.txt b/LayoutTests/fast/mutation/observe-exceptions-expected.txt
>index 3f4e3cd1ed7367c12cbd5760525764fad8e8eb74..5390120da891051f516e1983944d690b8bbfab72 100644
>--- a/LayoutTests/fast/mutation/observe-exceptions-expected.txt
>+++ b/LayoutTests/fast/mutation/observe-exceptions-expected.txt
>@@ -9,6 +9,11 @@ PASS observer.observe(undefined) threw exception TypeError: Not enough arguments
> PASS observer.observe(document.body) threw exception TypeError: Not enough arguments.
> PASS observer.observe(document.body, null) threw exception Error: TYPE_MISMATCH_ERR: DOM Exception 17.
> PASS observer.observe(document.body, undefined) threw exception Error: TYPE_MISMATCH_ERR: DOM Exception 17.
>+PASS observer.observe(null, {attributes: true}) threw exception Error: NOT_FOUND_ERR: DOM Exception 8.
>+PASS observer.observe(undefined, {attributes: true}) threw exception Error: NOT_FOUND_ERR: DOM Exception 8.
>+PASS observer.observe(document.body, {subtree: true}) threw exception Error: SYNTAX_ERR: DOM Exception 12.
>+PASS observer.observe(document.body, {childList: true, attributeOldValue: true}) threw exception Error: SYNTAX_ERR: DOM Exception 12.
>+PASS observer.observe(document.body, {attributes: true, characterDataOldValue: true}) threw exception Error: SYNTAX_ERR: DOM Exception 12.
> PASS successfullyParsed is true
> 
> TEST COMPLETE
>diff --git a/LayoutTests/fast/mutation/observe-exceptions.html b/LayoutTests/fast/mutation/observe-exceptions.html
>index a3491d2e8e126bdce483b5ae23b526c5ab85861d..a525a492ce9253d44c07ecf66fcefdda67aa57a0 100644
>--- a/LayoutTests/fast/mutation/observe-exceptions.html
>+++ b/LayoutTests/fast/mutation/observe-exceptions.html
>@@ -18,6 +18,13 @@ function runTest() {
>     shouldThrow('observer.observe(document.body)');
>     shouldThrow('observer.observe(document.body, null)');
>     shouldThrow('observer.observe(document.body, undefined)');
>+    shouldThrow('observer.observe(null, {attributes: true})');
>+    shouldThrow('observer.observe(undefined, {attributes: true})');
>+    shouldThrow('observer.observe(document.body, {subtree: true})');
>+    shouldThrow('observer.observe(document.body, {childList: true, attributeOldValue: true})');
>+    shouldThrow('observer.observe(document.body, {attributes: true, characterDataOldValue: true})');
>+    // FIXME: Uncomment the below when attributeFilter is supported.
>+    //shouldThrow('observer.observe(document.body, {characterData: true, attributeFilter: ["id"]})');
> }
> 
> description('Test that WebKitMutationObserver.observe throws exceptions appropriately');

Actions: View | Formatted Diff | Diff

Attachments on bug 71596: 113923 | 114094 | 114131