add hooks in ScrollStepStrategy: beginScrolls and endScrolls

A possible usage is the implementations can use the hooks to set/reset internal data

Change-Id: I882faf2a6f35098ef6742d9a61a9351533ab9cf9

5 files changed, 99 insertions, 16 deletions

diff --git a/src/com/google/android/droiddriver/scroll/AccessibilityEventScrollStepStrategy.java b/src/com/google/android/droiddriver/scroll/AccessibilityEventScrollStepStrategy.java
index 9a4fe05..98860e5 100644
--- a/src/com/google/android/droiddriver/scroll/AccessibilityEventScrollStepStrategy.java
+++ b/src/com/google/android/droiddriver/scroll/AccessibilityEventScrollStepStrategy.java
@@ -44,6 +44,29 @@ import java.util.concurrent.TimeoutException;
  * </p>
  */
 public class AccessibilityEventScrollStepStrategy implements ScrollStepStrategy {
+  /**
+   * Stores the data if we reached end at the last
+   * {@link AccessibilityEventScrollStepStrategy#scroll}. If the data match when
+   * a new scroll is requested, we can return immediately.
+   */
+  private static class EndData {
+    private Finder containerFinderAtEnd;
+    private PhysicalDirection directionAtEnd;
+
+    public boolean match(Finder containerFinder, PhysicalDirection direction) {
+      return containerFinderAtEnd == containerFinder && directionAtEnd == direction;
+    }
+
+    public void set(Finder containerFinder, PhysicalDirection direction) {
+      containerFinderAtEnd = containerFinder;
+      directionAtEnd = direction;
+    }
+
+    public void reset() {
+      set(null, null);
+    }
+  }
+
   private static final AccessibilityEventFilter SCROLL_EVENT_FILTER =
       new AccessibilityEventFilter() {
         @Override
@@ -55,9 +78,7 @@ public class AccessibilityEventScrollStepStrategy implements ScrollStepStrategy
   private final UiAutomation uiAutomation;
   private final long scrollEventTimeoutMillis;
   private final DirectionConverter directionConverter;
-
-  private Finder containerFinderAtEnd;
-  private PhysicalDirection directionAtEnd;
+  private final EndData atEndData = new EndData();
 
   public AccessibilityEventScrollStepStrategy(UiAutomation uiAutomation,
       long scrollEventTimeoutMillis, DirectionConverter converter) {
@@ -70,7 +91,7 @@ public class AccessibilityEventScrollStepStrategy implements ScrollStepStrategy
   public boolean scroll(DroidDriver driver, Finder containerFinder,
       final PhysicalDirection direction) {
     // Check if we've reached end after last scroll.
-    if (containerFinderAtEnd == containerFinder && directionAtEnd == direction) {
+    if (atEndData.match(containerFinder, direction)) {
       return false;
     }
 
@@ -84,12 +105,8 @@ public class AccessibilityEventScrollStepStrategy implements ScrollStepStrategy
       }, SCROLL_EVENT_FILTER, scrollEventTimeoutMillis);
 
       if (detectEnd(direction.axis(), event)) {
-        containerFinderAtEnd = containerFinder;
-        directionAtEnd = direction;
+        atEndData.set(containerFinder, direction);
         Logs.log(Log.DEBUG, "reached scroll end");
-      } else {
-        containerFinderAtEnd = null;
-        directionAtEnd = null;
       }
     } catch (TimeoutException e) {
       // If no TYPE_VIEW_SCROLLED event, no more scrolling is possible
@@ -125,4 +142,12 @@ public class AccessibilityEventScrollStepStrategy implements ScrollStepStrategy
     return String.format("AccessibilityEventScrollStepStrategy{scrollEventTimeoutMillis=%d}",
         scrollEventTimeoutMillis);
   }
+
+  @Override
+  public void beginScrolling(Finder containerFinder, Finder itemFinder, PhysicalDirection direction) {
+    atEndData.reset();
+  }
+
+  @Override
+  public void endScrolling(Finder containerFinder, Finder itemFinder, PhysicalDirection direction) {}
 }
diff --git a/src/com/google/android/droiddriver/scroll/BaseSentinelStrategy.java b/src/com/google/android/droiddriver/scroll/BaseSentinelStrategy.java
index 709a068..35a1636 100644
--- a/src/com/google/android/droiddriver/scroll/BaseSentinelStrategy.java
+++ b/src/com/google/android/droiddriver/scroll/BaseSentinelStrategy.java
@@ -90,6 +90,12 @@ public abstract class BaseSentinelStrategy implements SentinelStrategy {
   }
 
   @Override
+  public void beginScrolling(Finder containerFinder, Finder itemFinder, PhysicalDirection direction) {}
+
+  @Override
+  public void endScrolling(Finder containerFinder, Finder itemFinder, PhysicalDirection direction) {}
+
+  @Override
   public String toString() {
     return String.format("{backwardGetter=%s, forwardGetter=%s}", backwardGetter, forwardGetter);
   }
diff --git a/src/com/google/android/droiddriver/scroll/DynamicSentinelStrategy.java b/src/com/google/android/droiddriver/scroll/DynamicSentinelStrategy.java
index bf8c6d3..7019168 100644
--- a/src/com/google/android/droiddriver/scroll/DynamicSentinelStrategy.java
+++ b/src/com/google/android/droiddriver/scroll/DynamicSentinelStrategy.java
@@ -174,6 +174,7 @@ public class DynamicSentinelStrategy extends BaseSentinelStrategy {
   }
 
   private final IsUpdatedStrategy isUpdatedStrategy;
+  private UiElement lastSentinel;
 
   /**
    * Constructs with {@code Getter}s that decorate the given {@code Getter}s
@@ -207,12 +208,29 @@ public class DynamicSentinelStrategy extends BaseSentinelStrategy {
 
   @Override
   public boolean scroll(DroidDriver driver, Finder containerFinder, PhysicalDirection direction) {
-    UiElement oldSentinel = getSentinel(driver, containerFinder, direction);
+    UiElement oldSentinel = getOldSentinel(driver, containerFinder, direction);
     oldSentinel.getParent().scroll(direction);
     UiElement newSentinel = getSentinel(driver, containerFinder, direction);
+    lastSentinel = newSentinel;
     return isUpdatedStrategy.isSentinelUpdated(newSentinel, oldSentinel);
   }
 
+  private UiElement getOldSentinel(DroidDriver driver, Finder containerFinder,
+      PhysicalDirection direction) {
+    return lastSentinel != null ? lastSentinel : getSentinel(driver, containerFinder, direction);
+  }
+
+  @Override
+  public void beginScrolling(Finder containerFinder, Finder itemFinder, PhysicalDirection direction) {
+    lastSentinel = null;
+  }
+
+  @Override
+  public void endScrolling(Finder containerFinder, Finder itemFinder, PhysicalDirection direction) {
+    // Prevent memory leak
+    lastSentinel = null;
+  }
+
   @Override
   public String toString() {
     return String.format("DynamicSentinelStrategy{%s, isUpdatedStrategy=%s}", super.toString(),
diff --git a/src/com/google/android/droiddriver/scroll/ScrollStepStrategy.java b/src/com/google/android/droiddriver/scroll/ScrollStepStrategy.java
index 5bdeb83..96d298f 100644
--- a/src/com/google/android/droiddriver/scroll/ScrollStepStrategy.java
+++ b/src/com/google/android/droiddriver/scroll/ScrollStepStrategy.java
@@ -42,6 +42,30 @@ public interface ScrollStepStrategy {
   DirectionConverter getDirectionConverter();
 
   /**
+   * Called only if this step is at the beginning of a series of scroll steps
+   * with regard to the given arguments.
+   *
+   * @param containerFinder Finder for the container that can scroll, for
+   *        instance a ListView
+   * @param itemFinder Finder for the desired item; relative to
+   *        {@code containerFinder}
+   * @param direction
+   */
+  void beginScrolling(Finder containerFinder, Finder itemFinder, PhysicalDirection direction);
+
+  /**
+   * Called only if this step is at the end of a series of scroll steps with
+   * regard to the given arguments.
+   *
+   * @param containerFinder Finder for the container that can scroll, for
+   *        instance a ListView
+   * @param itemFinder Finder for the desired item; relative to
+   *        {@code containerFinder}
+   * @param direction
+   */
+  void endScrolling(Finder containerFinder, Finder itemFinder, PhysicalDirection direction);
+
+  /**
    * {@inheritDoc}
    *
    * <p>
diff --git a/src/com/google/android/droiddriver/scroll/StepBasedScroller.java b/src/com/google/android/droiddriver/scroll/StepBasedScroller.java
index d7fd368..d2852af 100644
--- a/src/com/google/android/droiddriver/scroll/StepBasedScroller.java
+++ b/src/com/google/android/droiddriver/scroll/StepBasedScroller.java
@@ -124,7 +124,12 @@ public class StepBasedScroller implements Scroller {
   @Override
   public UiElement scrollTo(DroidDriver driver, Finder containerFinder, Finder itemFinder,
       PhysicalDirection direction) {
-    return scrollTo(driver, containerFinder, itemFinder, direction, false);
+    try {
+      scrollStepStrategy.beginScrolling(containerFinder, itemFinder, direction);
+      return scrollTo(driver, containerFinder, itemFinder, direction, false);
+    } finally {
+      scrollStepStrategy.endScrolling(containerFinder, itemFinder, direction);
+    }
   }
 
   @Override
@@ -150,18 +155,23 @@ public class StepBasedScroller implements Scroller {
       } else {
         // Fling to beginning is not reliable; scroll to beginning
         // container.perform(SwipeAction.toFling(backwardDirection));
-        for (int i = 0; i < maxScrolls; i++) {
-          if (!scrollStepStrategy.scroll(driver, containerFinder, backwardDirection)) {
-            break;
+        try {
+          scrollStepStrategy.beginScrolling(containerFinder, itemFinder, backwardDirection);
+          for (int i = 0; i < maxScrolls; i++) {
+            if (!scrollStepStrategy.scroll(driver, containerFinder, backwardDirection)) {
+              break;
+            }
           }
+        } finally {
+          scrollStepStrategy.endScrolling(containerFinder, itemFinder, backwardDirection);
         }
       }
     } else {
-      // search backward
+      // search backward first
       try {
         return scrollTo(driver, containerFinder, itemFinder, backwardDirection, true);
       } catch (ElementNotFoundException e) {
-        // try another direction
+        // fall through to search forward
       }
     }