Commit 3f5187ae authored by Dan Harrington's avatar Dan Harrington Committed by Commit Bot

Fix crash on NTP

The problem was that we were handing the same view to RecyclerView
multiple times. We already wrap views in parent views before handing
them off to the recycler view. The fix is to always wrap the child
in a new parent.

Bug: 1131975
Change-Id: I6f1a92e3d8201d4395d9c5208d8289e961f450b3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2436420
Commit-Queue: Dan H <harringtond@chromium.org>
Reviewed-by: default avatarCathy Li <chili@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812978}
parent 2a1ff9be
......@@ -9,7 +9,6 @@ import android.view.LayoutInflater;
import android.view.View;
import android.view.ViewGroup;
import android.view.ViewGroup.LayoutParams;
import android.view.ViewParent;
import android.widget.FrameLayout;
import androidx.annotation.Nullable;
......@@ -19,6 +18,7 @@ import org.chromium.chrome.browser.xsurface.FeedActionsHandler;
import org.chromium.chrome.browser.xsurface.ListContentManager;
import org.chromium.chrome.browser.xsurface.ListContentManagerObserver;
import org.chromium.chrome.browser.xsurface.SurfaceActionsHandler;
import org.chromium.ui.UiUtils;
import java.util.ArrayList;
import java.util.Collections;
......@@ -114,11 +114,9 @@ public class FeedListContentManager implements ListContentManager {
// If there's already a parent, we have already enclosed this view previously.
// This can happen if a native view is added, removed, and added again.
ViewParent nativeViewParent = mNativeView.getParent();
if (nativeViewParent != null) {
assert nativeViewParent instanceof View;
return (View) nativeViewParent;
}
// In this case, it is important to make a new view because the RecyclerView
// may still have a reference to the old one. See crbug.com/1131975.
UiUtils.removeViewFromParent(mNativeView);
FrameLayout enclosingLayout = new FrameLayout(parent.getContext());
// Set the left and right margins.
......
......@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.feed.v2;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
......@@ -215,12 +216,12 @@ public class FeedListContentManagerTest implements ListContentManagerObserver {
ViewParent p = v.getParent();
// Remove and re-add the same view, but with a new NativeViewContent.
// This time, getNativeView() does not need to create the enclosing parent view.
// This time, getNativeView() creates a new enclosing parent view.
removeContents(0, 1);
c = createNativeViewContent(v);
addContents(0, ImmutableList.of(c));
assertEquals(v, getNativeView(mManager.getViewType(0)));
assertEquals(p, v.getParent());
assertNotEquals(p, v.getParent());
}
@Test
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment