Summary: | [MutationObservers] implement takeRecords() | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Rafael Weinstein <rafaelw> | ||||||
Component: | DOM | Assignee: | Rafael Weinstein <rafaelw> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, adamk, ojan, rniwa, sam, simon.fraser, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 68729 | ||||||||
Attachments: |
|
Description
Rafael Weinstein
2012-04-04 14:28:00 PDT
Created attachment 135685 [details]
Patch
Comment on attachment 135685 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=135685&action=review > Source/WebCore/dom/WebKitMutationObserver.h:36 > +#include "MutationCallback.h" No need for this include... > Source/WebCore/dom/WebKitMutationObserver.h:82 > + MutationRecordArray takeRecords(); ...or this use of the MutationRecordArray typedef. That's only necessary to deal with the way Callbacks are handled by WebKit's IDL processing. I'd make this the same return type name as m_records, i.e., Vector<RefPtr<MutationRecord> >. Alternatively, I suppose you could use the typedef as the type of m_records as well, if you think that type name's too much to be repeating. > Source/WebCore/dom/WebKitMutationObserver.idl:39 > + sequence<MutationRecord> takeRecords(); Awesome, no custom code? > LayoutTests/fast/mutation/observe-subtree.html:59 > +function testTakeRecords() { I'd rather see this test in its own file for clarity: it's testing the takeRecords method, not subtree observation. Comment on attachment 135685 [details]
Patch
r=me once you address Adam's comments.
Comment on attachment 135685 [details] Patch Attachment 135685 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12331064 (In reply to comment #4) > (From update of attachment 135685 [details]) > Attachment 135685 [details] did not pass mac-ews (mac): > Output: http://queues.webkit.org/results/12331064 As a bonus, not using the typedef (and thus the #include of MutationCallback.h) should make this error go away. Created attachment 135705 [details]
Patch
Comment on attachment 135705 [details] Patch Clearing flags on attachment: 135705 Committed r113279: <http://trac.webkit.org/changeset/113279> All reviewed patches have been landed. Closing bug. This broke the window-properties.html test: http://build.webkit.org/results/Lion%20Release%20(Tests)/r113279%20(7284)/fast/dom/Window/window-properties-pretty-diff.html |