Fix child view ordering for accessibility.

When reporting views to accessibility services we are ordering the
children in a parent based on their location on the screen. The
initial implementation worked pretty well in practive but violated
the transitivity property leading to rare crashes in apps. The current
implementation does not violate transitivity but does not produce
good ordering.

Given the lack of time and to minumize risk this change uses the
old strategy which works most of the time and if that fails we
fall back to the current strategy. Coming up with a correct strategy
that produces good results requires more time.

bug:17887986

Change-Id: I1c233ecdf318befc315e793696ac48bd6c652ab6

1 files changed, 40 insertions, 1 deletions

diff --git a/core/java/android/view/ViewGroup.java b/core/java/android/view/ViewGroup.java
index b586caab0780..4116b6b5b546 100644
--- a/core/java/android/view/ViewGroup.java
+++ b/core/java/android/view/ViewGroup.java
@@ -7106,7 +7106,7 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager
                     ViewLocationHolder holder = ViewLocationHolder.obtain(parent, child);
                     holders.add(holder);
                 }
-                Collections.sort(holders);
+                sort(holders);
                 for (int i = 0; i < childCount; i++) {
                     ViewLocationHolder holder = holders.get(i);
                     children.set(i, holder.mView);
@@ -7116,6 +7116,23 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager
             }
         }
 
+        private void sort(ArrayList<ViewLocationHolder> holders) {
+            // This is gross but the least risky solution. The current comparison
+            // strategy breaks transitivity but produces very good results. Coming
+            // up with a new strategy requires time which we do not have, so ...
+            try {
+                ViewLocationHolder.setComparisonStrategy(
+                        ViewLocationHolder.COMPARISON_STRATEGY_STRIPE);
+                Collections.sort(holders);
+            } catch (IllegalArgumentException iae) {
+                // Note that in practice this occurs extremely rarely in a couple
+                // of pathological cases.
+                ViewLocationHolder.setComparisonStrategy(
+                        ViewLocationHolder.COMPARISON_STRATEGY_LOCATION);
+                Collections.sort(holders);
+            }
+        }
+
         private void clear() {
             mChildren.clear();
         }
@@ -7134,6 +7151,12 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager
         private static final SynchronizedPool<ViewLocationHolder> sPool =
                 new SynchronizedPool<ViewLocationHolder>(MAX_POOL_SIZE);
 
+        public static final int COMPARISON_STRATEGY_STRIPE = 1;
+
+        public static final int COMPARISON_STRATEGY_LOCATION = 2;
+
+        private static int sComparisonStrategy = COMPARISON_STRATEGY_STRIPE;
+
         private final Rect mLocation = new Rect();
 
         public View mView;
@@ -7149,6 +7172,10 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager
             return holder;
         }
 
+        public static void setComparisonStrategy(int strategy) {
+            sComparisonStrategy = strategy;
+        }
+
         public void recycle() {
             clear();
             sPool.release(this);
@@ -7160,6 +7187,18 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager
             if (another == null) {
                 return 1;
             }
+
+            if (sComparisonStrategy == COMPARISON_STRATEGY_STRIPE) {
+                // First is above second.
+                if (mLocation.bottom - another.mLocation.top <= 0) {
+                    return -1;
+                }
+                // First is below second.
+                if (mLocation.top - another.mLocation.bottom >= 0) {
+                    return 1;
+                }
+            }
+
             // We are ordering left-to-right, top-to-bottom.
             if (mLayoutDirection == LAYOUT_DIRECTION_LTR) {
                 final int leftDifference = mLocation.left - another.mLocation.left;