Bug 83218

Summary: [MutationObservers] implement takeRecords()
Product: WebKit Reporter: Rafael Weinstein <rafaelw>
Component: DOMAssignee: 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 Flags
Patch
none
Patch none

Description Rafael Weinstein 2012-04-04 14:28:00 PDT
Per https://www.w3.org/Bugs/Public/show_bug.cgi?id=16606, MutationObserver should expose a takeRecords() call which synchronously retrieves & clears any pending mutation records. If none are pending, it should return an empty array. It should not do anything else (in particular, it should not clear transient observers).

Anne has committed to updating the spec by end of week.
Comment 1 Rafael Weinstein 2012-04-04 14:31:12 PDT
Created attachment 135685 [details]
Patch
Comment 2 Adam Klein 2012-04-04 14:40:22 PDT
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 3 Ojan Vafai 2012-04-04 15:01:42 PDT
Comment on attachment 135685 [details]
Patch

r=me once you address Adam's comments.
Comment 4 Build Bot 2012-04-04 15:10:11 PDT
Comment on attachment 135685 [details]
Patch

Attachment 135685 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12331064
Comment 5 Adam Klein 2012-04-04 15:12:03 PDT
(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.
Comment 6 Rafael Weinstein 2012-04-04 15:55:02 PDT
Created attachment 135705 [details]
Patch
Comment 7 WebKit Review Bot 2012-04-04 19:50:43 PDT
Comment on attachment 135705 [details]
Patch

Clearing flags on attachment: 135705

Committed r113279: <http://trac.webkit.org/changeset/113279>
Comment 8 WebKit Review Bot 2012-04-04 19:50:48 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Simon Fraser (smfr) 2012-04-05 11:02:28 PDT
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