Drop [UsePointersEvenForNonNullableObjectArguments] from MutationObserver
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 Apr 2016 15:59:10 +0000 (15:59 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 Apr 2016 15:59:10 +0000 (15:59 +0000)
https://bugs.webkit.org/show_bug.cgi?id=156890

Reviewed by Darin Adler.

Source/WebCore:

Drop [UsePointersEvenForNonNullableObjectArguments] from MutationObserver
and clean up / modernize the code a bit. There is not significant Web-
exposed behavior change except that MutationObserver.observe() now throws
a different kind of exception (a TypeError as per Web IDL) when passed in
a null Node.

No new tests, rebaselined existing test.

* bindings/js/JSMutationCallback.cpp:
(WebCore::JSMutationCallback::call):
* bindings/js/JSMutationCallback.h:
* bindings/js/JSMutationObserverCustom.cpp:
(WebCore::constructJSMutationObserver):
* css/PropertySetCSSStyleDeclaration.cpp:
* dom/ChildListMutationScope.cpp:
(WebCore::ChildListMutationAccumulator::enqueueMutationRecord):
* dom/MutationCallback.h:
* dom/MutationObserver.cpp:
(WebCore::MutationObserver::create):
(WebCore::MutationObserver::MutationObserver):
(WebCore::MutationObserver::observe):
(WebCore::MutationObserver::takeRecords):
(WebCore::MutationObserver::enqueueMutationRecord):
(WebCore::MutationObserver::deliver):
(WebCore::MutationObserver::disconnect): Deleted.
* dom/MutationObserver.h:
* dom/MutationObserver.idl:
* dom/MutationObserverInterestGroup.cpp:
(WebCore::MutationObserverInterestGroup::enqueueMutationRecord):
* dom/MutationObserverInterestGroup.h:
* dom/MutationRecord.cpp:
(WebCore::MutationRecord::createChildList):
* dom/MutationRecord.h:

LayoutTests:

Rebaseline now that MutationObserver.observe() throws a TypeError instead
of a NOT_FOUND_ERR when passed a null Node.

* fast/dom/MutationObserver/observe-exceptions-expected.txt:

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

16 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/dom/MutationObserver/observe-exceptions-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSMutationCallback.cpp
Source/WebCore/bindings/js/JSMutationCallback.h
Source/WebCore/bindings/js/JSMutationObserverCustom.cpp
Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp
Source/WebCore/dom/ChildListMutationScope.cpp
Source/WebCore/dom/MutationCallback.h
Source/WebCore/dom/MutationObserver.cpp
Source/WebCore/dom/MutationObserver.h
Source/WebCore/dom/MutationObserver.idl
Source/WebCore/dom/MutationObserverInterestGroup.cpp
Source/WebCore/dom/MutationObserverInterestGroup.h
Source/WebCore/dom/MutationRecord.cpp
Source/WebCore/dom/MutationRecord.h

index 87163ed..24b8187 100644 (file)
@@ -1,3 +1,15 @@
+2016-04-22  Chris Dumez  <cdumez@apple.com>
+
+        Drop [UsePointersEvenForNonNullableObjectArguments] from MutationObserver
+        https://bugs.webkit.org/show_bug.cgi?id=156890
+
+        Reviewed by Darin Adler.
+
+        Rebaseline now that MutationObserver.observe() throws a TypeError instead
+        of a NOT_FOUND_ERR when passed a null Node.
+
+        * fast/dom/MutationObserver/observe-exceptions-expected.txt:
+
 2016-04-22  Dave Hyatt  <hyatt@apple.com>
 
         REGRESSION (r189567): The top of Facebook's messenger.com looks visually broken
index 2f6bb4f..51eb460 100644 (file)
@@ -7,17 +7,17 @@ PASS observer.observe() threw exception TypeError: Not enough arguments.
 PASS observer.observe(null) threw exception TypeError: Not enough arguments.
 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: SyntaxError: DOM Exception 12.
-PASS observer.observe(document.body, undefined) threw exception Error: SyntaxError: DOM Exception 12.
-PASS observer.observe(null, {attributes: true}) threw exception Error: NotFoundError: DOM Exception 8.
-PASS observer.observe(undefined, {attributes: true}) threw exception Error: NotFoundError: DOM Exception 8.
-PASS observer.observe(document.body, {subtree: true}) threw exception Error: SyntaxError: DOM Exception 12.
+PASS observer.observe(document.body, null) threw exception TypeError: Type error.
+PASS observer.observe(document.body, undefined) threw exception TypeError: Type error.
+PASS observer.observe(null, {attributes: true}) threw exception TypeError: Type error.
+PASS observer.observe(undefined, {attributes: true}) threw exception TypeError: Type error.
+PASS observer.observe(document.body, {subtree: true}) threw exception TypeError: Type error.
 PASS observer.observe(document.body, {childList: true, attributeOldValue: true}) did not throw exception.
 PASS observer.observe(document.body, {attributes: true, characterDataOldValue: true}) did not throw exception.
 PASS observer.observe(document.body, {characterData: true, attributeFilter: ["id"]}) did not throw exception.
-PASS observer.observe(document.body, {attributes: false, attributeOldValue: true}) threw exception Error: SyntaxError: DOM Exception 12.
-PASS observer.observe(document.body, {attributes: false, attributeFilter: ["id"]}) threw exception Error: SyntaxError: DOM Exception 12.
-PASS observer.observe(document.body, {characterData: false, characterDataOldValue: true}) threw exception Error: SyntaxError: DOM Exception 12.
+PASS observer.observe(document.body, {attributes: false, attributeOldValue: true}) threw exception TypeError: Type error.
+PASS observer.observe(document.body, {attributes: false, attributeFilter: ["id"]}) threw exception TypeError: Type error.
+PASS observer.observe(document.body, {characterData: false, characterDataOldValue: true}) threw exception TypeError: Type error.
 PASS successfullyParsed is true
 
 TEST COMPLETE
index 3eb1f1d..f84b01f 100644 (file)
@@ -1,3 +1,44 @@
+2016-04-22  Chris Dumez  <cdumez@apple.com>
+
+        Drop [UsePointersEvenForNonNullableObjectArguments] from MutationObserver
+        https://bugs.webkit.org/show_bug.cgi?id=156890
+
+        Reviewed by Darin Adler.
+
+        Drop [UsePointersEvenForNonNullableObjectArguments] from MutationObserver
+        and clean up / modernize the code a bit. There is not significant Web-
+        exposed behavior change except that MutationObserver.observe() now throws
+        a different kind of exception (a TypeError as per Web IDL) when passed in
+        a null Node.
+
+        No new tests, rebaselined existing test.
+
+        * bindings/js/JSMutationCallback.cpp:
+        (WebCore::JSMutationCallback::call):
+        * bindings/js/JSMutationCallback.h:
+        * bindings/js/JSMutationObserverCustom.cpp:
+        (WebCore::constructJSMutationObserver):
+        * css/PropertySetCSSStyleDeclaration.cpp:
+        * dom/ChildListMutationScope.cpp:
+        (WebCore::ChildListMutationAccumulator::enqueueMutationRecord):
+        * dom/MutationCallback.h:
+        * dom/MutationObserver.cpp:
+        (WebCore::MutationObserver::create):
+        (WebCore::MutationObserver::MutationObserver):
+        (WebCore::MutationObserver::observe):
+        (WebCore::MutationObserver::takeRecords):
+        (WebCore::MutationObserver::enqueueMutationRecord):
+        (WebCore::MutationObserver::deliver):
+        (WebCore::MutationObserver::disconnect): Deleted.
+        * dom/MutationObserver.h:
+        * dom/MutationObserver.idl:
+        * dom/MutationObserverInterestGroup.cpp:
+        (WebCore::MutationObserverInterestGroup::enqueueMutationRecord):
+        * dom/MutationObserverInterestGroup.h:
+        * dom/MutationRecord.cpp:
+        (WebCore::MutationRecord::createChildList):
+        * dom/MutationRecord.h:
+
 2016-04-22  Dave Hyatt  <hyatt@apple.com>
 
         REGRESSION (r189567): The top of Facebook's messenger.com looks visually broken
index efe35aa..d0a7bb0 100644 (file)
@@ -51,7 +51,7 @@ JSMutationCallback::~JSMutationCallback()
 {
 }
 
-void JSMutationCallback::call(const Vector<RefPtr<MutationRecord>>& mutations, MutationObserver* observer)
+void JSMutationCallback::call(const Vector<Ref<MutationRecord>>& mutations, MutationObserver* observer)
 {
     if (!canInvokeCallback())
         return;
index 57e5949..5467aaa 100644 (file)
@@ -46,7 +46,7 @@ public:
 
     virtual ~JSMutationCallback();
 
-    void call(const Vector<RefPtr<MutationRecord>>&, MutationObserver*) override;
+    void call(const Vector<Ref<MutationRecord>>&, MutationObserver*) override;
 
     ScriptExecutionContext* scriptExecutionContext() const override { return ContextDestructionObserver::scriptExecutionContext(); }
 
index c73fca3..6e24f24 100644 (file)
@@ -54,8 +54,8 @@ EncodedJSValue JSC_HOST_CALL constructJSMutationObserver(ExecState* exec)
         return throwVMError(exec, createTypeError(exec, "Callback argument must be a function"));
 
     DOMConstructorObject* jsConstructor = jsCast<DOMConstructorObject*>(exec->callee());
-    RefPtr<JSMutationCallback> callback = JSMutationCallback::create(object, jsConstructor->globalObject());
-    JSObject* jsObserver = asObject(toJS(exec, jsConstructor->globalObject(), MutationObserver::create(callback.release())));
+    auto callback = JSMutationCallback::create(object, jsConstructor->globalObject());
+    JSObject* jsObserver = asObject(toJS(exec, jsConstructor->globalObject(), MutationObserver::create(WTFMove(callback))));
     PrivateName propertyName;
     jsObserver->putDirect(jsConstructor->globalObject()->vm(), propertyName, object);
     return JSValue::encode(jsObserver);
index 86418d4..8743214 100644 (file)
@@ -77,7 +77,7 @@ public:
             return;
 
         if (m_mutation && s_shouldDeliver)
-            m_mutationRecipients->enqueueMutationRecord(m_mutation);
+            m_mutationRecipients->enqueueMutationRecord(m_mutation.releaseNonNull());
 
         s_shouldDeliver = false;
         if (!s_shouldNotifyInspector) {
index f6de317..eadcdd0 100644 (file)
@@ -127,10 +127,8 @@ void ChildListMutationAccumulator::enqueueMutationRecord()
     ASSERT(hasObservers());
     ASSERT(!isEmpty());
 
-    RefPtr<NodeList> addedNodes = StaticNodeList::adopt(m_addedNodes);
-    RefPtr<NodeList> removedNodes = StaticNodeList::adopt(m_removedNodes);
-    RefPtr<MutationRecord> record = MutationRecord::createChildList(m_target, addedNodes.release(), removedNodes.release(), m_previousSibling.release(), m_nextSibling.release());
-    m_observers->enqueueMutationRecord(record.release());
+    auto record = MutationRecord::createChildList(m_target, StaticNodeList::adopt(m_addedNodes), StaticNodeList::adopt(m_removedNodes), WTFMove(m_previousSibling), WTFMove(m_nextSibling));
+    m_observers->enqueueMutationRecord(WTFMove(record));
     m_lastAdded = nullptr;
     ASSERT(isEmpty());
 }
index e698a59..620c2fc 100644 (file)
@@ -44,7 +44,7 @@ class MutationCallback : public RefCounted<MutationCallback> {
 public:
     virtual ~MutationCallback() { }
 
-    virtual void call(const Vector<RefPtr<MutationRecord>>&, MutationObserver*) = 0;
+    virtual void call(const Vector<Ref<MutationRecord>>&, MutationObserver*) = 0;
     virtual ScriptExecutionContext* scriptExecutionContext() const = 0;
 };
 
index b54a19f..a690cfa 100644 (file)
@@ -46,14 +46,14 @@ namespace WebCore {
 
 static unsigned s_observerPriority = 0;
 
-Ref<MutationObserver> MutationObserver::create(PassRefPtr<MutationCallback> callback)
+Ref<MutationObserver> MutationObserver::create(Ref<MutationCallback>&& callback)
 {
     ASSERT(isMainThread());
-    return adoptRef(*new MutationObserver(callback));
+    return adoptRef(*new MutationObserver(WTFMove(callback)));
 }
 
-MutationObserver::MutationObserver(PassRefPtr<MutationCallback> callback)
-    : m_callback(callback)
+MutationObserver::MutationObserver(Ref<MutationCallback>&& callback)
+    : m_callback(WTFMove(callback))
     , m_priority(s_observerPriority++)
 {
 }
@@ -71,13 +71,8 @@ bool MutationObserver::validateOptions(MutationObserverOptions options)
         && ((options & CharacterData) || !(options & CharacterDataOldValue));
 }
 
-void MutationObserver::observe(Node* node, const Dictionary& optionsDictionary, ExceptionCode& ec)
+void MutationObserver::observe(Node& node, const Dictionary& optionsDictionary, ExceptionCode& ec)
 {
-    if (!node) {
-        ec = NOT_FOUND_ERR;
-        return;
-    }
-
     static const struct {
         const char* name;
         MutationObserverOptions value;
@@ -107,16 +102,16 @@ void MutationObserver::observe(Node* node, const Dictionary& optionsDictionary,
         options |= CharacterData;
 
     if (!validateOptions(options)) {
-        ec = SYNTAX_ERR;
+        ec = TypeError;
         return;
     }
 
-    node->registerMutationObserver(this, options, attributeFilter);
+    node.registerMutationObserver(this, options, attributeFilter);
 }
 
-Vector<RefPtr<MutationRecord>> MutationObserver::takeRecords()
+Vector<Ref<MutationRecord>> MutationObserver::takeRecords()
 {
-    Vector<RefPtr<MutationRecord>> records;
+    Vector<Ref<MutationRecord>> records;
     records.swap(m_records);
     return records;
 }
@@ -189,10 +184,10 @@ static void queueMutationObserverCompoundMicrotask()
     MicrotaskQueue::mainThreadQueue().append(WTFMove(microtask));
 }
 
-void MutationObserver::enqueueMutationRecord(PassRefPtr<MutationRecord> mutation)
+void MutationObserver::enqueueMutationRecord(Ref<MutationRecord>&& mutation)
 {
     ASSERT(isMainThread());
-    m_records.append(mutation);
+    m_records.append(WTFMove(mutation));
     activeMutationObservers().add(this);
 
     queueMutationObserverCompoundMicrotask();
@@ -236,7 +231,7 @@ void MutationObserver::deliver()
     if (m_records.isEmpty())
         return;
 
-    Vector<RefPtr<MutationRecord>> records;
+    Vector<Ref<MutationRecord>> records;
     records.swap(m_records);
 
     m_callback->call(records, this);
index 1d8e1bd..d84aaf8 100644 (file)
@@ -73,16 +73,16 @@ public:
         CharacterDataOldValue = 1 << 6,
     };
 
-    static Ref<MutationObserver> create(PassRefPtr<MutationCallback>);
+    static Ref<MutationObserver> create(Ref<MutationCallback>&&);
 
     ~MutationObserver();
 
-    void observe(Node*, const Dictionary&, ExceptionCode&);
-    Vector<RefPtr<MutationRecord>> takeRecords();
+    void observe(Node&, const Dictionary&, ExceptionCode&);
+    Vector<Ref<MutationRecord>> takeRecords();
     void disconnect();
     void observationStarted(MutationObserverRegistration*);
     void observationEnded(MutationObserverRegistration*);
-    void enqueueMutationRecord(PassRefPtr<MutationRecord>);
+    void enqueueMutationRecord(Ref<MutationRecord>&&);
     void setHasTransientRegistration();
     bool canDeliver();
 
@@ -91,14 +91,14 @@ public:
 private:
     struct ObserverLessThan;
 
-    explicit MutationObserver(PassRefPtr<MutationCallback>);
+    explicit MutationObserver(Ref<MutationCallback>&&);
     void deliver();
 
     static void deliverAllMutations();
     static bool validateOptions(MutationObserverOptions);
 
-    RefPtr<MutationCallback> m_callback;
-    Vector<RefPtr<MutationRecord>> m_records;
+    Ref<MutationCallback> m_callback;
+    Vector<Ref<MutationRecord>> m_records;
     HashSet<MutationObserverRegistration*> m_registrations;
     unsigned m_priority;
 };
index 5a7c71f..9ee9792 100644 (file)
@@ -31,7 +31,6 @@
 [
     CustomConstructor(MutationCallback callback),
     CustomIsReachable,
-    UsePointersEvenForNonNullableObjectArguments,
     ImplementationLacksVTable,
 ] interface MutationObserver {
     [RaisesException] void observe(Node target, Dictionary options);
index 7f4adc4..f6b42e4 100644 (file)
@@ -64,23 +64,22 @@ bool MutationObserverInterestGroup::isOldValueRequested()
     return false;
 }
 
-void MutationObserverInterestGroup::enqueueMutationRecord(PassRefPtr<MutationRecord> prpMutation)
+void MutationObserverInterestGroup::enqueueMutationRecord(Ref<MutationRecord>&& mutation)
 {
-    RefPtr<MutationRecord> mutation = prpMutation;
     RefPtr<MutationRecord> mutationWithNullOldValue;
     for (auto& observerOptionsPair : m_observers) {
         MutationObserver* observer = observerOptionsPair.key;
         if (hasOldValue(observerOptionsPair.value)) {
-            observer->enqueueMutationRecord(mutation);
+            observer->enqueueMutationRecord(mutation.copyRef());
             continue;
         }
         if (!mutationWithNullOldValue) {
             if (mutation->oldValue().isNull())
-                mutationWithNullOldValue = mutation;
+                mutationWithNullOldValue = mutation.ptr();
             else
-                mutationWithNullOldValue = MutationRecord::createWithNullOldValue(*mutation).ptr();
+                mutationWithNullOldValue = MutationRecord::createWithNullOldValue(mutation).ptr();
         }
-        observer->enqueueMutationRecord(mutationWithNullOldValue);
+        observer->enqueueMutationRecord(*mutationWithNullOldValue);
     }
 }
 
index 2f773bc..055971e 100644 (file)
@@ -70,7 +70,7 @@ public:
     }
 
     bool isOldValueRequested();
-    void enqueueMutationRecord(PassRefPtr<MutationRecord>);
+    void enqueueMutationRecord(Ref<MutationRecord>&&);
 
 private:
     static std::unique_ptr<MutationObserverInterestGroup> createIfNeeded(Node& target, MutationObserver::MutationType, MutationRecordDeliveryOptions oldValueFlag, const QualifiedName* attributeName = nullptr);
index 9dfde3e..402686e 100644 (file)
@@ -42,12 +42,12 @@ namespace {
 
 class ChildListRecord : public MutationRecord {
 public:
-    ChildListRecord(ContainerNode& target, PassRefPtr<NodeList> added, PassRefPtr<NodeList> removed, PassRefPtr<Node> previousSibling, PassRefPtr<Node> nextSibling)
+    ChildListRecord(ContainerNode& target, Ref<NodeList>&& added, Ref<NodeList>&& removed, RefPtr<Node>&& previousSibling, RefPtr<Node>&& nextSibling)
         : m_target(target)
-        , m_addedNodes(added)
-        , m_removedNodes(removed)
-        , m_previousSibling(previousSibling)
-        , m_nextSibling(nextSibling)
+        , m_addedNodes(WTFMove(added))
+        , m_removedNodes(WTFMove(removed))
+        , m_previousSibling(WTFMove(previousSibling))
+        , m_nextSibling(WTFMove(nextSibling))
     {
     }
 
@@ -164,9 +164,9 @@ const AtomicString& CharacterDataRecord::type()
 
 } // namespace
 
-Ref<MutationRecord> MutationRecord::createChildList(ContainerNode& target, PassRefPtr<NodeList> added, PassRefPtr<NodeList> removed, PassRefPtr<Node> previousSibling, PassRefPtr<Node> nextSibling)
+Ref<MutationRecord> MutationRecord::createChildList(ContainerNode& target, Ref<NodeList>&& added, Ref<NodeList>&& removed, RefPtr<Node>&& previousSibling, RefPtr<Node>&& nextSibling)
 {
-    return adoptRef(static_cast<MutationRecord&>(*new ChildListRecord(target, added, removed, previousSibling, nextSibling)));
+    return adoptRef(static_cast<MutationRecord&>(*new ChildListRecord(target, WTFMove(added), WTFMove(removed), WTFMove(previousSibling), WTFMove(nextSibling))));
 }
 
 Ref<MutationRecord> MutationRecord::createAttributes(Element& target, const QualifiedName& name, const AtomicString& oldValue)
index 1709ab6..3683519 100644 (file)
@@ -47,7 +47,7 @@ class QualifiedName;
 
 class MutationRecord : public RefCounted<MutationRecord> {
 public:
-    static Ref<MutationRecord> createChildList(ContainerNode& target, PassRefPtr<NodeList> added, PassRefPtr<NodeList> removed, PassRefPtr<Node> previousSibling, PassRefPtr<Node> nextSibling);
+    static Ref<MutationRecord> createChildList(ContainerNode& target, Ref<NodeList>&& added, Ref<NodeList>&& removed, RefPtr<Node>&& previousSibling, RefPtr<Node>&& nextSibling);
     static Ref<MutationRecord> createAttributes(Element& target, const QualifiedName&, const AtomicString& oldValue);
     static Ref<MutationRecord> createCharacterData(CharacterData& target, const String& oldValue);