Summary: | Drop [UsePointersEvenForNonNullableObjectArguments] from MutationObserver | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||
Component: | Bindings | Assignee: | Chris Dumez <cdumez> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, darin, youennf | ||||||
Priority: | P2 | ||||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Chris Dumez
2016-04-21 20:13:49 PDT
Created attachment 277005 [details]
Patch
Comment on attachment 277005 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=277005&action=review > Source/WebCore/bindings/js/JSMutationObserverCustom.cpp:58 > + Ref<JSMutationCallback> callback = JSMutationCallback::create(object, jsConstructor->globalObject()); > + JSObject* jsObserver = asObject(toJS(exec, jsConstructor->globalObject(), MutationObserver::create(WTFMove(callback)))); I would have used auto here or maybe even put everything on one line! > Source/WebCore/dom/ChildListMutationScope.cpp:133 > + Ref<NodeList> addedNodes = StaticNodeList::adopt(m_addedNodes); > + Ref<NodeList> removedNodes = StaticNodeList::adopt(m_removedNodes); > + Ref<MutationRecord> record = MutationRecord::createChildList(m_target, WTFMove(addedNodes), WTFMove(removedNodes), WTFMove(m_previousSibling), WTFMove(m_nextSibling)); > + m_observers->enqueueMutationRecord(WTFMove(record)); I would have used auto here more. Or not used local variables so much. Or some of each. > Source/WebCore/dom/MutationObserverInterestGroup.cpp:82 > + observer->enqueueMutationRecord(*mutationWithNullOldValue); This should be mutationWithNullOldValue.releaseNonNull(). Comment on attachment 277005 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=277005&action=review >> Source/WebCore/dom/MutationObserverInterestGroup.cpp:82 >> + observer->enqueueMutationRecord(*mutationWithNullOldValue); > > This should be mutationWithNullOldValue.releaseNonNull(). I think this may change behavior since this is in a for loop and we check if mutationWithNullOldValue is null earlier. Created attachment 277061 [details]
Patch
Comment on attachment 277061 [details] Patch Clearing flags on attachment: 277061 Committed r199878: <http://trac.webkit.org/changeset/199878> All reviewed patches have been landed. Closing bug. |