[webkit-reviews] review granted: [Bug 70289] Add JSC support for delivering mutations when the outermost script context exits : [Attachment 123864] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has granted Adam Klein <adamk at chromium.org>'s
request for review:
Bug 70289: Add JSC support for delivering mutations when the outermost script
context exits
https://bugs.webkit.org/show_bug.cgi?id=70289

Attachment 123864: Patch
https://bugs.webkit.org/attachment.cgi?id=123864&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=123864&action=review


This looks very reasonable to me.  I suspect the DRT Obj-c code could be
slightly nicer.  I'm comfortable r+ing this based on ggaren's OKing of the
patch, and Sam's original posting of the JSC ->JSMaintThreadExecState change.

I'm assuming here that you audited (or Sam audited?) all callers of JSC::call? 
It seems very possible that one got forgotten or that one will be forgotten in
the future.

> Tools/DumpRenderTree/mac/EventSendingController.mm:728
> +    [character release];
> +    [modifiers release];

You don't need these. :)

> Tools/DumpRenderTree/mac/EventSendingController.mm:734
> +    NSInvocation *invocation = [NSInvocation
invocationWithMethodSignature:[EventSendingController
instanceMethodSignatureForSelector:@selector(keyDownWrapper:withModifiers:withL
ocation:)]];
> +    [invocation retainArguments];

I suspect you could also do this with a block in modern Obj-C 2.  I don't know
how much (if any) of Obj-C 2 we use in DRT, but I"m sure it's supported.

> Tools/DumpRenderTree/mac/EventSendingController.mm:740
> +    [invocation performSelector:@selector(invoke) withObject:nil
afterDelay:0];

What about a timer?  Again, possibly using a block?