Addressing sadruls comments from the previous CL https://codereview.chromium.org/22865036/ which was

ui/aura/root_window_unittest.cc

 // Copyright (c) 2012 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/aura/root_window.h"
 
 #include <vector>
 
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/aura/client/event_client.h"
@@ skipping 890 matching lines @@
       ui::EventTimeForNow(),
       details,
       0);
   root_window()->RepostEvent(event);
   RunAllPendingInMessageLoop();
   EXPECT_TRUE(EventTypesToString(filter->events()).find("GESTURE_TAP_DOWN") !=
               std::string::npos);
   filter->events().clear();
 }
 
+// This class inherits from the EventFilterRecorder class which provides a
+// facility to record events. This class additionally provides a facility to
+// repost the ET_GESTURE_TAP_DOWN gesture to the target window and records
+// events after that.
+class RepostGestureEventRecorder : public EventFilterRecorder {
+ public:
+  explicit RepostGestureEventRecorder(aura::Window* repost_source,

[sadrul, 2013/08/26 19:52:50]

remove 'explicit'

[ananta, 2013/08/26 21:23:16]

Done.

+                                      aura::Window* repost_target)
+      : repost_source_(repost_source),
+        repost_target_(repost_target),
+        reposted_(false) {}
+
+  virtual void OnGestureEvent(ui::GestureEvent* event) OVERRIDE {
+    if (event->type() == ui::ET_GESTURE_TAP_DOWN) {
+      if (!reposted_) {
+        EXPECT_NE(repost_target_, event->target());
+        reposted_ = true;
+        EXPECT_EQ(static_cast<aura::Window*>(event->target()), repost_source_);
+        repost_target_->GetRootWindow()->RepostEvent(*event);
+        // Ensure that the reposted gesture event above goes to the
+        // repost_target_;
+        repost_source_->GetRootWindow()->RemoveChild(repost_source_);
+        return;
+      } else {
+        EXPECT_EQ(static_cast<aura::Window*>(event->target()), repost_target_);

[sadrul, 2013/08/26 19:52:50]

This should be outside of the == TAP_DOWN check in line 924. i.e.

  virtual void ... {
    EXPECT_EQ(reposted_ ? repost_target_ : repost_source_, event->target());

    if (event->type() == TAP_DOWN) {
      .. repost
    }

    EFR::OnGestureEvent(event);
  }

[ananta, 2013/08/26 21:23:16]

Done.

+      }
+    }
+    EventFilterRecorder::OnGestureEvent(event);
+  }
+
+ private:
+  aura::Window* repost_source_;
+  aura::Window* repost_target_;
+  // set to true if we reposted the ET_GESTURE_TAP_DOWN event.
+  bool reposted_;
+  DISALLOW_COPY_AND_ASSIGN(RepostGestureEventRecorder);
+};
+
+// Tests whether events which are generated after the reposted gesture event
+// are received after that. In this case the scroll sequence events should
+// be received after the reposted gesture event.
+TEST_F(RootWindowTest, GestureRepostEventOrder) {
+  // We create two windows.
+  // The first window (repost_source) is the one to which the initial tap
+  // gesture is sent. It reposts this event to the second window
+  // (repost_target).
+  // We then generate the scroll sequence for repost_target and look for two
+  // ET_GESTURE_TAP_DOWN events in the event list at the end.
+  test::TestWindowDelegate delegate;
+  scoped_ptr<aura::Window> repost_target(CreateTestWindowWithDelegate(
+      &delegate, 1, gfx::Rect(0, 0, 100, 100), root_window()));
+
+  scoped_ptr<aura::Window> repost_source(CreateTestWindowWithDelegate(
+      &delegate, 1, gfx::Rect(0, 0, 50, 50), root_window()));
+
+  RepostGestureEventRecorder* repost_event_recorder =
+      new RepostGestureEventRecorder(repost_source.get(), repost_target.get());
+  root_window()->SetEventFilter(repost_event_recorder);  // passes ownership
+
+  // Generate a tap down gesture for the repost_source. This will be reposted
+  // to repost_target.
+  test::EventGenerator repost_generator(root_window(), repost_source.get());
+  repost_generator.GestureTapAt(gfx::Point(40, 40));
+  RunAllPendingInMessageLoop();
+
+  test::EventGenerator scroll_generator(root_window(), repost_target.get());
+  scroll_generator.GestureScrollSequence(
+      gfx::Point(80, 80),
+      gfx::Point(100, 100),
+      base::TimeDelta::FromMilliseconds(100),
+      3);
+  RunAllPendingInMessageLoop();
+
+  std::string event_string = EventTypesToString(
+      repost_event_recorder->events());
+  DLOG(INFO) << "Event types: " << event_string;

[sadrul, 2013/08/26 19:52:50]

We shouldn't add this in the log.

[ananta, 2013/08/26 21:23:16]

Done.

+
+  int tap_down_count = 0;
+  for (size_t i = 0; i < repost_event_recorder->events().size(); ++i) {
+    if (repost_event_recorder->events()[i] == ui::ET_GESTURE_TAP_DOWN)
+      ++tap_down_count;
+  }
+
+  // We expect two tap down events. One from the repost and the other one from
+  // the scroll sequence posted above.
+  EXPECT_EQ(tap_down_count, 2);

[sadrul, 2013/08/26 19:52:50]

Why did we switch from checking the sequence of events in an earlier patch set
to counting just the number of tap-downs? Checking the sequence of events would
be better. (also, if you change the test-expectations between patchsets, please
add a comment about it, preferably explaining the reason for the change).

Changing the target of gestures in the middle of a gesture-sequence can be
problematic (e.g. when doing captures, etc.). So I want to make sure when we
repost an event, with the specific intention of changing the target of the
gesture in the process, the event-targetting of all the subsequent
touch/gesture-events in the same gesture-sequence is done correctly. Simply
counting the number of tap-downs, or checking the target of tap-down gesture
does not accomplish that.

[ananta, 2013/08/26 21:23:16]

I removed that with a comment in the gcl upload indicating that I did that
because it looked more readable. Sadly the comment did not make it through. 

The event list check has been restored.

PTAL

+}
+
 }  // namespace aura