Fixes #1836075. Adds consistency checks for the View hierarchy. To enable them, you need a debug build and ViewDebug.sConsistencyCheckEnabled set to true in debug.prop. This change also lets you easily enable drawing and layout profiling in ViewRoot by setting ViewRoot.sProfileDrawing, ViewRoot.sProfileLayout and ViewRoot.sShowFps in debug.prop with a debug build.

4 files changed, 211 insertions, 22 deletions

diff --git a/core/java/android/view/View.java b/core/java/android/view/View.java
index 335b43c..af5dca6 100644
--- a/core/java/android/view/View.java
+++ b/core/java/android/view/View.java
@@ -49,6 +49,7 @@ import android.util.Poolable;
 import android.util.Pool;
 import android.util.Pools;
 import android.util.PoolableManager;
+import android.util.Config;
 import android.view.ContextMenu.ContextMenuInfo;
 import android.view.animation.Animation;
 import android.view.inputmethod.InputConnection;
@@ -5641,7 +5642,7 @@ public class View implements Drawable.Callback, KeyEvent.Callback {
             if (ViewDebug.TRACE_HIERARCHY) {
                 ViewDebug.trace(this, ViewDebug.HierarchyTraceType.BUILD_CACHE);
             }
-            if (ViewRoot.PROFILE_DRAWING) {
+            if (Config.DEBUG && ViewDebug.profileDrawing) {
                 EventLog.writeEvent(60002, hashCode());
             }
 
@@ -7166,6 +7167,60 @@ public class View implements Drawable.Callback, KeyEvent.Callback {
     }
 
     /**
+     * @param consistency The type of consistency. See ViewDebug for more information.
+     *
+     * @hide
+     */
+    protected boolean dispatchConsistencyCheck(int consistency) {
+        return onConsistencyCheck(consistency);
+    }
+
+    /**
+     * Method that subclasses should implement to check their consistency. The type of
+     * consistency check is indicated by the bit field passed as a parameter.
+     * 
+     * @param consistency The type of consistency. See ViewDebug for more information.
+     *
+     * @throws IllegalStateException if the view is in an inconsistent state.
+     *
+     * @hide
+     */
+    protected boolean onConsistencyCheck(int consistency) {
+        boolean result = true;
+
+        final boolean checkLayout = (consistency & ViewDebug.CONSISTENCY_LAYOUT) != 0;
+        final boolean checkDrawing = (consistency & ViewDebug.CONSISTENCY_DRAWING) != 0;
+
+        if (checkLayout) {
+            if (getParent() == null) {
+                result = false;
+                android.util.Log.d(ViewDebug.CONSISTENCY_LOG_TAG,
+                        "View " + this + " does not have a parent.");
+            }
+
+            if (mAttachInfo == null) {
+                result = false;
+                android.util.Log.d(ViewDebug.CONSISTENCY_LOG_TAG,
+                        "View " + this + " is not attached to a window.");
+            }
+        }
+
+        if (checkDrawing) {
+            // Do not check the DIRTY/DRAWN flags because views can call invalidate()
+            // from their draw() method
+
+            if ((mPrivateFlags & DRAWN) != DRAWN &&
+                    (mPrivateFlags & DRAWING_CACHE_VALID) == DRAWING_CACHE_VALID) {
+                result = false;
+                android.util.Log.d(ViewDebug.CONSISTENCY_LOG_TAG,
+                        "View " + this + " was invalidated but its drawing cache is valid.");
+            }
+        }
+
+        return result;
+    }
+
+    /**
      * Prints information about this view in the log output, with the tag
      * {@link #VIEW_LOG_TAG}.
      *
diff --git a/core/java/android/view/ViewDebug.java b/core/java/android/view/ViewDebug.java
index 367c9a2..4436f4b 100644
--- a/core/java/android/view/ViewDebug.java
+++ b/core/java/android/view/ViewDebug.java
@@ -54,6 +54,27 @@ import java.lang.reflect.AccessibleObject;
  */
 public class ViewDebug {
     /**
+     * Log tag used to log errors related to the consistency of the view hierarchy.
+     *
+     * @hide
+     */
+    public static final String CONSISTENCY_LOG_TAG = "ViewConsistency";
+
+    /**
+     * Flag indicating the consistency check should check layout-related properties.
+     *
+     * @hide
+     */
+    public static final int CONSISTENCY_LAYOUT = 0x1;
+
+    /**
+     * Flag indicating the consistency check should check drawing-related properties.
+     *
+     * @hide
+     */
+    public static final int CONSISTENCY_DRAWING = 0x2;
+
+    /**
      * Enables or disables view hierarchy tracing. Any invoker of
      * {@link #trace(View, android.view.ViewDebug.HierarchyTraceType)} should first
      * check that this value is set to true as not to affect performance.
@@ -80,6 +101,49 @@ public class ViewDebug {
     static final String SYSTEM_PROPERTY_CAPTURE_EVENT = "debug.captureevent";
 
     /**
+     * Profiles drawing times in the events log.
+     *
+     * @hide
+     */
+    @Debug.DebugProperty
+    public static boolean profileDrawing = false;
+
+    /**
+     * Profiles layout times in the events log.
+     *
+     * @hide
+     */
+    @Debug.DebugProperty
+    public static boolean profileLayout = false;
+
+    /**
+     * Profiles real fps (times between draws) and displays the result.
+     *
+     * @hide
+     */
+    @Debug.DebugProperty
+    public static boolean showFps = false;
+
+    /**
+     * <p>Enables or disables views consistency check. Even when this property is enabled,
+     * view consistency checks happen only if {@link android.util.Config#DEBUG} is set
+     * to true. The value of this property can be configured externally in one of the
+     * following files:</p>
+     * <ul>
+     *  <li>/system/debug.prop</li>
+     *  <li>/debug.prop</li>
+     *  <li>/data/debug.prop</li>
+     * </ul>
+     * @hide
+     */
+    @Debug.DebugProperty
+    public static boolean consistencyCheckEnabled = false;
+
+    static {
+        Debug.setFieldsOn(ViewDebug.class, true);
+    }
+
+    /**
      * This annotation can be used to mark fields and methods to be dumped by
      * the view server. Only non-void methods with no arguments can be annotated
      * by this annotation.
diff --git a/core/java/android/view/ViewGroup.java b/core/java/android/view/ViewGroup.java
index 31159d7..26fe776 100644
--- a/core/java/android/view/ViewGroup.java
+++ b/core/java/android/view/ViewGroup.java
@@ -32,6 +32,7 @@ import android.util.AttributeSet;
 import android.util.EventLog;
 import android.util.Log;
 import android.util.SparseArray;
+import android.util.Config;
 import android.view.animation.Animation;
 import android.view.animation.AnimationUtils;
 import android.view.animation.LayoutAnimationController;
@@ -1404,7 +1405,7 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager
 
         // Clear the flag as early as possible to allow draw() implementations
         // to call invalidate() successfully when doing animations
-        child.mPrivateFlags |= DRAWN;
+        child.mPrivateFlags = (child.mPrivateFlags & ~DIRTY_MASK) | DRAWN;
 
         if (!concatMatrix && canvas.quickReject(cl, ct, cr, cb, Canvas.EdgeType.BW) &&
                 (child.mPrivateFlags & DRAW_ANIMATION) == 0) {
@@ -1494,7 +1495,7 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager
                 cachePaint.setAlpha(255);
                 mGroupFlags &= ~FLAG_ALPHA_LOWER_THAN_ONE;
             }
-            if (ViewRoot.PROFILE_DRAWING) {
+            if (Config.DEBUG && ViewDebug.profileDrawing) {
                 EventLog.writeEvent(60003, hashCode());
             }
             canvas.drawBitmap(cache, 0.0f, 0.0f, cachePaint);
@@ -2750,6 +2751,61 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager
     }
 
     /**
+     * @hide
+     */
+    @Override
+    protected boolean dispatchConsistencyCheck(int consistency) {
+        boolean result = super.dispatchConsistencyCheck(consistency);
+
+        final int count = mChildrenCount;
+        final View[] children = mChildren;
+        for (int i = 0; i < count; i++) {
+            if (!children[i].dispatchConsistencyCheck(consistency)) result = false;
+        }
+
+        return result;
+    }
+
+    /**
+     * @hide
+     */
+    @Override
+    protected boolean onConsistencyCheck(int consistency) {
+        boolean result = super.onConsistencyCheck(consistency);
+
+        final boolean checkLayout = (consistency & ViewDebug.CONSISTENCY_LAYOUT) != 0;
+        final boolean checkDrawing = (consistency & ViewDebug.CONSISTENCY_DRAWING) != 0;
+
+        if (checkLayout) {
+            final int count = mChildrenCount;
+            final View[] children = mChildren;
+            for (int i = 0; i < count; i++) {
+                if (children[i].getParent() != this) {
+                    result = false;
+                    android.util.Log.d(ViewDebug.CONSISTENCY_LOG_TAG,
+                            "View " + children[i] + " has no parent/a parent that is not " + this);
+                }
+            }
+        }
+
+        if (checkDrawing) {
+            // If this group is dirty, check that the parent is dirty as well
+            if ((mPrivateFlags & DIRTY_MASK) != 0) {
+                final ViewParent parent = getParent();
+                if (parent != null && !(parent instanceof ViewRoot)) {
+                    if ((((View) parent).mPrivateFlags & DIRTY_MASK) == 0) {
+                        result = false;
+                        android.util.Log.d(ViewDebug.CONSISTENCY_LOG_TAG,
+                                "ViewGroup " + this + " is dirty but its parent is not: " + this);
+                    }
+                }
+            }
+        }
+
+        return result;
+    }
+
+    /**
      * {@inheritDoc}
      */
     @Override
diff --git a/core/java/android/view/ViewRoot.java b/core/java/android/view/ViewRoot.java
index 8494d5e..90453ba 100644
--- a/core/java/android/view/ViewRoot.java
+++ b/core/java/android/view/ViewRoot.java
@@ -75,13 +75,6 @@ public final class ViewRoot extends Handler implements ViewParent,
     private static final boolean DEBUG_IMF = false || LOCAL_LOGV;
     private static final boolean WATCH_POINTER = false;
 
-    static final boolean PROFILE_DRAWING = false;
-    private static final boolean PROFILE_LAYOUT = false;
-    // profiles real fps (times between draws) and displays the result
-    private static final boolean SHOW_FPS = false;
-    // used by SHOW_FPS
-    private static int sDrawTime;
-
     /**
      * Maximum time we allow the user to roll the trackball enough to generate
      * a key event, before resetting the counters.
@@ -97,6 +90,8 @@ public final class ViewRoot extends Handler implements ViewParent,
 
     static final ThreadLocal<RunQueue> sRunQueues = new ThreadLocal<RunQueue>();
 
+    private static int sDrawTime;    
+
     long mLastTrackballTime = 0;
     final TrackballAxis mTrackballAxisX = new TrackballAxis();
     final TrackballAxis mTrackballAxisY = new TrackballAxis();
@@ -796,7 +791,7 @@ public final class ViewRoot extends Handler implements ViewParent,
             final Rect frame = mWinFrame;
             boolean initialized = false;
             boolean contentInsetsChanged = false;
-            boolean visibleInsetsChanged = false;
+            boolean visibleInsetsChanged;
             try {
                 boolean hadSurface = mSurface.isValid();
                 int fl = 0;
@@ -937,14 +932,22 @@ public final class ViewRoot extends Handler implements ViewParent,
             if (DEBUG_ORIENTATION || DEBUG_LAYOUT) Log.v(
                 "ViewRoot", "Laying out " + host + " to (" +
                 host.mMeasuredWidth + ", " + host.mMeasuredHeight + ")");
-            long startTime;
-            if (PROFILE_LAYOUT) {
+            long startTime = 0L;
+            if (Config.DEBUG && ViewDebug.profileLayout) {
                 startTime = SystemClock.elapsedRealtime();
             }
 
             host.layout(0, 0, host.mMeasuredWidth, host.mMeasuredHeight);
 
-            if (PROFILE_LAYOUT) {
+            if (Config.DEBUG && ViewDebug.consistencyCheckEnabled) {
+                if (!host.dispatchConsistencyCheck(ViewDebug.CONSISTENCY_LAYOUT)) {
+                    throw new IllegalStateException("The view hierarchy is an inconsistent state,"
+                            + "please refer to the logs with the tag "
+                            + ViewDebug.CONSISTENCY_LOG_TAG + " for more infomation.");
+                }
+            }
+
+            if (Config.DEBUG && ViewDebug.profileLayout) {
                 EventLog.writeEvent(60001, SystemClock.elapsedRealtime() - startTime);
             }
 
@@ -960,10 +963,11 @@ public final class ViewRoot extends Handler implements ViewParent,
                         mTmpLocation[1] + host.mBottom - host.mTop);
 
                 host.gatherTransparentRegion(mTransparentRegion);
-                if (mAppScale != 1.0f) {
-                    mTransparentRegion.scale(mAppScale);
-                }
 
+                // TODO: scale the region, like:
+                // Region uses native methods. We probabl should have ScalableRegion class.
+
+                // Region does not have equals method ?
                 if (!mTransparentRegion.equals(mPreviousTransparentRegion)) {
                     mPreviousTransparentRegion.set(mTransparentRegion);
                     // reconfigure window manager
@@ -1168,6 +1172,9 @@ public final class ViewRoot extends Handler implements ViewParent,
                             canvas.scale(scale, scale);
                         }
                         mView.draw(canvas);
+                        if (Config.DEBUG && ViewDebug.consistencyCheckEnabled) {
+                            mView.dispatchConsistencyCheck(ViewDebug.CONSISTENCY_DRAWING);
+                        }
                     } finally {
                         canvas.restoreToCount(saveCount);
                     }
@@ -1175,7 +1182,7 @@ public final class ViewRoot extends Handler implements ViewParent,
                     mEgl.eglSwapBuffers(mEglDisplay, mEglSurface);
                     checkEglErrors();
 
-                    if (SHOW_FPS) {
+                    if (Config.DEBUG && ViewDebug.showFps) {
                         int now = (int)SystemClock.elapsedRealtime();
                         if (sDrawTime != 0) {
                             nativeShowFPS(canvas, now - sDrawTime);
@@ -1216,7 +1223,7 @@ public final class ViewRoot extends Handler implements ViewParent,
 
         try {
             if (!dirty.isEmpty() || mIsAnimating) {
-                long startTime;
+                long startTime = 0L;
 
                 if (DEBUG_ORIENTATION || DEBUG_DRAW) {
                     Log.v("ViewRoot", "Surface " + surface + " drawing to bitmap w="
@@ -1224,7 +1231,7 @@ public final class ViewRoot extends Handler implements ViewParent,
                     //canvas.drawARGB(255, 255, 0, 0);
                 }
 
-                if (PROFILE_DRAWING) {
+                if (Config.DEBUG && ViewDebug.profileDrawing) {
                     startTime = SystemClock.elapsedRealtime();
                 }
 
@@ -1259,11 +1266,15 @@ public final class ViewRoot extends Handler implements ViewParent,
                         canvas.scale(scale, scale);
                     }
                     mView.draw(canvas);
+
+                    if (Config.DEBUG && ViewDebug.consistencyCheckEnabled) {
+                        mView.dispatchConsistencyCheck(ViewDebug.CONSISTENCY_DRAWING);
+                    }
                 } finally {
                     canvas.restoreToCount(saveCount);
                 }
 
-                if (SHOW_FPS) {
+                if (Config.DEBUG && ViewDebug.showFps) {
                     int now = (int)SystemClock.elapsedRealtime();
                     if (sDrawTime != 0) {
                         nativeShowFPS(canvas, now - sDrawTime);
@@ -1271,7 +1282,7 @@ public final class ViewRoot extends Handler implements ViewParent,
                     sDrawTime = now;
                 }
 
-                if (PROFILE_DRAWING) {
+                if (Config.DEBUG && ViewDebug.profileDrawing) {
                     EventLog.writeEvent(60000, SystemClock.elapsedRealtime() - startTime);
                 }
             }
@@ -1878,6 +1889,9 @@ public final class ViewRoot extends Handler implements ViewParent,
         } else {
             didFinish = false;
         }
+        if (event != null) {
+            event.scale(mAppScaleInverted);
+        }
 
         if (DEBUG_TRACKBALL) Log.v(TAG, "Motion event:" + event);