[webkit-reviews] review granted: [Bug 100884] [chromium] Use embedder-supported gesture curves : [Attachment 174533] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Nov 15 16:28:55 PST 2012


James Robinson <jamesr at chromium.org> has granted Robert Kroeger
<rjkroege at chromium.org>'s request for review:
Bug 100884: [chromium] Use embedder-supported gesture curves
https://bugs.webkit.org/show_bug.cgi?id=100884

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

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


R=me

> Source/WebKit/chromium/src/WebViewImpl.cpp:698
>	   // FIXME: Make the curve parametrizable from the browser.
> -	   OwnPtr<PlatformGestureCurve> flingCurve =
PlatformGestureCurveFactory::get()->createCurve(event.data.flingStart.sourceDev
ice, FloatPoint(event.data.flingStart.velocityX,
event.data.flingStart.velocityY));
> -	   m_gestureAnimation =
ActivePlatformGestureAnimation::create(flingCurve.release(), this);
> +	   OwnPtr<WebGestureCurve> flingCurve =
adoptPtr(Platform::current()->createFlingAnimationCurve(event.data.flingStart.s
ourceDevice, WebFloatPoint(event.data.flingStart.velocityX,
event.data.flingStart.velocityY), WebSize()));

I think you can remove the FIXME here now, right?

>> Source/WebKit/chromium/src/WebViewImpl.h:123
>> +		      , public WebGestureCurveTarget
> 
> Weird number of spaces at line-start.  Are you using a 4-space indent? 
[whitespace/indent] [3]

I think it's being a bit silly here, although this method of listing base
classes is a bit unorthodox.


More information about the webkit-reviews mailing list