From 43ad57e548b86daf50e0ebfde440fe431a41cb1e Mon Sep 17 00:00:00 2001 From: Dake Gu Date: Tue, 1 Aug 2017 13:42:42 -0700 Subject: [PATCH] View: fix setTransientState Following four steps with RecyclerView - CardView - ImageView: a. fade in imageview in onBind: calling imageView.setTransientState(true) b. RecyclerView animate the item: calling cardView.setTransientState(true) c. fade-in finishes, calling imageView.setTransientState(false) d. RecyclerView animation finishes: calling cardView.setTransientState(false) After these four steps, RecyclerView unexpectedly has transient state. The problem is in step b, when calling cardView.setTransientState() it incorrectly calls parent.childHasTransientStateChanged(this, true) which causes RecyclerView's mChildCountWithTransientState increased to 2. And it's decreased to 1 in step d and stay as 1 forever. The child should only call childHasTransientStateChanged() when actual hasTransientState() changed. Bug: 64235615 Test: ViewTransientStateTest Change-Id: I99ed35cc9c49e54d36590d8f1d206501fd3288f2 --- core/java/android/view/View.java | 6 +- core/tests/coretests/AndroidManifest.xml | 7 ++ .../res/layout/view_transient_state.xml | 45 +++++++++ .../src/android/view/ViewTransientState.java | 33 +++++++ .../android/view/ViewTransientStateTest.java | 98 +++++++++++++++++++ 5 files changed, 187 insertions(+), 2 deletions(-) create mode 100644 core/tests/coretests/res/layout/view_transient_state.xml create mode 100644 core/tests/coretests/src/android/view/ViewTransientState.java create mode 100644 core/tests/coretests/src/android/view/ViewTransientStateTest.java diff --git a/core/java/android/view/View.java b/core/java/android/view/View.java index 6af01f66ac6eb..eea692aa952a7 100644 --- a/core/java/android/view/View.java +++ b/core/java/android/view/View.java @@ -9711,6 +9711,7 @@ public class View implements Drawable.Callback, KeyEvent.Callback, * @param hasTransientState true if this view has transient state */ public void setHasTransientState(boolean hasTransientState) { + final boolean oldHasTransientState = hasTransientState(); mTransientStateCount = hasTransientState ? mTransientStateCount + 1 : mTransientStateCount - 1; if (mTransientStateCount < 0) { @@ -9722,9 +9723,10 @@ public class View implements Drawable.Callback, KeyEvent.Callback, // update flag if we've just incremented up from 0 or decremented down to 0 mPrivateFlags2 = (mPrivateFlags2 & ~PFLAG2_HAS_TRANSIENT_STATE) | (hasTransientState ? PFLAG2_HAS_TRANSIENT_STATE : 0); - if (mParent != null) { + final boolean newHasTransientState = hasTransientState(); + if (mParent != null && newHasTransientState != oldHasTransientState) { try { - mParent.childHasTransientStateChanged(this, hasTransientState); + mParent.childHasTransientStateChanged(this, newHasTransientState); } catch (AbstractMethodError e) { Log.e(VIEW_LOG_TAG, mParent.getClass().getSimpleName() + " does not fully implement ViewParent", e); diff --git a/core/tests/coretests/AndroidManifest.xml b/core/tests/coretests/AndroidManifest.xml index e127896a70912..ab9912a438d4c 100644 --- a/core/tests/coretests/AndroidManifest.xml +++ b/core/tests/coretests/AndroidManifest.xml @@ -978,6 +978,13 @@ + + + + + + + diff --git a/core/tests/coretests/res/layout/view_transient_state.xml b/core/tests/coretests/res/layout/view_transient_state.xml new file mode 100644 index 0000000000000..887301547ef14 --- /dev/null +++ b/core/tests/coretests/res/layout/view_transient_state.xml @@ -0,0 +1,45 @@ + + + + + + + + + + + + + + + + diff --git a/core/tests/coretests/src/android/view/ViewTransientState.java b/core/tests/coretests/src/android/view/ViewTransientState.java new file mode 100644 index 0000000000000..206ff81660547 --- /dev/null +++ b/core/tests/coretests/src/android/view/ViewTransientState.java @@ -0,0 +1,33 @@ +/* + * Copyright (C) 2007 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.view; + +import android.app.Activity; +import android.os.Bundle; + +import com.android.frameworks.coretests.R; + +/** + * Exercise set View's transient state + */ +public class ViewTransientState extends Activity { + @Override + protected void onCreate(Bundle savedInstanceState) { + super.onCreate(savedInstanceState); + setContentView(R.layout.view_transient_state); + } +} diff --git a/core/tests/coretests/src/android/view/ViewTransientStateTest.java b/core/tests/coretests/src/android/view/ViewTransientStateTest.java new file mode 100644 index 0000000000000..36ea01deea220 --- /dev/null +++ b/core/tests/coretests/src/android/view/ViewTransientStateTest.java @@ -0,0 +1,98 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.view; + +import android.app.Activity; +import android.test.ActivityInstrumentationTestCase; +import android.test.UiThreadTest; +import android.test.suitebuilder.annotation.MediumTest; + +import com.android.frameworks.coretests.R; + +import static org.junit.Assert.assertFalse; + +/** + * Exercise set View's transient state + */ +public class ViewTransientStateTest extends ActivityInstrumentationTestCase { + + View mP1; + View mP2; + View mP3; + + public ViewTransientStateTest() { + super("com.android.frameworks.coretests", ViewTransientState.class); + } + + @Override + public void setUp() throws Exception { + super.setUp(); + + final Activity a = getActivity(); + mP1 = a.findViewById(R.id.p1); + mP2 = a.findViewById(R.id.p2); + mP3 = a.findViewById(R.id.p3); + } + + @UiThreadTest + @MediumTest + public void testSetTransientState1() throws Exception { + mP3.setHasTransientState(true); + mP2.setHasTransientState(true); + mP3.setHasTransientState(false); + mP2.setHasTransientState(false); + assertFalse(mP3.hasTransientState()); + assertFalse(mP2.hasTransientState()); + assertFalse(mP1.hasTransientState()); + } + + @UiThreadTest + @MediumTest + public void testSetTransientState2() throws Exception { + mP3.setHasTransientState(true); + mP2.setHasTransientState(true); + mP2.setHasTransientState(false); + mP3.setHasTransientState(false); + assertFalse(mP3.hasTransientState()); + assertFalse(mP2.hasTransientState()); + assertFalse(mP1.hasTransientState()); + } + + @UiThreadTest + @MediumTest + public void testSetTransientState3() throws Exception { + mP2.setHasTransientState(true); + mP3.setHasTransientState(true); + mP3.setHasTransientState(false); + mP2.setHasTransientState(false); + assertFalse(mP3.hasTransientState()); + assertFalse(mP2.hasTransientState()); + assertFalse(mP1.hasTransientState()); + } + + @UiThreadTest + @MediumTest + public void testSetTransientState4() throws Exception { + mP2.setHasTransientState(true); + mP3.setHasTransientState(true); + mP2.setHasTransientState(false); + mP3.setHasTransientState(false); + assertFalse(mP3.hasTransientState()); + assertFalse(mP2.hasTransientState()); + assertFalse(mP1.hasTransientState()); + } +}