OSDN Git Service

Changed Content Catpure workflow so it notifies when multiple changes are made.
authorFelipe Leme <felipeal@google.com>
Fri, 22 Feb 2019 17:26:29 +0000 (09:26 -0800)
committerFelipe Leme <felipeal@google.com>
Fri, 22 Feb 2019 22:57:51 +0000 (14:57 -0800)
Prior to this change, it sent a pair TYPE_INITIAL_VIEW_TREE_APPEARING and
TYPE_INITIAL_VIEW_TREE_APPEARED after the initial layout, then it would send invididual events for
the views appeared / disappeared.

This change improves the workflow by also sending this pair of events after each change, which lets
the service know that a bunch of changes were made at the same layout pass.

Test: atest CtsContentCaptureServiceTestCases # which was updated to listen to the new events
Test: m update-api

Bug: 125395044

Change-Id: Ied9def9c95dd0f7711f59bccb2cc89a766fdc36b

core/java/android/view/View.java
core/java/android/view/ViewRootImpl.java
core/java/android/view/contentcapture/ChildContentCaptureSession.java
core/java/android/view/contentcapture/ContentCaptureEvent.java
core/java/android/view/contentcapture/ContentCaptureSession.java
core/java/android/view/contentcapture/MainContentCaptureSession.java

index b9ba1a0..77aaee4 100644 (file)
@@ -9408,20 +9408,13 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
             }
             setNotifiedContentCaptureAppeared();
 
-            // TODO(b/125395044): instead of post, we should queue it on AttachInfo and then
-            // dispatch on RootImpl, as we're doing with the removed ones (in that case, we should
-            // merge the delayNotifyContentCaptureDisappeared() into a more generic method that
-            // takes a session and a command, where the command is either view added or removed
-
-            // The code below doesn't take much for a unique view, but it's called for all views
-            // the first time the view hiearchy is laid off, which could acccumulative delay the
-            // initial layout. Hence, we're postponing it to a later stage - it might still cost a
-            // lost frame (or more), but that jank cost would only happen after the 1st layout.
-            Choreographer.getInstance().postCallback(Choreographer.CALLBACK_COMMIT, () -> {
-                final ViewStructure structure = session.newViewStructure(this);
-                onProvideContentCaptureStructure(structure, /* flags= */ 0);
-                session.notifyViewAppeared(structure);
-            }, /* token= */ null);
+            if (ai != null) {
+                ai.delayNotifyContentCaptureEvent(session, this, appeared);
+            } else {
+                if (DEBUG_CONTENT_CAPTURE) {
+                    Log.w(CONTENT_CAPTURE_LOG_TAG, "no AttachInfo on appeared for " + this);
+                }
+            }
         } else {
             if ((mPrivateFlags4 & PFLAG4_NOTIFIED_CONTENT_CAPTURE_APPEARED) == 0
                     || (mPrivateFlags4 & PFLAG4_NOTIFIED_CONTENT_CAPTURE_DISAPPEARED) != 0) {
@@ -9440,13 +9433,11 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
             mPrivateFlags4 &= ~PFLAG4_NOTIFIED_CONTENT_CAPTURE_APPEARED;
 
             if (ai != null) {
-                ai.delayNotifyContentCaptureDisappeared(session, getAutofillId());
+                ai.delayNotifyContentCaptureEvent(session, this, appeared);
             } else {
                 if (DEBUG_CONTENT_CAPTURE) {
-                    Log.v(CONTENT_CAPTURE_LOG_TAG, "no AttachInfo on gone for " + this);
+                    Log.v(CONTENT_CAPTURE_LOG_TAG, "no AttachInfo on disappeared for " + this);
                 }
-                Choreographer.getInstance().postCallback(Choreographer.CALLBACK_COMMIT,
-                        () -> session.notifyViewDisappeared(getAutofillId()), /* token= */ null);
             }
         }
     }
@@ -28364,11 +28355,12 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
         boolean mReadyForContentCaptureUpdates;
 
         /**
-         * Map of ids (per session) that need to be notified after as gone the view hierchy is
-         * traversed.
+         * Map(keyed by session) of content capture events that need to be notified after the view
+         * hierarchy is traversed: value is either the view itself for appearead events, or its
+         * autofill id for disappeared.
          */
         // TODO(b/121197119): use SparseArray once session id becomes integer
-        ArrayMap<String, ArrayList<AutofillId>> mContentCaptureRemovedIds;
+        ArrayMap<String, ArrayList<Object>> mContentCaptureEvents;
 
         /**
          * Cached reference to the {@link ContentCaptureManager}.
@@ -28394,20 +28386,20 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
             mTreeObserver = new ViewTreeObserver(context);
         }
 
-        private void delayNotifyContentCaptureDisappeared(@NonNull ContentCaptureSession session,
-                @NonNull AutofillId id) {
-            if (mContentCaptureRemovedIds == null) {
+        private void delayNotifyContentCaptureEvent(@NonNull ContentCaptureSession session,
+                @NonNull View view, boolean appeared) {
+            if (mContentCaptureEvents == null) {
                 // Most of the time there will be just one session, so intial capacity is 1
-                mContentCaptureRemovedIds = new ArrayMap<>(1);
+                mContentCaptureEvents = new ArrayMap<>(1);
             }
             String sessionId = session.getId();
             // TODO: life would be much easier if we provided a MultiMap implementation somwhere...
-            ArrayList<AutofillId> ids = mContentCaptureRemovedIds.get(sessionId);
-            if (ids == null) {
-                ids = new ArrayList<>();
-                mContentCaptureRemovedIds.put(sessionId, ids);
+            ArrayList<Object> events = mContentCaptureEvents.get(sessionId);
+            if (events == null) {
+                events = new ArrayList<>();
+                mContentCaptureEvents.put(sessionId, events);
             }
-            ids.add(id);
+            events.add(appeared ? view : view.getAutofillId());
         }
 
         @Nullable
index cbcb520..33795e5 100644 (file)
@@ -2796,28 +2796,58 @@ public final class ViewRootImpl implements ViewParent,
             }
         }
 
-        // TODO(b/125395044): might need to check for added events too and flush them
-        if (mAttachInfo.mContentCaptureRemovedIds != null) {
+        if (mAttachInfo.mContentCaptureEvents != null) {
+            notifyContentCatpureEvents();
+        }
+
+        mIsInTraversal = false;
+    }
+
+    private void notifyContentCatpureEvents() {
+        Trace.traceBegin(Trace.TRACE_TAG_VIEW, "notifyContentCaptureEvents");
+        try {
             MainContentCaptureSession mainSession = mAttachInfo.mContentCaptureManager
                     .getMainContentCaptureSession();
-            Trace.traceBegin(Trace.TRACE_TAG_VIEW, "notifyContentCaptureViewsGone");
-            try {
-                for (int i = 0; i < mAttachInfo.mContentCaptureRemovedIds.size(); i++) {
-                    String sessionId = mAttachInfo.mContentCaptureRemovedIds
-                            .keyAt(i);
-                    ArrayList<AutofillId> ids = mAttachInfo.mContentCaptureRemovedIds
-                            .valueAt(i);
-                    mainSession.notifyViewsDisappeared(sessionId, ids);
-                }
-                mAttachInfo.mContentCaptureRemovedIds = null;
-                mAttachInfo.mContentCaptureManager
-                        .flush(ContentCaptureSession.FLUSH_REASON_POST_VIEW_ROOT_TRAVERSAL);
-            } finally {
-                Trace.traceEnd(Trace.TRACE_TAG_VIEW);
+            if (mainSession == null) {
+                Log.w(mTag, "no MainContentCaptureSession on AttachInfo");
+                return;
             }
+            for (int i = 0; i < mAttachInfo.mContentCaptureEvents.size(); i++) {
+                String sessionId = mAttachInfo.mContentCaptureEvents
+                        .keyAt(i);
+                mainSession.notifyViewHierarchyEvent(sessionId, /* started = */ true);
+                ArrayList<Object> events = mAttachInfo.mContentCaptureEvents
+                        .valueAt(i);
+                for_each_event: for (int j = 0; j < events.size(); j++) {
+                    Object event = events.get(j);
+                    if (event instanceof AutofillId) {
+                        mainSession.notifyViewDisappeared(sessionId, (AutofillId) event);
+                    } else if (event instanceof View) {
+                        View view = (View) event;
+                        ContentCaptureSession session = view.getContentCaptureSession();
+                        if (session == null) {
+                            Log.w(mTag, "no content capture session on view: " + view);
+                            continue for_each_event;
+                        }
+                        String actualId = session.getId().toString();
+                        if (!actualId.equals(sessionId)) {
+                            Log.w(mTag, "content capture session mismatch for view (" + view
+                                    + "): was " + sessionId + " before, it's " + actualId + " now");
+                            continue for_each_event;
+                        }
+                        ViewStructure structure = session.newViewStructure(view);
+                        view.onProvideContentCaptureStructure(structure, /* flags= */ 0);
+                        session.notifyViewAppeared(structure);
+                    } else {
+                        Log.w(mTag, "invalid content capture event: " + event);
+                    }
+                }
+                mainSession.notifyViewHierarchyEvent(sessionId, /* started = */ false);
+            }
+            mAttachInfo.mContentCaptureEvents = null;
+        } finally {
+            Trace.traceEnd(Trace.TRACE_TAG_VIEW);
         }
-
-        mIsInTraversal = false;
     }
 
     private void notifySurfaceDestroyed() {
@@ -2950,10 +2980,9 @@ public final class ViewRootImpl implements ViewParent,
             }
         }
         mFirstInputStage.onWindowFocusChanged(hasWindowFocus);
-        // TODO(b/125395044): right now the list of events is always empty on
-        // when hasWindowFocus is false, as the removed events are effectively flushed on
-        // FLUSH_REASON_POST_VIEW_ROOT_TRAVERSAL. If after the final refactorings that's still the
-        // case, we should add another reason for FLUSH_REASON_VIEW_ROOT_EXITED
+        // NOTE: there's no view visibility (appeared / disapparead) events when the windows focus
+        // is lost, so we don't need to to force a flush - there might be other events such as
+        // text changes, but these should be flushed independently.
         if (hasWindowFocus) {
             performContentCaptureFlushIfNecessary(
                     ContentCaptureSession.FLUSH_REASON_VIEW_ROOT_ENTERED);
index 13e8a65..03eef5e 100644 (file)
@@ -85,7 +85,7 @@ final class ChildContentCaptureSession extends ContentCaptureSession {
 
     @Override
     public void internalNotifyViewHierarchyEvent(boolean started) {
-        getMainCaptureSession().notifyInitialViewHierarchyEvent(mId, started);
+        getMainCaptureSession().notifyViewHierarchyEvent(mId, started);
     }
 
     @Override
index 9cdbefa..1dd1bee 100644 (file)
@@ -72,23 +72,25 @@ public final class ContentCaptureEvent implements Parcelable {
     public static final int TYPE_VIEW_TEXT_CHANGED = 3;
 
     /**
-     * Called before events (such as {@link #TYPE_VIEW_APPEARED}) representing the initial view
-     * hierarchy are sent.
+     * Called before events (such as {@link #TYPE_VIEW_APPEARED} and/or
+     * {@link #TYPE_VIEW_DISAPPEARED}) representing a view hierarchy are sent.
      *
      * <p><b>NOTE</b>: there is no guarantee this event will be sent. For example, it's not sent
      * if the initial view hierarchy doesn't initially have any view that's important for content
      * capture.
      */
+    // TODO(b/125395044): change to TYPE_VIEW_TREE_APPEARING
     public static final int TYPE_INITIAL_VIEW_TREE_APPEARING = 4;
 
     /**
-     * Called after events (such as {@link #TYPE_VIEW_APPEARED}) representing the initial view
-     * hierarchy are sent.
+     * Called after events (such as {@link #TYPE_VIEW_APPEARED} and/or
+     * {@link #TYPE_VIEW_DISAPPEARED}) representing a view hierarchy were sent.
      *
      * <p><b>NOTE</b>: there is no guarantee this event will be sent. For example, it's not sent
      * if the initial view hierarchy doesn't initially have any view that's important for content
      * capture.
      */
+    // TODO(b/125395044): change to TYPE_VIEW_TREE_APPEARED
     public static final int TYPE_INITIAL_VIEW_TREE_APPEARED = 5;
 
     /**
@@ -418,9 +420,9 @@ public final class ContentCaptureEvent implements Parcelable {
             case TYPE_VIEW_TEXT_CHANGED:
                 return "VIEW_TEXT_CHANGED";
             case TYPE_INITIAL_VIEW_TREE_APPEARING:
-                return "INITIAL_VIEW_HIERARCHY_STARTED";
+                return "VIEW_TREE_APPEARING";
             case TYPE_INITIAL_VIEW_TREE_APPEARED:
-                return "INITIAL_VIEW_HIERARCHY_FINISHED";
+                return "VIEW_TREE_APPEARED";
             case TYPE_CONTEXT_UPDATED:
                 return "CONTEXT_UPDATED";
             default:
index 49d985c..544a4f6 100644 (file)
@@ -134,8 +134,6 @@ public abstract class ContentCaptureSession implements AutoCloseable {
     /** @hide */
     public static final int FLUSH_REASON_VIEW_ROOT_ENTERED = 2;
     /** @hide */
-    public static final int FLUSH_REASON_POST_VIEW_ROOT_TRAVERSAL = 3;
-    /** @hide */
     public static final int FLUSH_REASON_SESSION_STARTED = 4;
     /** @hide */
     public static final int FLUSH_REASON_SESSION_FINISHED = 5;
@@ -149,7 +147,6 @@ public abstract class ContentCaptureSession implements AutoCloseable {
             FLUSH_REASON_SESSION_FINISHED,
             FLUSH_REASON_IDLE_TIMEOUT,
             FLUSH_REASON_VIEW_ROOT_ENTERED,
-            FLUSH_REASON_POST_VIEW_ROOT_TRAVERSAL
     })
     @Retention(RetentionPolicy.SOURCE)
     public @interface FlushReason{}
@@ -512,8 +509,6 @@ public abstract class ContentCaptureSession implements AutoCloseable {
                 return "IDLE";
             case FLUSH_REASON_VIEW_ROOT_ENTERED:
                 return "ENTERED";
-            case FLUSH_REASON_POST_VIEW_ROOT_TRAVERSAL:
-                return "TRAVERSAL";
             default:
                 return "UNKOWN-" + reason;
         }
index b55b975..14b2b28 100644 (file)
@@ -126,7 +126,6 @@ public final class MainContentCaptureSession extends ContentCaptureSession {
     @Nullable
     private final LocalLog mFlushHistory;
 
-    /** @hide */
     protected MainContentCaptureSession(@NonNull Context context,
             @NonNull ContentCaptureManager manager, @NonNull Handler handler,
             @NonNull IContentCaptureManager systemServerInterface) {
@@ -153,8 +152,6 @@ public final class MainContentCaptureSession extends ContentCaptureSession {
 
     /**
      * Starts this session.
-     *
-     * @hide
      */
     @UiThread
     void start(@NonNull IBinder token, @NonNull ComponentName component,
@@ -547,7 +544,7 @@ public final class MainContentCaptureSession extends ContentCaptureSession {
 
     @Override
     public void internalNotifyViewHierarchyEvent(boolean started) {
-        notifyInitialViewHierarchyEvent(mId, started);
+        notifyViewHierarchyEvent(mId, started);
     }
 
     @Override
@@ -581,21 +578,9 @@ public final class MainContentCaptureSession extends ContentCaptureSession {
                 .setViewNode(node.mNode));
     }
 
-    void notifyViewDisappeared(@NonNull String sessionId, @NonNull AutofillId id) {
-        sendEvent(
-                new ContentCaptureEvent(sessionId, TYPE_VIEW_DISAPPEARED).setAutofillId(id));
-    }
-
-    /** @hide */
-    public void notifyViewsDisappeared(@NonNull String sessionId,
-            @NonNull ArrayList<AutofillId> ids) {
-        final ContentCaptureEvent event = new ContentCaptureEvent(sessionId, TYPE_VIEW_DISAPPEARED);
-        if (ids.size() == 1) {
-            event.setAutofillId(ids.get(0));
-        } else {
-            event.setAutofillIds(ids);
-        }
-        sendEvent(event);
+    /** Public because is also used by ViewRootImpl */
+    public void notifyViewDisappeared(@NonNull String sessionId, @NonNull AutofillId id) {
+        sendEvent(new ContentCaptureEvent(sessionId, TYPE_VIEW_DISAPPEARED).setAutofillId(id));
     }
 
     void notifyViewTextChanged(@NonNull String sessionId, @NonNull AutofillId id,
@@ -604,13 +589,11 @@ public final class MainContentCaptureSession extends ContentCaptureSession {
                 .setText(text));
     }
 
-    void notifyInitialViewHierarchyEvent(@NonNull String sessionId, boolean started) {
-        if (started) {
-            sendEvent(new ContentCaptureEvent(sessionId, TYPE_INITIAL_VIEW_TREE_APPEARING));
-        } else {
-            sendEvent(new ContentCaptureEvent(sessionId, TYPE_INITIAL_VIEW_TREE_APPEARED),
-                    FORCE_FLUSH);
-        }
+    /** Public because is also used by ViewRootImpl */
+    public void notifyViewHierarchyEvent(@NonNull String sessionId, boolean started) {
+        final int type = started ? TYPE_INITIAL_VIEW_TREE_APPEARING
+                : TYPE_INITIAL_VIEW_TREE_APPEARED;
+        sendEvent(new ContentCaptureEvent(sessionId, type), FORCE_FLUSH);
     }
 
     void notifyContextUpdated(@NonNull String sessionId,