Refactoring to make adding ink drop animations easier

ui/events/scoped_target_handler.cc

Index: ui/events/scoped_target_handler.cc
diff --git a/ui/events/scoped_target_handler.cc b/ui/events/scoped_target_handler.cc
new file mode 100644
index 0000000000000000000000000000000000000000..0e9db019edeeba1151541160698b6acefe9b8f28
--- /dev/null
+++ b/ui/events/scoped_target_handler.cc
@@ -0,0 +1,53 @@
+// 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_target.h"
+
+namespace ui {
+
+ScopedTargetHandler::ScopedTargetHandler(EventTarget* target)
+    : target_(target) {
+  original_handler_ = target_->SetTargetHandler(this);
+}
+
+ScopedTargetHandler::~ScopedTargetHandler() {
+  EventHandler* handler = target_->SetTargetHandler(original_handler_);
+  DCHECK_EQ(this, handler);
+}
+
+void ScopedTargetHandler::OnEvent(ui::Event* event) {
+  // Dispatch to |target_| first and then to |original_handler_|.
+  EventHandler::OnEvent(event);

[bruthig, 2015/11/20 16:25:33]

Don't we want the ToolbarActionView to handle the event before the
ToolbarInkDropDelegate?

[varkha, 2015/11/20 17:48:26]

This is what happens with this code (inside EventHandler::OnEvent).
The stack when ToolbarActionView gets the event is this:
View::OnGestureEvent()
CustomButton::OnGestureEvent()
MenuButton::OnGestureEvent()
ToolbarActionView::OnGestureEvent() <- is set as a |target_|
ScopedTargetHandler::OnGestureEvent
** ToolbarInkDropDelegate::OnGestureEvent()
EventHandler::OnEvent()
ScopedTargetHandler::OnEvent() <- in an instance of ToolbarInkDropDelegate

After ScopedTargetHandler::OnGestureEvent() returns in ** above
ToolbarInkDropDelegate gets to handle the ink drop animations.

[bruthig, 2015/11/20 18:56:27]

Ah yes you're right, man that is hard to follow.
If we made the ScopedTargetHandler take in the EventHandler as well then it
would enforce all InkDropDelegates to call the original event handlers first. 
Maybe a good thing? We would just have to make sure that plays nicely with the
destruction detection.

[varkha, 2015/11/21 01:07:37]

See if this way (using "has" instead of "is") makes this chaining more
understandable.

+  if (original_handler_)
+    original_handler_->OnEvent(event);
+}
+
+void ScopedTargetHandler::OnKeyEvent(ui::KeyEvent* event) {
+  static_cast<EventHandler*>(target_)->OnKeyEvent(event);

[bruthig, 2015/11/20 16:25:33]

Are these casts necessary?  EventTarget extends EventHandler.

[varkha, 2015/11/20 17:48:26]

Yes they are even though yes it does. This is because the calls are on |target_|
(which is the View) and not on |this| (which is an instance of InkDropDelegate).

[bruthig, 2015/11/20 18:56:27]

I guess this begs the question, should the On*Event() methods really be
protected in EventTarget?  They are public in EventHandler AND in View.

[varkha, 2015/11/21 01:07:37]

I think this prevents _random_ code from doing something like aura::Window*
window = GetSomeWindow();
window->OnMouseEvent();
In our case this is not a random code but generic event dispatching code so I
hope this is OK.

+}
+
+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