Commit 490b8678 authored by Jinsuk Kim's avatar Jinsuk Kim Committed by Commit Bot

gesturenav: Fix a crash on swipes after destroy

Triggering requests may come after the action delegate is nulled out
when the activity is being destroyed, which can cause crashes for
swipes on the rendered pages. This CL checks the situation and ignores
the requests accordingly.

Bug: 1131424
Change-Id: I4914ef220638ea69cf1522aa2ac974c73ed89ece
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2428512
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Reviewed-by: default avatarDonn Denman <donnd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811045}
parent bc1c3411
...@@ -224,6 +224,9 @@ class NavigationHandler implements TouchEventObserver { ...@@ -224,6 +224,9 @@ class NavigationHandler implements TouchEventObserver {
* @see {@link HistoryNavigationCoordinator#triggerUi(boolean, float, float)} * @see {@link HistoryNavigationCoordinator#triggerUi(boolean, float, float)}
*/ */
boolean triggerUi(boolean forward, float x, float y) { boolean triggerUi(boolean forward, float x, float y) {
// Triggering requests may come after the action delegate is nulled out
// when the activity is being destroyed. Ignore it.
if (mActionDelegate == null) return false;
mModel.set(DIRECTION, forward); mModel.set(DIRECTION, forward);
boolean navigable = mActionDelegate.canNavigate(forward); boolean navigable = mActionDelegate.canNavigate(forward);
if (navigable) { if (navigable) {
......
...@@ -71,11 +71,7 @@ public class NavigationHandlerTest { ...@@ -71,11 +71,7 @@ public class NavigationHandlerTest {
mActivityTestRule.getActivity().getWindowManager().getDefaultDisplay().getMetrics( mActivityTestRule.getActivity().getWindowManager().getDefaultDisplay().getMetrics(
displayMetrics); displayMetrics);
mEdgeWidthPx = displayMetrics.density * NavigationHandler.EDGE_WIDTH_DP; mEdgeWidthPx = displayMetrics.density * NavigationHandler.EDGE_WIDTH_DP;
TabbedRootUiCoordinator uiCoordinator = HistoryNavigationCoordinator coordinator = getNavigationCoordinator();
(TabbedRootUiCoordinator) mActivityTestRule.getActivity()
.getRootUiCoordinatorForTesting();
HistoryNavigationCoordinator coordinator =
uiCoordinator.getHistoryNavigationCoordinatorForTesting();
mNavigationLayout = coordinator.getLayoutForTesting(); mNavigationLayout = coordinator.getLayoutForTesting();
mNavigationHandler = coordinator.getNavigationHandlerForTesting(); mNavigationHandler = coordinator.getNavigationHandlerForTesting();
} }
...@@ -85,6 +81,13 @@ public class NavigationHandlerTest { ...@@ -85,6 +81,13 @@ public class NavigationHandlerTest {
if (mTestServer != null) mTestServer.stopAndDestroyServer(); if (mTestServer != null) mTestServer.stopAndDestroyServer();
} }
private HistoryNavigationCoordinator getNavigationCoordinator() {
TabbedRootUiCoordinator uiCoordinator =
(TabbedRootUiCoordinator) mActivityTestRule.getActivity()
.getRootUiCoordinatorForTesting();
return uiCoordinator.getHistoryNavigationCoordinatorForTesting();
}
private Tab currentTab() { private Tab currentTab() {
return mActivityTestRule.getActivity().getActivityTabProvider().get(); return mActivityTestRule.getActivity().getActivityTabProvider().get();
} }
...@@ -236,6 +239,24 @@ public class NavigationHandlerTest { ...@@ -236,6 +239,24 @@ public class NavigationHandlerTest {
ApplicationStatus.getStateForActivity(mActivityTestRule.getActivity())); ApplicationStatus.getStateForActivity(mActivityTestRule.getActivity()));
} }
@Test
@SmallTest
public void testSwipeAfterDestroy() {
mTestServer = EmbeddedTestServer.createAndStartServer(
InstrumentationRegistry.getInstrumentation().getContext());
mActivityTestRule.loadUrl(mTestServer.getURL(RENDERED_PAGE));
getNavigationCoordinator().destroy();
// |triggerUi| can be invoked by SwipeRefreshHandler on the rendered
// page. Make sure this won't crash after the coordinator (and also
// handler action delegate) is destroyed.
assert !mNavigationHandler.triggerUi(LEFT_EDGE, 0, 0);
// Just check we're still on the same URL.
Assert.assertEquals(mTestServer.getURL(RENDERED_PAGE),
ChromeTabUtils.getUrlStringOnUiThread(currentTab()));
}
@Test @Test
@SmallTest @SmallTest
@Restriction(UiRestriction.RESTRICTION_TYPE_PHONE) @Restriction(UiRestriction.RESTRICTION_TYPE_PHONE)
......
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