[webkit-reviews] review cancelled: [Bug 26350] Make WebInspector's console evaluation/completion asynchronous. : [Attachment 31206] patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has cancelled Pavel Feldman
<pfeldman at chromium.org>'s request for review:
Bug 26350: Make WebInspector's console evaluation/completion asynchronous.
https://bugs.webkit.org/show_bug.cgi?id=26350

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

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
Thanks for the review!

(In reply to comment #2)
> (From update of attachment 31206 [review])
> The code looks correct. I think this just gets ready for asynchronous
> evaluation, but the callback is still called synchronously. Is that correct?
> 

Calls receiving callbacks are always the last ones in the methods, so that it
should not really matter. However you are right - putting these into SetTimeout
would give better guarantees. This is now done.

> Maybe use setTimeout with a zero/quick delay to simulate asynchronous
> behaviour.
> 

Done.

> One concern I have with the completion code being asynchronous is if the user

> keeps typing and the completions for the previous range have not been
returned
> yet. When the results are returned to the callback, the user typed text might

> get messed up or mangled in some way.
> 
> So I think completion results need to be ignored if more user typing has
> happened since. Make sense?
> 

I've added post-evaluation filtering into completionReady, so that only the
ones that do match currently added query are suggested.

I've also extracted couple of methods in the code: 
- installation of the console API 
    (just for convenience)
- Console::doEvalInWindow and ScriptsPanel::doEvalInCallFrame 
    (these are supposed to do the actual eval work over the serialization
layer).