touch: Return an enum from OnTouchEvent.

touch: Return an enum from OnTouchEvent.

The enum returned from OnTouchEvent reflects the current status of the touch-sequence. This can be used by the RootView to determine when the touch-event-handler should be reset.

BUG=none
TEST=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=71677

[sadrul, 2011-01-14 21:05:02]

Please add additional reviewers as you see fit.

[sadrul, 2011-01-14 21:26:37]

OWNERS files have just been added :)

[sky, 2011-01-14 21:47:20]

http://codereview.chromium.org/6347002/diff/1/chrome/browser/renderer_host/re...
File chrome/browser/renderer_host/render_widget_host_view_views.cc (right):

http://codereview.chromium.org/6347002/diff/1/chrome/browser/renderer_host/re...
chrome/browser/renderer_host/render_widget_host_view_views.cc:738: if
(touch_event_.touchPointsLength == 0) {
This pattern is much different from how we've typically done processing like
this.

Can OnTouchSequenceEnd be a real method on View (like OnTouchEvent), and have
whoever invokes OnTouchEvent be responsible for invoking OnTouchSequenceEnd?

[sadrul, 2011-01-14 22:11:33]

http://codereview.chromium.org/6347002/diff/1/chrome/browser/renderer_host/re...
File chrome/browser/renderer_host/render_widget_host_view_views.cc (right):

http://codereview.chromium.org/6347002/diff/1/chrome/browser/renderer_host/re...
chrome/browser/renderer_host/render_widget_host_view_views.cc:738: if
(touch_event_.touchPointsLength == 0) {
On 2011/01/14 21:47:20, sky wrote:
> This pattern is much different from how we've typically done processing like
> this.
> 
> Can OnTouchSequenceEnd be a real method on View (like OnTouchEvent), and have
> whoever invokes OnTouchEvent be responsible for invoking OnTouchSequenceEnd?

I think you mean that the RootView, or the AcceleratorHandler (which reads the
touch-event from X) should invoke OnTouchSequenceEnd?

Currently, the list of touch points are maintained by the
RenderWidgetHostViewViews (RWHVV). The RootView (RV) receives a views
touch-event (e.g. a touch-point press, or a touch-point move etc.) from
AcceleratorHandler (AH) and dispatches it down the views hierarchy to the RWHVV.
Upon receiving the event, the RWHVV updates the webkit touchevent it maintains,
and sends it off to the renderer. It is not possible for the RV (or AH) to
recognize the end of a touch sequence (because, e.g. a touch-point release may
not be the end of the sequence because other points remain pressed). So it is
necessary for the RWHVV to notify the RV that the touch-sequence has ended. This
allows the RV to reset the touch-event-handler, so that subsequent touch-events
can be sent to the correct view, instead of sending all touch-events to the view
that received the very first touch-event.

An alternate solution considered was for the RWHVV to send a TOUCH_END event to
the RV (using OnTouchEvent). But the reversed direction of the message (RWHVV ->
RV, instead of the usual RV -> RWHVV) felt a bit unclean.

Another alternate solution considered was to maintain the list of touch-points
in RV (or somewhere in views, e.g. a singleton TouchFactory). However, in this
approach, for each event from each views-touch-point, the RWHVV will need to
update the state for each webkit-touch-point before sending it off to the
renderer. This may have a significant effect on multi-touch performance.

Did I make sense? Please let me know if what I said needs farther
explanation/clarification.

[sky, 2011-01-14 22:18:29]

On Fri, Jan 14, 2011 at 2:11 PM,  <sadrul@chromium.org> wrote:
>
>
http://codereview.chromium.org/6347002/diff/1/chrome/browser/renderer_host/re...
> File chrome/browser/renderer_host/render_widget_host_view_views.cc
> (right):
>
>
http://codereview.chromium.org/6347002/diff/1/chrome/browser/renderer_host/re...
> chrome/browser/renderer_host/render_widget_host_view_views.cc:738: if
> (touch_event_.touchPointsLength == 0) {
> On 2011/01/14 21:47:20, sky wrote:
>>
>> This pattern is much different from how we've typically done
>
> processing like
>>
>> this.
>
>> Can OnTouchSequenceEnd be a real method on View (like OnTouchEvent),
>
> and have
>>
>> whoever invokes OnTouchEvent be responsible for invoking
>
> OnTouchSequenceEnd?
>
> I think you mean that the RootView, or the AcceleratorHandler (which
> reads the touch-event from X) should invoke OnTouchSequenceEnd?
>
> Currently, the list of touch points are maintained by the
> RenderWidgetHostViewViews (RWHVV). The RootView (RV) receives a views
> touch-event (e.g. a touch-point press, or a touch-point move etc.) from
> AcceleratorHandler (AH) and dispatches it down the views hierarchy to
> the RWHVV. Upon receiving the event, the RWHVV updates the webkit
> touchevent it maintains, and sends it off to the renderer. It is not
> possible for the RV (or AH) to recognize the end of a touch sequence
> (because, e.g. a touch-point release may not be the end of the sequence
> because other points remain pressed). So it is necessary for the RWHVV
> to notify the RV that the touch-sequence has ended. This allows the RV
> to reset the touch-event-handler, so that subsequent touch-events can be
> sent to the correct view, instead of sending all touch-events to the
> view that received the very first touch-event.
>
> An alternate solution considered was for the RWHVV to send a TOUCH_END
> event to the RV (using OnTouchEvent). But the reversed direction of the
> message (RWHVV -> RV, instead of the usual RV -> RWHVV) felt a bit
> unclean.
>
> Another alternate solution considered was to maintain the list of
> touch-points in RV (or somewhere in views, e.g. a singleton
> TouchFactory). However, in this approach, for each event from each
> views-touch-point, the RWHVV will need to update the state for each
> webkit-touch-point before sending it off to the renderer. This may have
> a significant effect on multi-touch performance.

I like this solution best. I would be inclined to push the state
(count?) into View. RootView could invoke private methods on View that
return whether it corresponds to an end and then invoke the right
method. This doesn't seem very expensive, but perhaps I'm missing
something.

  -Scott

>
> Did I make sense? Please let me know if what I said needs farther
> explanation/clarification.
>
> http://codereview.chromium.org/6347002/
>

[rjkroege, 2011-01-14 23:16:28]

On 2011/01/14 22:18:29, sky wrote:
> On Fri, Jan 14, 2011 at 2:11 PM,  <mailto:sadrul@chromium.org> wrote:
> >
> >
>
http://codereview.chromium.org/6347002/diff/1/chrome/browser/renderer_host/re...
> > File chrome/browser/renderer_host/render_widget_host_view_views.cc
> > (right):
> >
> >
>
http://codereview.chromium.org/6347002/diff/1/chrome/browser/renderer_host/re...
> > chrome/browser/renderer_host/render_widget_host_view_views.cc:738: if
> > (touch_event_.touchPointsLength == 0) {
> > On 2011/01/14 21:47:20, sky wrote:
> >>
> >> This pattern is much different from how we've typically done
> >
> > processing like
> >>
> >> this.
> >
> >> Can OnTouchSequenceEnd be a real method on View (like OnTouchEvent),
> >
> > and have
> >>
> >> whoever invokes OnTouchEvent be responsible for invoking
> >
> > OnTouchSequenceEnd?
> >
> > I think you mean that the RootView, or the AcceleratorHandler (which
> > reads the touch-event from X) should invoke OnTouchSequenceEnd?
> >
> > Currently, the list of touch points are maintained by the
> > RenderWidgetHostViewViews (RWHVV). The RootView (RV) receives a views
> > touch-event (e.g. a touch-point press, or a touch-point move etc.) from
> > AcceleratorHandler (AH) and dispatches it down the views hierarchy to
> > the RWHVV. Upon receiving the event, the RWHVV updates the webkit
> > touchevent it maintains, and sends it off to the renderer. It is not
> > possible for the RV (or AH) to recognize the end of a touch sequence
> > (because, e.g. a touch-point release may not be the end of the sequence
> > because other points remain pressed). So it is necessary for the RWHVV
> > to notify the RV that the touch-sequence has ended. This allows the RV
> > to reset the touch-event-handler, so that subsequent touch-events can be
> > sent to the correct view, instead of sending all touch-events to the
> > view that received the very first touch-event.
> >
> > An alternate solution considered was for the RWHVV to send a TOUCH_END
> > event to the RV (using OnTouchEvent). But the reversed direction of the
> > message (RWHVV -> RV, instead of the usual RV -> RWHVV) felt a bit
> > unclean.
> >
> > Another alternate solution considered was to maintain the list of
> > touch-points in RV (or somewhere in views, e.g. a singleton
> > TouchFactory). However, in this approach, for each event from each
> > views-touch-point, the RWHVV will need to update the state for each
> > webkit-touch-point before sending it off to the renderer. This may have
> > a significant effect on multi-touch performance.
> 
> I like this solution best. I would be inclined to push the state
> (count?) into View. RootView could invoke private methods on View that
> return whether it corresponds to an end and then invoke the right
> method. This doesn't seem very expensive, but perhaps I'm missing
> something.

I (and Sadrul too based on our discussion here) don't entirely understand your
comment.

First, I note in passing: the necessary state must be maintained in the RWHVV
regardless because the tracked information must be made into an array of
(WebKit) WebTouchPoints for sending to the renderer. Consequently, the RWHVV
will always be doing the necessary work to know if it the touch sequence has
completed.

I think we could add state to the RootView which holds a set of in-progress
touch ids.  But I am not thrilled about replicating information available from
the RWHVV either.

This lead me to suggesting that the RootView could be a TouchSequenceCompleted
observer and register for notifications from the RWVHH -- the observer pattern
seems to show up frequently.

Anyway: any additional guidance at this juncture would be helpful.

Rob.

> 
>   -Scott
> 
> >
> > Did I make sense? Please let me know if what I said needs farther
> > explanation/clarification.
> >
> > http://codereview.chromium.org/6347002/
> >

[sadrul, 2011-01-14 23:22:04]

On 2011/01/14 22:18:29, sky wrote:
[snip]
> >
> > Another alternate solution considered was to maintain the list of
> > touch-points in RV (or somewhere in views, e.g. a singleton
> > TouchFactory). However, in this approach, for each event from each
> > views-touch-point, the RWHVV will need to update the state for each
> > webkit-touch-point before sending it off to the renderer. This may have
> > a significant effect on multi-touch performance.
> 
> I like this solution best. I would be inclined to push the state
> (count?) into View.

This I am not sure I understand.

> RootView could invoke private methods on View that
> return whether it corresponds to an end and then invoke the right
> method. This doesn't seem very expensive, but perhaps I'm missing
> something.

This is what I understand from this: There can be a View::TouchSequenceStatus()
function that RWHVV can implement to advise whether a touch sequence just
started, a previous touch sequence is continuing, or a touch sequence just
ended. RV will call touch_pressed_handler_->TouchSequenceStatus() after
touch_pressed_handler_->OnTouchEvent(), and determine whether or not to reset
touch_pressed_handler_. In patch form (as pseudocode):


diff --git a/chrome/browser/renderer_host/render_widget_host_view_views.cc
b/chrome/browser/renderer_host/render_widget_host_view_views.cc
index 3751bf0..58dfad1 100644
--- a/chrome/browser/renderer_host/render_widget_host_view_views.cc
+++ b/chrome/browser/renderer_host/render_widget_host_view_views.cc
@@ -746,6 +746,15 @@ bool RenderWidgetHostViewViews::OnTouchEvent(const
views::TouchEvent& e) {
   return true;
 }
 
+TouchStatus RenderWidgetHostViewViews::TouchSequenceStatus() {
+  if (touch_event_.touchPointsLength == 0)
+    return TOUCH_SEQUENCE_END;
+  else if (touch_event_.touchPointsLength == 1)
+    return TOUCH_SEQUENCE_START;  // This is deliberately incorrect.
+  else
+    return TOUCH_SEQUENCE_CONTINUE;
+}
+
 // static
 RenderWidgetHostView*
     RenderWidgetHostView::GetRenderWidgetHostViewFromNativeView(
diff --git a/views/view.h b/views/view.h
index 6889a7f..1ec5aec 100644
--- a/views/view.h
+++ b/views/view.h
@@ -709,6 +709,10 @@ class View : public AcceleratorTarget {
   // This method is invoked for each touch event. Default implementation
   // does nothing. Override as needed.
   virtual bool OnTouchEvent(const TouchEvent& event);
+
+  virtual TouchStatus TouchSequenceStatus() {
+    return TOUCH_SEQUENCE_NONE;
+  }
 #endif
 
   // Set the MouseHandler for a drag session.
diff --git a/views/widget/root_view.cc b/views/widget/root_view.cc
index 2ad45ea..c9bdfb2 100644
--- a/views/widget/root_view.cc
+++ b/views/widget/root_view.cc
@@ -315,6 +315,10 @@ bool RootView::OnTouchEvent(const TouchEvent& e) {
     TouchEvent touch_event(e, this, touch_pressed_handler_);
     touch_pressed_handler_->ProcessTouchEvent(touch_event);
     gesture_manager_->ProcessTouchEventForGesture(e, this, true);
+    if (touch_pressed_handler_ &&
+        touch_pressed_handler_->TouchSequenceStatus() == TOUCH_SEQUENCE_END) {
+      touch_pressed_handler_ = NULL;
+    }
     return true;
   }
 
Did I understand this correctly?

[sky, 2011-01-14 23:29:53]

Sorry for not being clear.

It sounded like from the email there was some state as to whether the
touch operation is ongoing that could be associated with each view. I
was proposing that state be pushed to View. RWHVV, and any other view,
would update the state in response to touch events and RootView would
then know when a sequence was done. It sounds like I'm wrong though,
that the state is stored separately.

Based on my understanding I suggest making OnTouchEvent return an
enum. That way RootView knows easily knows if the sequence is done and
can do the right thing. Sadrul indicated this might work and is going
to look into it.

  -Scott

On Fri, Jan 14, 2011 at 3:16 PM,  <rjkroege@chromium.org> wrote:
> On 2011/01/14 22:18:29, sky wrote:
>>
>> On Fri, Jan 14, 2011 at 2:11 PM,  <mailto:sadrul@chromium.org> wrote:
>> >
>> >
>
>
http://codereview.chromium.org/6347002/diff/1/chrome/browser/renderer_host/re...
>>
>> > File chrome/browser/renderer_host/render_widget_host_view_views.cc
>> > (right):
>> >
>> >
>
>
http://codereview.chromium.org/6347002/diff/1/chrome/browser/renderer_host/re...
>>
>> > chrome/browser/renderer_host/render_widget_host_view_views.cc:738: if
>> > (touch_event_.touchPointsLength == 0) {
>> > On 2011/01/14 21:47:20, sky wrote:
>> >>
>> >> This pattern is much different from how we've typically done
>> >
>> > processing like
>> >>
>> >> this.
>> >
>> >> Can OnTouchSequenceEnd be a real method on View (like OnTouchEvent),
>> >
>> > and have
>> >>
>> >> whoever invokes OnTouchEvent be responsible for invoking
>> >
>> > OnTouchSequenceEnd?
>> >
>> > I think you mean that the RootView, or the AcceleratorHandler (which
>> > reads the touch-event from X) should invoke OnTouchSequenceEnd?
>> >
>> > Currently, the list of touch points are maintained by the
>> > RenderWidgetHostViewViews (RWHVV). The RootView (RV) receives a views
>> > touch-event (e.g. a touch-point press, or a touch-point move etc.) from
>> > AcceleratorHandler (AH) and dispatches it down the views hierarchy to
>> > the RWHVV. Upon receiving the event, the RWHVV updates the webkit
>> > touchevent it maintains, and sends it off to the renderer. It is not
>> > possible for the RV (or AH) to recognize the end of a touch sequence
>> > (because, e.g. a touch-point release may not be the end of the sequence
>> > because other points remain pressed). So it is necessary for the RWHVV
>> > to notify the RV that the touch-sequence has ended. This allows the RV
>> > to reset the touch-event-handler, so that subsequent touch-events can be
>> > sent to the correct view, instead of sending all touch-events to the
>> > view that received the very first touch-event.
>> >
>> > An alternate solution considered was for the RWHVV to send a TOUCH_END
>> > event to the RV (using OnTouchEvent). But the reversed direction of the
>> > message (RWHVV -> RV, instead of the usual RV -> RWHVV) felt a bit
>> > unclean.
>> >
>> > Another alternate solution considered was to maintain the list of
>> > touch-points in RV (or somewhere in views, e.g. a singleton
>> > TouchFactory). However, in this approach, for each event from each
>> > views-touch-point, the RWHVV will need to update the state for each
>> > webkit-touch-point before sending it off to the renderer. This may have
>> > a significant effect on multi-touch performance.
>
>> I like this solution best. I would be inclined to push the state
>> (count?) into View. RootView could invoke private methods on View that
>> return whether it corresponds to an end and then invoke the right
>> method. This doesn't seem very expensive, but perhaps I'm missing
>> something.
>
> I (and Sadrul too based on our discussion here) don't entirely understand
> your
> comment.
>
> First, I note in passing: the necessary state must be maintained in the
> RWHVV
> regardless because the tracked information must be made into an array of
> (WebKit) WebTouchPoints for sending to the renderer. Consequently, the RWHVV
> will always be doing the necessary work to know if it the touch sequence has
> completed.
>
> I think we could add state to the RootView which holds a set of in-progress
> touch ids.  But I am not thrilled about replicating information available
> from
> the RWHVV either.
>
> This lead me to suggesting that the RootView could be a
> TouchSequenceCompleted
> observer and register for notifications from the RWVHH -- the observer
> pattern
> seems to show up frequently.
>
> Anyway: any additional guidance at this juncture would be helpful.
>
> Rob.
>
>
>>   -Scott
>
>> >
>> > Did I make sense? Please let me know if what I said needs farther
>> > explanation/clarification.
>> >
>> > http://codereview.chromium.org/6347002/
>> >
>
>
>
> http://codereview.chromium.org/6347002/
>

[rjkroege, 2011-01-14 23:45:14]

On 2011/01/14 23:29:53, sky wrote:
> Sorry for not being clear.
> 
> It sounded like from the email there was some state as to whether the
> touch operation is ongoing that could be associated with each view. I
> was proposing that state be pushed to View. RWHVV, and any other view,
> would update the state in response to touch events and RootView would
> then know when a sequence was done. It sounds like I'm wrong though,
> that the state is stored separately.
> 
> Based on my understanding I suggest making OnTouchEvent return an
> enum. That way RootView knows easily knows if the sequence is done and
> can do the right thing. Sadrul indicated this might work and is going
> to look into it.

I like this solution. I wish I'd thought of it. :-)

Thanks.

Rob.

> 
>   -Scott
> 
> On Fri, Jan 14, 2011 at 3:16 PM,  <mailto:rjkroege@chromium.org> wrote:
> > On 2011/01/14 22:18:29, sky wrote:
> >>
> >> On Fri, Jan 14, 2011 at 2:11 PM, &nbsp;<mailto:sadrul@chromium.org> wrote:
> >> >
> >> >
> >
> >
>
http://codereview.chromium.org/6347002/diff/1/chrome/browser/renderer_host/re...
> >>
> >> > File chrome/browser/renderer_host/render_widget_host_view_views.cc
> >> > (right):
> >> >
> >> >
> >
> >
>
http://codereview.chromium.org/6347002/diff/1/chrome/browser/renderer_host/re...
> >>
> >> > chrome/browser/renderer_host/render_widget_host_view_views.cc:738: if
> >> > (touch_event_.touchPointsLength == 0) {
> >> > On 2011/01/14 21:47:20, sky wrote:
> >> >>
> >> >> This pattern is much different from how we've typically done
> >> >
> >> > processing like
> >> >>
> >> >> this.
> >> >
> >> >> Can OnTouchSequenceEnd be a real method on View (like OnTouchEvent),
> >> >
> >> > and have
> >> >>
> >> >> whoever invokes OnTouchEvent be responsible for invoking
> >> >
> >> > OnTouchSequenceEnd?
> >> >
> >> > I think you mean that the RootView, or the AcceleratorHandler (which
> >> > reads the touch-event from X) should invoke OnTouchSequenceEnd?
> >> >
> >> > Currently, the list of touch points are maintained by the
> >> > RenderWidgetHostViewViews (RWHVV). The RootView (RV) receives a views
> >> > touch-event (e.g. a touch-point press, or a touch-point move etc.) from
> >> > AcceleratorHandler (AH) and dispatches it down the views hierarchy to
> >> > the RWHVV. Upon receiving the event, the RWHVV updates the webkit
> >> > touchevent it maintains, and sends it off to the renderer. It is not
> >> > possible for the RV (or AH) to recognize the end of a touch sequence
> >> > (because, e.g. a touch-point release may not be the end of the sequence
> >> > because other points remain pressed). So it is necessary for the RWHVV
> >> > to notify the RV that the touch-sequence has ended. This allows the RV
> >> > to reset the touch-event-handler, so that subsequent touch-events can be
> >> > sent to the correct view, instead of sending all touch-events to the
> >> > view that received the very first touch-event.
> >> >
> >> > An alternate solution considered was for the RWHVV to send a TOUCH_END
> >> > event to the RV (using OnTouchEvent). But the reversed direction of the
> >> > message (RWHVV -> RV, instead of the usual RV -> RWHVV) felt a bit
> >> > unclean.
> >> >
> >> > Another alternate solution considered was to maintain the list of
> >> > touch-points in RV (or somewhere in views, e.g. a singleton
> >> > TouchFactory). However, in this approach, for each event from each
> >> > views-touch-point, the RWHVV will need to update the state for each
> >> > webkit-touch-point before sending it off to the renderer. This may have
> >> > a significant effect on multi-touch performance.
> >
> >> I like this solution best. I would be inclined to push the state
> >> (count?) into View. RootView could invoke private methods on View that
> >> return whether it corresponds to an end and then invoke the right
> >> method. This doesn't seem very expensive, but perhaps I'm missing
> >> something.
> >
> > I (and Sadrul too based on our discussion here) don't entirely understand
> > your
> > comment.
> >
> > First, I note in passing: the necessary state must be maintained in the
> > RWHVV
> > regardless because the tracked information must be made into an array of
> > (WebKit) WebTouchPoints for sending to the renderer. Consequently, the RWHVV
> > will always be doing the necessary work to know if it the touch sequence has
> > completed.
> >
> > I think we could add state to the RootView which holds a set of in-progress
> > touch ids. &nbsp;But I am not thrilled about replicating information
available
> > from
> > the RWHVV either.
> >
> > This lead me to suggesting that the RootView could be a
> > TouchSequenceCompleted
> > observer and register for notifications from the RWVHH -- the observer
> > pattern
> > seems to show up frequently.
> >
> > Anyway: any additional guidance at this juncture would be helpful.
> >
> > Rob.
> >
> >
> >> &nbsp; -Scott
> >
> >> >
> >> > Did I make sense? Please let me know if what I said needs farther
> >> > explanation/clarification.
> >> >
> >> > http://codereview.chromium.org/6347002/
> >> >
> >
> >
> >
> > http://codereview.chromium.org/6347002/
> >

[sadrul, 2011-01-15 00:17:01]

On 2011/01/14 23:29:53, sky wrote:
> Sorry for not being clear.
> 
> It sounded like from the email there was some state as to whether the
> touch operation is ongoing that could be associated with each view. I
> was proposing that state be pushed to View. RWHVV, and any other view,
> would update the state in response to touch events and RootView would
> then know when a sequence was done. It sounds like I'm wrong though,
> that the state is stored separately.
> 
> Based on my understanding I suggest making OnTouchEvent return an
> enum. That way RootView knows easily knows if the sequence is done and
> can do the right thing. Sadrul indicated this might work and is going
> to look into it.
> 

I have made this change. Please take another look.

[rjkroege, 2011-01-17 19:27:39]

LGTM (with nits)

http://codereview.chromium.org/6347002/diff/20001/views/widget/root_view.cc
File views/widget/root_view.cc (right):

http://codereview.chromium.org/6347002/diff/20001/views/widget/root_view.cc#n...
views/widget/root_view.cc:358: gesture_manager_->ProcessTouchEventForGesture(e,
this, handled);
there should probably be a TODO (and it can be for me) to change the GM to take
view::TouchStatus instead of bool handled.

[sadrul, 2011-01-18 03:50:15]

I made a change: a View needs to return TOUCH_STATUS_START to notify the
RootView that subsequent touch-events should be delivered to it. (instead of
returning TOUCH_STATUS_END to notify that subsequent touch-events should not be
delivered to it).

http://codereview.chromium.org/6347002/diff/20001/views/widget/root_view.cc
File views/widget/root_view.cc (right):

http://codereview.chromium.org/6347002/diff/20001/views/widget/root_view.cc#n...
views/widget/root_view.cc:358: gesture_manager_->ProcessTouchEventForGesture(e,
this, handled);
On 2011/01/17 19:27:40, rjkroege wrote:
> there should probably be a TODO (and it can be for me) to change the GM to
take
> view::TouchStatus instead of bool handled.

Done.

[sky, 2011-01-18 17:16:49]

LGTM

http://codereview.chromium.org/6347002/diff/26001/views/view.cc
File views/view.cc (right):

http://codereview.chromium.org/6347002/diff/26001/views/view.cc#newcode546
views/view.cc:546: const TouchStatus status = OnTouchEvent(e);
return OnTouchEvent(e);
Also, we generally don't use const in cases like this.

http://codereview.chromium.org/6347002/diff/26001/views/view.h
File views/view.h (right):

http://codereview.chromium.org/6347002/diff/26001/views/view.h#newcode137
views/view.h:137: enum TouchStatus {
Please document what these values mean.

[sadrul, 2011-01-18 18:48:20]

http://codereview.chromium.org/6347002/diff/26001/views/view.cc
File views/view.cc (right):

http://codereview.chromium.org/6347002/diff/26001/views/view.cc#newcode546
views/view.cc:546: const TouchStatus status = OnTouchEvent(e);
On 2011/01/18 17:16:50, sky wrote:
> return OnTouchEvent(e);
> Also, we generally don't use const in cases like this.

Noted, and fixed. Thanks.

http://codereview.chromium.org/6347002/diff/26001/views/view.h
File views/view.h (right):

http://codereview.chromium.org/6347002/diff/26001/views/view.h#newcode137
views/view.h:137: enum TouchStatus {
On 2011/01/18 17:16:50, sky wrote:
> Please document what these values mean.

Done.