[Webkit-unassigned] [Bug 49345] [Chromium] Add a basic GestureManager to Chromium

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=49345





--- Comment #9 from Robert Kroeger <rjkroege at chromium.org>  2010-12-10 15:49:22 PST ---
(In reply to comment #6)
> (From update of attachment 75402 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=75402&action=review
> 
> My review comments are mostly just style issues.  I don't know too much about touch, so I'm not able to review the substance of your change.  We'll want to find someone who's familiar with this kind of thing to do a review of the substance.

Sure. Sounds good. I am (at least in theory) one of the "touch people" for Chrome but can stand to learn a lot about how touch works in WebKit.

> 
> > WebCore/page/GestureManager.h:68
> > +    void addEdgeFunction(State,
> > +                         unsigned,
> > +                         PlatformTouchPoint::State,
> > +                         bool,
> > +                         GestureTransitionFunction);
> > +  
> 
> Generally, we omit formal parameter names in header files if the meaning is obvious.  For example "Frame* frame" is kind of redundant. However, here, it looks like you've gone a bit overboard.  What does the second parameter mean?  Also, the comment seems to refer to parameter names that are no longer present.  (Also, these declarations should be on one line.)


I cleaned this up.

> 
> > WebCore/page/GestureManager.h:95
> > +    inline void setState(State s)
> > +    {
> > +      m_state = s;
> > +    }
> 
> No need to declare these functions inline.  Also, you can just write the whole function on one line.  (You've also got a bad indent here, but that will get fixed when you put the whole thing on one line.)  The general rule is that simple setter/getter functions should take up as little space as possible.

done.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.