Fix NPE when a shared element doesn't have a matching transitionName.
Bug 33053923
Also added an early error check when the same shared element is
used for multiple shared elements in the source or target fragment.
Test: gradlew connectedCheck in fragment/
Change-Id: If5dfbac66cfd9e60660439ce5c519547de3ef1d0
diff --git a/fragment/java/android/support/v4/app/BackStackRecord.java b/fragment/java/android/support/v4/app/BackStackRecord.java
index 85ded71..75cae8a 100644
--- a/fragment/java/android/support/v4/app/BackStackRecord.java
+++ b/fragment/java/android/support/v4/app/BackStackRecord.java
@@ -513,6 +513,12 @@
if (mSharedElementSourceNames == null) {
mSharedElementSourceNames = new ArrayList<String>();
mSharedElementTargetNames = new ArrayList<String>();
+ } else if (mSharedElementTargetNames.contains(name)) {
+ throw new IllegalArgumentException("A shared element with the target name '"
+ + name + "' has already been added to the transaction.");
+ } else if (mSharedElementSourceNames.contains(transitionName)) {
+ throw new IllegalArgumentException("A shared element with the source name '"
+ + transitionName + " has already been added to the transaction.");
}
mSharedElementSourceNames.add(transitionName);
diff --git a/fragment/java/android/support/v4/app/FragmentTransition.java b/fragment/java/android/support/v4/app/FragmentTransition.java
index 4676434..aa64859 100644
--- a/fragment/java/android/support/v4/app/FragmentTransition.java
+++ b/fragment/java/android/support/v4/app/FragmentTransition.java
@@ -364,10 +364,12 @@
}
if (exitingViews != null) {
- ArrayList<View> tempExiting = new ArrayList<>();
- tempExiting.add(nonExistentView);
- FragmentTransitionCompat21.replaceTargets(exitTransition, exitingViews,
- tempExiting);
+ if (exitTransition != null) {
+ ArrayList<View> tempExiting = new ArrayList<>();
+ tempExiting.add(nonExistentView);
+ FragmentTransitionCompat21.replaceTargets(exitTransition, exitingViews,
+ tempExiting);
+ }
exitingViews.clear();
exitingViews.add(nonExistentView);
}
@@ -476,9 +478,17 @@
if (nameOverrides.isEmpty()) {
sharedElementTransition = null;
+ if (outSharedElements != null) {
+ outSharedElements.clear();
+ }
+ if (inSharedElements != null) {
+ inSharedElements.clear();
+ }
} else {
- sharedElementsOut.addAll(outSharedElements.values());
- sharedElementsIn.addAll(inSharedElements.values());
+ addSharedElementsWithMatchingNames(sharedElementsOut, outSharedElements,
+ nameOverrides.keySet());
+ addSharedElementsWithMatchingNames(sharedElementsIn, inSharedElements,
+ nameOverrides.values());
}
if (enterTransition == null && exitTransition == null && sharedElementTransition == null) {
@@ -523,6 +533,25 @@
}
/**
+ * Add Views from sharedElements into views that have the transitionName in the
+ * nameOverridesSet.
+ *
+ * @param views Views list to add shared elements to
+ * @param sharedElements List of shared elements
+ * @param nameOverridesSet The transition names for all views to be copied from
+ * sharedElements to views.
+ */
+ private static void addSharedElementsWithMatchingNames(ArrayList<View> views,
+ ArrayMap<String, View> sharedElements, Collection<String> nameOverridesSet) {
+ for (int i = sharedElements.size() - 1; i >= 0; i--) {
+ View view = sharedElements.valueAt(i);
+ if (nameOverridesSet.contains(ViewCompat.getTransitionName(view))) {
+ views.add(view);
+ }
+ }
+ }
+
+ /**
* Configures the shared elements of an unoptimized fragment transaction's transition.
* This retrieves the shared elements of the incoming fragments, and schedules capturing
* the incoming fragment's shared elements. It also maps the views, and sets up the epicenter
diff --git a/fragment/tests/java/android/support/v4/app/FragmentTransitionTest.java b/fragment/tests/java/android/support/v4/app/FragmentTransitionTest.java
index bc8e9e1..002e051 100644
--- a/fragment/tests/java/android/support/v4/app/FragmentTransitionTest.java
+++ b/fragment/tests/java/android/support/v4/app/FragmentTransitionTest.java
@@ -19,6 +19,7 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.verify;
@@ -592,6 +593,70 @@
verifyNoOtherTransitions(fragment2);
}
+ // Ensure that shared element without matching transition name doesn't error out
+ @Test
+ public void sharedElementMismatch() throws Throwable {
+ final TransitionFragment fragment1 = setupInitialFragment();
+
+ // Now do a transition to scene2
+ TransitionFragment fragment2 = new TransitionFragment();
+ fragment2.setLayoutId(R.layout.scene2);
+
+ final View startBlue = findBlue();
+ final View startGreen = findGreen();
+ final Rect startBlueBounds = getBoundsOnScreen(startBlue);
+
+ mFragmentManager.beginTransaction()
+ .addSharedElement(startBlue, "fooSquare")
+ .replace(R.id.fragmentContainer, fragment2)
+ .setAllowOptimization(mOptimize)
+ .addToBackStack(null)
+ .commit();
+ FragmentTestUtil.waitForExecution(mActivityRule);
+
+ fragment1.waitForTransition();
+ fragment2.waitForTransition();
+
+ final View endBlue = findBlue();
+ final View endGreen = findGreen();
+
+ if (mOptimize) {
+ verifyAndClearTransition(fragment1.exitTransition, null, startGreen, startBlue);
+ } else {
+ verifyAndClearTransition(fragment1.exitTransition, startBlueBounds, startGreen);
+ verifyAndClearTransition(fragment2.sharedElementEnter, startBlueBounds, startBlue);
+ }
+ verifyNoOtherTransitions(fragment1);
+
+ verifyAndClearTransition(fragment2.enterTransition, null, endGreen, endBlue);
+ verifyNoOtherTransitions(fragment2);
+ }
+
+ // Ensure that using the same source or target shared element results in an exception.
+ @Test
+ public void sharedDuplicateTargetNames() throws Throwable {
+ setupInitialFragment();
+
+ final View startBlue = findBlue();
+ final View startGreen = findGreen();
+
+ FragmentTransaction ft = mFragmentManager.beginTransaction();
+ ft.addSharedElement(startBlue, "blueSquare");
+ try {
+ ft.addSharedElement(startGreen, "blueSquare");
+ fail("Expected IllegalArgumentException");
+ } catch (IllegalArgumentException e) {
+ // expected
+ }
+
+ try {
+ ft.addSharedElement(startBlue, "greenSquare");
+ fail("Expected IllegalArgumentException");
+ } catch (IllegalArgumentException e) {
+ // expected
+ }
+ }
+
private TransitionFragment setupInitialFragment() throws Throwable {
TransitionFragment fragment1 = new TransitionFragment();
fragment1.setLayoutId(R.layout.scene1);
diff --git a/fragment/tests/java/android/support/v4/app/TransitionFragment.java b/fragment/tests/java/android/support/v4/app/TransitionFragment.java
index fda2784..e6493c2 100644
--- a/fragment/tests/java/android/support/v4/app/TransitionFragment.java
+++ b/fragment/tests/java/android/support/v4/app/TransitionFragment.java
@@ -51,9 +51,11 @@
setSharedElementEnterTransition(sharedElementEnter);
setSharedElementReturnTransition(sharedElementReturn);
enterTransition.addListener(mListener);
+ sharedElementEnter.addListener(mListener);
reenterTransition.addListener(mListener);
exitTransition.addListener(mListener);
returnTransition.addListener(mListener);
+ sharedElementReturn.addListener(mListener);
}
@Override