Defer most Quick View processing until after delay

[redmunds]

This is for #6538. This uses the getHoveredEditor() function from @SAplayer's pull request #6571, so this is a replacement of that PR.

Details:

• only processes mousemove event in requestAnimationFrame()
• defers all processing until after setTimeout() delay
• exception: when Quick View is already being displayed, then processing is immediate

Results:
I did some simple profiling on Win7 using the src/extensions/default/QuickView/unittest-files/test.css file and Process Explorer. I moved mouse over the css rules in that file at an even pace -- so that only mouse processing was executed (i.e. didn't stop long enough to display Quick View).

• Before: cpu averages ~2%
• After: cpu averages ~1%

cc: @peterflynn, @dangoor

Note: this is a squashed version of PR #6697 (which was used for testing a Travis problem, so had several unnecessary commits)

[dangoor]

I think I will likely get to this on Monday.

[peterflynn]

@redmunds What do you think of the larger improvement suggested in #6571? That is, ditch usage of coordsChar() and just tolerate a fixed number of pixels movement before restarting the timeout? That seems like it could be an equally big win (maybe even bigger) compared to the limited debouncing that rAF gives us...

[peterflynn]

Nit: don't need the webkit prefix here (which I think will be desupported soon anyway).

[redmunds]

What do you think of the larger improvement suggested in #6571? That is, ditch usage of coordsChar() and just tolerate a fixed number of pixels movement before restarting the timeout? That seems like it could be an equally big win (maybe even bigger) compared to the limited debouncing that rAF gives us...

Debouncing rAF is only one part of the improvements here. It also delays the call to coordsChar() until immediately before displaying popover, so it rarely gets called anymore. Of course, the exception to this is once the popover is already showing, but that's not a common case.

[dangoor]

I haven't started looking at this, but it looks like @peterflynn has... Did you want to take this @peterflynn, or should I go through it as well?

[dangoor]

I haven't started on this and I picked it up before the new PR process. I'm going to unassign myself so that someone else is free to pick it up. I have a bunch of pull requests on my plate right now.

[JeffryBooher]

I'll take a look

[JeffryBooher]

@redmunds what do you think about changing window.webkitRequestAnimationFrame() to window.requestAnimationFrame()?

[Mark-Simulacrum]

@JeffryBooher According to this MDN article, perhaps keeping the webkit and moz prefixes in is a good idea for future in-browser Brackets development? Or does the Brackets project not aim to support old/out-of-date browsers (when the project starts supporting in-browser editing)?

[JeffryBooher]

@Mark-Simulacrum we wouldn't support legacy browsers.

[Mark-Simulacrum]

@JeffryBooher I see. Well, my opinion would be to drop the vendor prefixing then, as it seems unneeded.

[redmunds]

FYI, there is work going on to create an in-browser version of Brackets, so we shouldn't be in a hurry to drop any existing support for vendor prefixes.

[redmunds]

@JeffryBooher

what do you think about changing window.webkitRequestAnimationFrame() to window.requestAnimationFrame() ?

I already did this for the CubicBezierEditor, so I guess that cat is already out of the bag :) Done.

[peterflynn]

@redmunds Re your comment on June 9 -- Ok, so I think it's basically already doing that suggestion then. Basically the degenerate case of it, where we tolerate zero pixels of mouse movement before restarting the timer. As long as that doesn't feel too hard to keep the mouse steady enough, it seems plenty fine to me!

[peterflynn]

I think we can ditch this lastPos stuff now: it's basically redundant with the posWithinRange() check below, and this codepath (mousemove while popup already open) doesn't need to care about efficiency that much. (It's probably barely faster than the checks posWithinRange() does, so this early exit might not gain much/any perf anyway).

With this removed, the coordsChar() result can just be a local var in showPreview() that's used to sort out which pos/token to hand the providers.

[redmunds]

Removed lastPos. Good catch.

[peterflynn]

@redmunds Not sure if @JeffryBooher still wants to give this a look, but it looks pretty solid from my point of view.

[JeffryBooher]

I'm good ... Merge away

[redmunds]

@peterflynn Changes pushed. Ready for another review.

[peterflynn]

@redmunds Found one minor issue, a variant of #5102:

1. Place a color, etc. as the very last thing on a line (no ';' or other chars after it)
2. Mouse over it so you get a popover
3. Move the mouse to the right, past the end of the line

Result: popover stays visible even though mouse is now far away from the popover's highlighted text

So I think this section needs a copy of that "if mouse is past last char on line" check from above too. Should probably go inside this if since it's the less common of the cases?

[redmunds]

@peterflynn Good catch. Fixed.

[peterflynn]

Looks good to go otherwise, though!

[redmunds]

Changes pushed. Ready for re-re-review.

[peterflynn]

Looks great! Thanks.