Refactoring to make adding ink drop animations easier

ui/events/scoped_target_handler.cc

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ui/events/scoped_target_handler.h"
+
+#include "ui/events/event.h"
+#include "ui/events/event_handler.h"
+#include "ui/events/event_target.h"
+
+namespace ui {
+
+ScopedTargetHandler::ScopedTargetHandler(EventTarget* target,
+                                         EventHandler* handler)
+    : destroyed_flag_(NULL), target_(target), new_handler_(handler){
+  original_handler_ = target_->SetTargetHandler(this);
+}
+
+ScopedTargetHandler::~ScopedTargetHandler() {
+  EventHandler* handler = target_->SetTargetHandler(original_handler_);
+  DCHECK_EQ(this, handler);
+  if (destroyed_flag_)
+    *destroyed_flag_ = true;
+}
+
+void ScopedTargetHandler::OnEvent(ui::Event* event) {
+  bool destroyed = false;
+  destroyed_flag_ = &destroyed;
+
+  if (original_handler_)
+    original_handler_->OnEvent(event);
+  else
+    EventHandler::OnEvent(event);
+
+  if (destroyed)
+    return;
+  destroyed_flag_ = NULL;
+
+  new_handler_->OnEvent(event);
+}

[sadrul, 2015/11/25 22:38:58]

I think this should just be:

  if (original_handler_)
    original_handler_->OnEvent(event);
  if (!destroyed)
    new_handler_->OnEvent(event);

And the various On*Event() overrides below should be NOTREACHED()

[varkha, 2015/11/26 00:05:46]

I don't think so. |original_handler_| here is a delegate that may be set on the
|target_| (which is our View) with SetTargetHandler(). First we need to call
that delegate _or_ call the concrete View event handlers (which the
EventHandler::OnEvent does for us). Only after that we need to call the
InkDropDelegate which here is represented by |new_handler|.
Calling only new_handler_->OnEvent() will result in a call into say,
ButtonInkDropDelegate::OnGestureEvent() but will not call the original View's
OnGestureEvent().

[sadrul, 2015/11/26 17:48:19]

Ah, I see what you mean. But sending event directly to the EventTarget from the
On*Event() callbacks can send the event back to this ScopedTargetHandler,
because it is the |target_handler_| for the target (since this is a generic and
deals with EventTarget, we can't assume that it overrides all the On*Event()
methods like View), right? And we should avoid that. Can this be more like this:

  if (original_handler_)
    original_handler_->OnEvent(event);
  else
    target_->OnEvent(event);
  new_handler_->OnEvent(event);

and the various On*Event() overrides in ScopedTargetHandler() are no-ops, where
they don't do anything (i.e. don't need to override them at all).

Can there be a test for this ScopedTargetHandler?

[varkha, 2015/11/26 22:12:40]

As we talked offline - passing in a View instead of TargetHandler should avoid
the infinite loop in case we are sending in an EventTarget that is not a View.
Using target_->OnEvent(event) would not work because it will trigger that
infinite recursion right there calling into  ScopedTargetHandler::OnEvent again
and so on.

+
+void ScopedTargetHandler::OnKeyEvent(ui::KeyEvent* event) {
+  static_cast<EventHandler*>(target_)->OnKeyEvent(event);
+}
+
+void ScopedTargetHandler::OnMouseEvent(ui::MouseEvent* event) {
+  static_cast<EventHandler*>(target_)->OnMouseEvent(event);
+}
+
+void ScopedTargetHandler::OnScrollEvent(ui::ScrollEvent* event) {
+  static_cast<EventHandler*>(target_)->OnScrollEvent(event);
+}
+
+void ScopedTargetHandler::OnTouchEvent(ui::TouchEvent* event) {
+  static_cast<EventHandler*>(target_)->OnTouchEvent(event);
+}
+
+void ScopedTargetHandler::OnGestureEvent(ui::GestureEvent* event) {
+  static_cast<EventHandler*>(target_)->OnGestureEvent(event);
+}
+
+void ScopedTargetHandler::OnCancelMode(ui::CancelModeEvent* event) {
+  static_cast<EventHandler*>(target_)->OnCancelMode(event);
+}
+
+}  // namespace ui