[webkit-reviews] review denied: [Bug 103465] [chromium] Add latency calculation plumbing : [Attachment 179800] Patch

[bugzilla-daemon at webkit.org]

James Robinson <jamesr at chromium.org> has denied John Bauman
<jbauman at chromium.org>'s request for review:
Bug 103465: [chromium] Add latency calculation plumbing
https://bugs.webkit.org/show_bug.cgi?id=103465

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

------- Additional Comments from James Robinson <jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=179800&action=review


> Source/Platform/chromium/public/WebGraphicsContext3D.h:151
> +    class WebGraphicsLatencyInfoCallbackCHROMIUM {

As I mentioned on the chromium side, I think having this setup on the context
itself is going to cause lots of dependency headaches.	Can you route this
through something surface-aware?  Maybe cc::OutputSurface?

> Source/Platform/chromium/public/WebInputHandlerClient.h:57
> +    virtual bool scrollByIfPossible(int64_t, WebPoint point, WebSize size) {
return scrollByIfPossible(point, size); }

Name the int64_t parameter, I can't figure out what it means from the signature
alone.	There could be any number of scrollBy..() calls for the same input
event (think of animations, flings, etc) so this is unclear.

> Source/WebKit/chromium/public/WebCompositorInputHandler.h:46
> +    virtual void handleInputEvent(const WebInputEvent&, int64_t inputNumber)
= 0;

Instead of adding an additional parameter to every function that uses a
WebInputEvent, did you consider putting it in the WebInputEvent struct itself? 
Then you won't have to update all the interfaces and it will pass through
functions that are ignorant of the inputNumber system without any extra code.