From e18af6e995853398b8461afdd79e77b3c2f8bd79 Mon Sep 17 00:00:00 2001
From: Robert Horvath <robhor@google.com>
Date: Thu, 5 May 2022 11:00:26 +0200
Subject: [PATCH] Fix reporting of unchanged keep clear areas

ViewRootRectTracker#computeChangedRects returns all tracked Rects if
there are changes to the Rects since the last call to that method, or
null if there aren't.
When ViewRootImpl checked for changed Rects, if only one of either set
of restricted or unrestricted keep clear rects changed,
then #computeChangedRects returned null for the other, which was
subsequently reported as an empty list. If there were keep clear rects
previously reported, this would mistakenly clear them.

This change separates the check for changes and retrieving the list of
latest Rects. If either set of Rects has changed, now it's always the
latest Rects that get reported.

Bug: 231532058
Test: Manual with app with restricted & unrestricted keep clear areas
Test: atest KeepClearRectsTests
Change-Id: Ida51420d0f0e7935a5dc9f3eab902857927c33f5
---
 core/java/android/view/ViewRootImpl.java      | 46 ++++++-------------
 .../android/view/ViewRootRectTracker.java     | 33 +++++++++++--
 2 files changed, 45 insertions(+), 34 deletions(-)

diff --git a/core/java/android/view/ViewRootImpl.java b/core/java/android/view/ViewRootImpl.java
index 48c102bd48831..75e6e92031af3 100644
--- a/core/java/android/view/ViewRootImpl.java
+++ b/core/java/android/view/ViewRootImpl.java
@@ -222,7 +222,6 @@ import java.io.StringWriter;
 import java.lang.ref.WeakReference;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Objects;
@@ -800,8 +799,7 @@ public final class ViewRootImpl implements ViewParent,
             new ViewRootRectTracker(v -> v.collectPreferKeepClearRects());
     private final ViewRootRectTracker mUnrestrictedKeepClearRectsTracker =
             new ViewRootRectTracker(v -> v.collectUnrestrictedPreferKeepClearRects());
-    private List<Rect> mPendingKeepClearAreas;
-    private List<Rect> mPendingUnrestrictedKeepClearAreas;
+    private boolean mHasPendingKeepClearAreaChange;
 
     private IAccessibilityEmbeddedConnection mAccessibilityEmbeddedConnection;
 
@@ -4819,45 +4817,31 @@ public final class ViewRootImpl implements ViewParent,
     }
 
     void keepClearRectsChanged() {
-        List<Rect> restrictedKeepClearRects = mKeepClearRectsTracker.computeChangedRects();
-        List<Rect> unrestrictedKeepClearRects =
-                mUnrestrictedKeepClearRectsTracker.computeChangedRects();
-        if ((restrictedKeepClearRects != null || unrestrictedKeepClearRects != null)
-                && mView != null) {
-            if (restrictedKeepClearRects == null) {
-                restrictedKeepClearRects = Collections.emptyList();
-            }
-            if (unrestrictedKeepClearRects == null) {
-                unrestrictedKeepClearRects = Collections.emptyList();
-            }
+        boolean restrictedKeepClearRectsChanged = mKeepClearRectsTracker.computeChanges();
+        boolean unrestrictedKeepClearRectsChanged =
+                mUnrestrictedKeepClearRectsTracker.computeChanges();
 
-            if (mHandler.hasMessages(MSG_REPORT_KEEP_CLEAR_RECTS)) {
-                // Keep clear areas have been reported recently, wait before reporting new set
-                // of keep clear areas
-                mPendingKeepClearAreas = restrictedKeepClearRects;
-                mPendingUnrestrictedKeepClearAreas = unrestrictedKeepClearRects;
-            } else {
+        if ((restrictedKeepClearRectsChanged || unrestrictedKeepClearRectsChanged)
+                && mView != null) {
+            mHasPendingKeepClearAreaChange = true;
+            // Only report keep clear areas immediately if they have not been reported recently
+            if (!mHandler.hasMessages(MSG_REPORT_KEEP_CLEAR_RECTS)) {
                 mHandler.sendEmptyMessageDelayed(MSG_REPORT_KEEP_CLEAR_RECTS,
                         KEEP_CLEAR_AREA_REPORT_RATE_MILLIS);
-                try {
-                    mWindowSession.reportKeepClearAreasChanged(mWindow, restrictedKeepClearRects,
-                            unrestrictedKeepClearRects);
-                } catch (RemoteException e) {
-                    throw e.rethrowFromSystemServer();
-                }
+                reportKeepClearAreasChanged();
             }
         }
     }
 
     void reportKeepClearAreasChanged() {
-        final List<Rect> restrictedKeepClearRects = mPendingKeepClearAreas;
-        final List<Rect> unrestrictedKeepClearRects = mPendingUnrestrictedKeepClearAreas;
-        if (restrictedKeepClearRects == null && unrestrictedKeepClearRects == null) {
+        if (!mHasPendingKeepClearAreaChange) {
             return;
         }
+        mHasPendingKeepClearAreaChange = false;
 
-        mPendingKeepClearAreas = null;
-        mPendingUnrestrictedKeepClearAreas = null;
+        final List<Rect> restrictedKeepClearRects = mKeepClearRectsTracker.getLastComputedRects();
+        final List<Rect> unrestrictedKeepClearRects =
+                mUnrestrictedKeepClearRectsTracker.getLastComputedRects();
 
         try {
             mWindowSession.reportKeepClearAreasChanged(mWindow, restrictedKeepClearRects,
diff --git a/core/java/android/view/ViewRootRectTracker.java b/core/java/android/view/ViewRootRectTracker.java
index fd9cc1920b393..152729b8d1d84 100644
--- a/core/java/android/view/ViewRootRectTracker.java
+++ b/core/java/android/view/ViewRootRectTracker.java
@@ -73,10 +73,25 @@ class ViewRootRectTracker {
     }
 
     /**
-     * @return all visible rects from all views in the global (root) coordinate system
+     * @return all Rects from all visible Views in the global (root) coordinate system,
+     * or {@code null} if Rects are unchanged since the last call to this method.
      */
     @Nullable
     public List<Rect> computeChangedRects() {
+        if (computeChanges()) {
+            return mRects;
+        }
+        return null;
+    }
+
+    /**
+     * Computes changes to all Rects from all Views.
+     * After calling this method, the updated list of Rects can be retrieved
+     * with {@link #getLastComputedRects()}.
+     *
+     * @return {@code true} if there were changes, {@code false} otherwise.
+     */
+    public boolean computeChanges() {
         boolean changed = mRootRectsChanged;
         final Iterator<ViewInfo> i = mViewInfos.iterator();
         final List<Rect> rects = new ArrayList<>(mRootRects);
@@ -100,10 +115,22 @@ class ViewRootRectTracker {
             mRootRectsChanged = false;
             if (!mRects.equals(rects)) {
                 mRects = rects;
-                return rects;
+                return true;
             }
         }
-        return null;
+        return false;
+    }
+
+    /**
+     * Returns a List of all Rects from all visible Views in the global (root) coordinate system.
+     * This list is only updated when calling {@link #computeChanges()} or
+     * {@link #computeChangedRects()}.
+     *
+     * @return all Rects from all visible Views in the global (root) coordinate system
+     */
+    @NonNull
+    public List<Rect> getLastComputedRects() {
+        return mRects;
     }
 
     /**