Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit eb2b0af2 authored by Adam Powell's avatar Adam Powell
Browse files

Restore app expectations around drawable visibility change timing 

When we update drawable visibility has changed to be part of
View.onVisibilityChanged instead of an overload of
setVisibility. While this covers more cases that we were previously
missing, it also means that we now set drawable visibility from the
View constructor via the call chain view.setFlags to
view.onVisibilityChanged to drawable.setVisibility, resulting in us
passing a 'this' pointer all over before the object is fully
initialized. (i.e. a Bad Thing.)

In general we've gotten away with playing fast and loose with this
sort of thing as a part of view inflation - calling various non-final
setters that may invoke callbacks as needed rather than initializing
view fields directly. Unfortunately it also means that we can cause a
lot of hard to trace bugs and in the long run we should try to clean
up as much of it as we can.

In this case, some apps were expecting inflation to have finished
completely before any drawable visibility changed. If a view's
visibility changes but it's not attached to a window, does it make a
callback?

The fix: no. We won't dispatch onVisibilityChanged to detached views,
but we will dispatch it when a view becomes attached.

Also fix a bug where we could end up telling a view its visibility
changed to (INVISIBLE | GONE), which just doesn't make any sense.

Bug 20103422

Change-Id: Ifba54c36114e85cf64869afcca766c30d601a16c
parent f5b1e39c

core/java/android/view/View.java

+23 −13
Original line number Diff line number Diff line
@@ -1766,6 +1766,9 @@ public class View implements Drawable.Callback, KeyEvent.Callback, 
    /**

     * Indicates that we should awaken scroll bars once attached

     *

     * PLEASE NOTE: This flag is now unused as we now send onVisibilityChanged

     * during window attachment and it is no longer needed. Feel free to repurpose it.

     *

     * @hide

     */

    private static final int PFLAG_AWAKEN_SCROLL_BARS_ON_ATTACH = 0x08000000;
@@ -9524,12 +9527,8 @@ public class View implements Drawable.Callback, KeyEvent.Callback, 
     */

    protected void onVisibilityChanged(@NonNull View changedView, @Visibility int visibility) {

        final boolean visible = visibility == VISIBLE && getVisibility() == VISIBLE;

        if (visible) {

            if (mAttachInfo != null) {

        if (visible && mAttachInfo != null) {

            initialAwakenScrollBars();

            } else {

                mPrivateFlags |= PFLAG_AWAKEN_SCROLL_BARS_ON_ATTACH;

            }

        }















        final Drawable dr = mBackground;










@@ -10585,10 +10584,12 @@ public class View implements Drawable.Callback, KeyEvent.Callback,













            } else if (mParent != null) {















                mParent.invalidateChild(this, null);















            }













            dispatchVisibilityChanged(this, newVisibility);



























            if (mAttachInfo != null) {













                dispatchVisibilityChanged(this, newVisibility);















                notifySubtreeAccessibilityStateChangedIfNeeded();















            }













        }





























        if ((changed & WILL_NOT_CACHE_DRAWING) != 0) {















            destroyDrawingCache();










@@ -13984,11 +13985,6 @@ public class View implements Drawable.Callback, KeyEvent.Callback,













            mParent.requestTransparentRegion(this);















        }



























        if ((mPrivateFlags & PFLAG_AWAKEN_SCROLL_BARS_ON_ATTACH) != 0) {













            initialAwakenScrollBars();













            mPrivateFlags &= ~PFLAG_AWAKEN_SCROLL_BARS_ON_ATTACH;













        }



























        mPrivateFlags3 &= ~PFLAG3_IS_LAID_OUT;





























        jumpDrawablesToCurrentState();










@@ -14432,6 +14428,14 @@ public class View implements Drawable.Callback, KeyEvent.Callback,













        return mAttachInfo != null ? mAttachInfo.mSession : null;















    }



























    /**













     * Return the visibility value of the least visible component passed.













     */













    int combineVisibility(int vis1, int vis2) {













        // This works because VISIBLE < INVISIBLE < GONE.













        return Math.max(vis1, vis2);













    }



























    /**















     * @param info the {@link android.view.View.AttachInfo} to associated with















     *        this view










@@ -14473,6 +14477,12 @@ public class View implements Drawable.Callback, KeyEvent.Callback,













        if (vis != GONE) {















            onWindowVisibilityChanged(vis);















        }

























        // Send onVisibilityChanged directly instead of dispatchVisibilityChanged.













        // As all views in the subtree will already receive dispatchAttachedToWindow













        // traversing the subtree again here is not desired.













        onVisibilityChanged(this, visibility);



























        if ((mPrivateFlags&PFLAG_DRAWABLE_STATE_DIRTY) != 0) {















            // If nobody has evaluated the drawable state yet, then do it now.















            refreshDrawableState();

























core/java/android/view/ViewGroup.java



+3
−2




























Original line number
Diff line number
Diff line








@@ -2830,12 +2830,13 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager













        for (int i = 0; i < count; i++) {















            final View child = children[i];















            child.dispatchAttachedToWindow(info,













                    visibility | (child.mViewFlags & VISIBILITY_MASK));













                    combineVisibility(visibility, child.getVisibility()));















        }















        final int transientCount = mTransientIndices == null ? 0 : mTransientIndices.size();















        for (int i = 0; i < transientCount; ++i) {















            View view = mTransientViews.get(i);













            view.dispatchAttachedToWindow(info, visibility | (view.mViewFlags & VISIBILITY_MASK));













            view.dispatchAttachedToWindow(info,













                    combineVisibility(visibility, view.getVisibility()));















        }















    }