Commit 83034cc5 authored by dgn's avatar dgn Committed by Commit bot

Tentative fix for scroll event related crashes on the NTP

A crash[1] seems to be caused by events attempting to get data from
the NTP while it is not the current page anymore. This CL adds
a check to ensure the page processing the events is the currently
displayed NTP.

[1]: https://crbug.com/649670#c17

BUG=649670

Review-Url: https://codereview.chromium.org/2400163002
Cr-Commit-Position: refs/heads/master@{#423858}
parent 2553af0f
...@@ -199,6 +199,11 @@ public class NewTabPage ...@@ -199,6 +199,11 @@ public class NewTabPage
* just tapped the fakebox. * just tapped the fakebox.
*/ */
void requestUrlFocusFromFakebox(String pastedText); void requestUrlFocusFromFakebox(String pastedText);
/**
* @return whether the provided native page is the one currently displayed to the user.
*/
boolean isCurrentPage(NativePage nativePage);
} }
/** /**
...@@ -661,6 +666,13 @@ public class NewTabPage ...@@ -661,6 +666,13 @@ public class NewTabPage
mSignInStateObserver = signInStateObserver; mSignInStateObserver = signInStateObserver;
SigninManager.get(mActivity).addSignInStateObserver(mSignInStateObserver); SigninManager.get(mActivity).addSignInStateObserver(mSignInStateObserver);
} }
@Override
public boolean isCurrentPage() {
if (mIsDestroyed) return false;
if (mFakeboxDelegate == null) return false;
return mFakeboxDelegate.isCurrentPage(NewTabPage.this);
}
}; };
/** /**
......
...@@ -300,6 +300,12 @@ public class NewTabPageView extends FrameLayout ...@@ -300,6 +300,12 @@ public class NewTabPageView extends FrameLayout
* Page goes away. * Page goes away.
*/ */
void registerSignInStateObserver(SignInStateObserver signInStateObserver); void registerSignInStateObserver(SignInStateObserver signInStateObserver);
/**
* @return whether the {@link NewTabPage} associated with this manager is the current page
* displayed to the user.
*/
boolean isCurrentPage();
} }
/** /**
...@@ -491,6 +497,12 @@ public class NewTabPageView extends FrameLayout ...@@ -491,6 +497,12 @@ public class NewTabPageView extends FrameLayout
private void updateSearchBoxOnScroll() { private void updateSearchBoxOnScroll() {
if (mDisableUrlFocusChangeAnimations) return; if (mDisableUrlFocusChangeAnimations) return;
// When the page changes (tab switching or new page loading), it is possible that events
// (e.g. delayed RecyclerView change notifications) trigger calls to these methods after
// the current page changes. We check it again to make sure we don't attempt to update the
// wrong page.
if (!mManager.isCurrentPage()) return;
if (mSearchBoxScrollListener != null) { if (mSearchBoxScrollListener != null) {
mSearchBoxScrollListener.onNtpScrollChanged(getToolbarTransitionPercentage()); mSearchBoxScrollListener.onNtpScrollChanged(getToolbarTransitionPercentage());
} }
......
...@@ -60,6 +60,7 @@ import org.chromium.base.metrics.RecordUserAction; ...@@ -60,6 +60,7 @@ import org.chromium.base.metrics.RecordUserAction;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.ChromeActivity; import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.ChromeSwitches; import org.chromium.chrome.browser.ChromeSwitches;
import org.chromium.chrome.browser.NativePage;
import org.chromium.chrome.browser.WindowDelegate; import org.chromium.chrome.browser.WindowDelegate;
import org.chromium.chrome.browser.appmenu.AppMenuButtonHelper; import org.chromium.chrome.browser.appmenu.AppMenuButtonHelper;
import org.chromium.chrome.browser.ntp.NativePageFactory; import org.chromium.chrome.browser.ntp.NativePageFactory;
...@@ -1134,6 +1135,12 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener, ...@@ -1134,6 +1135,12 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener,
} }
} }
@Override
public boolean isCurrentPage(NativePage nativePage) {
assert nativePage != null;
return nativePage == mToolbarDataProvider.getNewTabPageForCurrentTab();
}
@Override @Override
public void showUrlBarCursorWithoutFocusAnimations() { public void showUrlBarCursorWithoutFocusAnimations() {
if (mUrlHasFocus || mUrlFocusedFromFakebox) return; if (mUrlHasFocus || mUrlFocusedFromFakebox) return;
......
...@@ -358,5 +358,10 @@ public class ArticleSnippetsTest extends ChromeActivityTestCaseBase<ChromeActivi ...@@ -358,5 +358,10 @@ public class ArticleSnippetsTest extends ChromeActivityTestCaseBase<ChromeActivi
public void closeContextMenu() { public void closeContextMenu() {
throw new UnsupportedOperationException(); throw new UnsupportedOperationException();
} }
@Override
public boolean isCurrentPage() {
return true;
}
} }
} }
...@@ -9,8 +9,8 @@ import static org.junit.Assert.assertFalse; ...@@ -9,8 +9,8 @@ import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThat; import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail; import static org.junit.Assert.fail;
import static org.mockito.Mockito.any; import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.anyString; import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy; import static org.mockito.Mockito.spy;
...@@ -18,10 +18,6 @@ import static org.mockito.Mockito.times; ...@@ -18,10 +18,6 @@ import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when; import static org.mockito.Mockito.when;
import static org.chromium.base.test.util.Matchers.greaterThanOrEqualTo;
import static org.chromium.chrome.browser.ntp.cards.ContentSuggestionsTestUtils
.createDummySuggestions;
import android.support.annotation.Nullable; import android.support.annotation.Nullable;
import android.view.ContextMenu; import android.view.ContextMenu;
import android.view.Menu; import android.view.Menu;
...@@ -34,6 +30,10 @@ import org.junit.runner.RunWith; ...@@ -34,6 +30,10 @@ import org.junit.runner.RunWith;
import org.robolectric.RuntimeEnvironment; import org.robolectric.RuntimeEnvironment;
import org.robolectric.annotation.Config; import org.robolectric.annotation.Config;
import static org.chromium.base.test.util.Matchers.greaterThanOrEqualTo;
import static org.chromium.chrome.browser.ntp.cards.ContentSuggestionsTestUtils
.createDummySuggestions;
import org.chromium.base.Callback; import org.chromium.base.Callback;
import org.chromium.base.metrics.RecordHistogram; import org.chromium.base.metrics.RecordHistogram;
import org.chromium.base.metrics.RecordUserAction; import org.chromium.base.metrics.RecordUserAction;
...@@ -1049,5 +1049,10 @@ public class NewTabPageAdapterTest { ...@@ -1049,5 +1049,10 @@ public class NewTabPageAdapterTest {
public void registerSignInStateObserver(SignInStateObserver signInStateObserver) { public void registerSignInStateObserver(SignInStateObserver signInStateObserver) {
mSignInStateObserver = signInStateObserver; mSignInStateObserver = signInStateObserver;
} }
@Override
public boolean isCurrentPage() {
return true;
}
} }
} }
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