Commit 2b002979 authored by Shakti Sahu's avatar Shakti Sahu Committed by Commit Bot

Download Home : Back button should clear back stack

This CL fixes the issue of long backstack for download home filters. With
this fix, we would always exit download home when back pressed. While we
are on download home, selecting filters will only update the current
navigation entry instead of creating a new entry.

Bug: 899938
Change-Id: Ia532d1441ff096a78fdc4cb509b4a99ee413ee3f
Reviewed-on: https://chromium-review.googlesource.com/c/1309428
Commit-Queue: Shakti Sahu <shaktisahu@chromium.org>
Reviewed-by: default avatarTheresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605930}
parent 4be2a193
...@@ -347,7 +347,7 @@ public class BookmarkManager implements BookmarkDelegate, SearchDelegate, ...@@ -347,7 +347,7 @@ public class BookmarkManager implements BookmarkDelegate, SearchDelegate,
BookmarkUtils.setLastUsedUrl(mActivity, state.mUrl); BookmarkUtils.setLastUsedUrl(mActivity, state.mUrl);
// If a loading state is replaced by another loading state, do not notify this change. // If a loading state is replaced by another loading state, do not notify this change.
if (mNativePage != null) { if (mNativePage != null) {
mNativePage.onStateChange(state.mUrl); mNativePage.onStateChange(state.mUrl, false);
} }
} }
......
...@@ -11,10 +11,10 @@ import android.os.Bundle; ...@@ -11,10 +11,10 @@ import android.os.Bundle;
import org.chromium.base.VisibleForTesting; import org.chromium.base.VisibleForTesting;
import org.chromium.chrome.browser.IntentHandler; import org.chromium.chrome.browser.IntentHandler;
import org.chromium.chrome.browser.SnackbarActivity; import org.chromium.chrome.browser.SnackbarActivity;
import org.chromium.chrome.browser.UrlConstants;
import org.chromium.chrome.browser.download.home.DownloadManagerCoordinator; import org.chromium.chrome.browser.download.home.DownloadManagerCoordinator;
import org.chromium.chrome.browser.download.home.DownloadManagerCoordinatorFactory; import org.chromium.chrome.browser.download.home.DownloadManagerCoordinatorFactory;
import org.chromium.chrome.browser.download.home.DownloadManagerUiConfig; import org.chromium.chrome.browser.download.home.DownloadManagerUiConfig;
import org.chromium.chrome.browser.download.home.filter.Filters;
import org.chromium.chrome.browser.download.items.OfflineContentAggregatorNotificationBridgeUiFactory; import org.chromium.chrome.browser.download.items.OfflineContentAggregatorNotificationBridgeUiFactory;
import org.chromium.chrome.browser.download.ui.DownloadManagerUi; import org.chromium.chrome.browser.download.ui.DownloadManagerUi;
import org.chromium.chrome.browser.util.IntentUtils; import org.chromium.chrome.browser.util.IntentUtils;
...@@ -22,25 +22,25 @@ import org.chromium.ui.base.ActivityAndroidPermissionDelegate; ...@@ -22,25 +22,25 @@ import org.chromium.ui.base.ActivityAndroidPermissionDelegate;
import org.chromium.ui.base.AndroidPermissionDelegate; import org.chromium.ui.base.AndroidPermissionDelegate;
import java.lang.ref.WeakReference; import java.lang.ref.WeakReference;
import java.util.Deque;
import java.util.LinkedList;
/** /**
* Activity for managing downloads handled through Chrome. * Activity for managing downloads handled through Chrome.
*/ */
public class DownloadActivity extends SnackbarActivity { public class DownloadActivity extends SnackbarActivity {
private static final String BUNDLE_KEY_CURRENT_URL = "current_url";
private DownloadManagerCoordinator mDownloadCoordinator; private DownloadManagerCoordinator mDownloadCoordinator;
private boolean mIsOffTheRecord; private boolean mIsOffTheRecord;
private AndroidPermissionDelegate mPermissionDelegate; private AndroidPermissionDelegate mPermissionDelegate;
/** Caches the stack of filters applied to let the user backtrack through their history. */ /** Caches the current URL for the filter being applied. */
private final Deque<String> mBackStack = new LinkedList<>(); private String mCurrentUrl;
private final DownloadManagerCoordinator.Observer mUiObserver = private final DownloadManagerCoordinator.Observer mUiObserver =
new DownloadManagerCoordinator.Observer() { new DownloadManagerCoordinator.Observer() {
@Override @Override
public void onUrlChanged(String url) { public void onUrlChanged(String url) {
if (!url.equals(mBackStack.peek())) mBackStack.push(url); mCurrentUrl = url;
} }
}; };
...@@ -64,9 +64,21 @@ public class DownloadActivity extends SnackbarActivity { ...@@ -64,9 +64,21 @@ public class DownloadActivity extends SnackbarActivity {
setContentView(mDownloadCoordinator.getView()); setContentView(mDownloadCoordinator.getView());
mIsOffTheRecord = isOffTheRecord; mIsOffTheRecord = isOffTheRecord;
mDownloadCoordinator.addObserver(mUiObserver); mDownloadCoordinator.addObserver(mUiObserver);
// Call updateForUrl() to align with how DownloadPage interacts with DownloadManagerUi.
mDownloadCoordinator.updateForUrl(UrlConstants.DOWNLOADS_URL); if (savedInstanceState != null) {
if (showPrefetchContent) mDownloadCoordinator.showPrefetchSection(); mCurrentUrl = savedInstanceState.getString(BUNDLE_KEY_CURRENT_URL);
} else {
mCurrentUrl = Filters.toUrl(
showPrefetchContent ? Filters.FilterType.PREFETCHED : Filters.FilterType.NONE);
}
mDownloadCoordinator.updateForUrl(mCurrentUrl);
}
@Override
protected void onSaveInstanceState(Bundle outState) {
super.onSaveInstanceState(outState);
if (mCurrentUrl != null) outState.putString(BUNDLE_KEY_CURRENT_URL, mCurrentUrl);
} }
@Override @Override
...@@ -78,17 +90,8 @@ public class DownloadActivity extends SnackbarActivity { ...@@ -78,17 +90,8 @@ public class DownloadActivity extends SnackbarActivity {
@Override @Override
public void onBackPressed() { public void onBackPressed() {
if (mDownloadCoordinator.onBackPressed()) return; if (mDownloadCoordinator.onBackPressed()) return;
// The top of the stack always represents the current filter. When back is pressed,
// the top is popped off and the new top indicates what filter to use. If there are
// no filters remaining, the Activity itself is closed.
if (mBackStack.size() > 1) {
mBackStack.pop();
mDownloadCoordinator.updateForUrl(mBackStack.peek());
} else {
if (!mBackStack.isEmpty()) mBackStack.pop();
super.onBackPressed(); super.onBackPressed();
} }
}
@Override @Override
protected void onDestroy() { protected void onDestroy() {
......
...@@ -98,6 +98,10 @@ public class DownloadPage extends BasicNativePage implements DownloadManagerCoor ...@@ -98,6 +98,10 @@ public class DownloadPage extends BasicNativePage implements DownloadManagerCoor
// DownloadManagerCoordinator.Observer implementation. // DownloadManagerCoordinator.Observer implementation.
@Override @Override
public void onUrlChanged(String url) { public void onUrlChanged(String url) {
onStateChange(url); // We want to squash consecutive download home URLs having different filters into the one
// having the latest filter. This will avoid requiring user to press back button too many
// times to exit download home. In the event, chrome gets killed or if user navigates away
// from download home, we still will be able to come back to the latest filter.
onStateChange(url, true);
} }
} }
...@@ -418,7 +418,7 @@ public class DownloadManagerUi implements OnMenuItemClickListener, SearchDelegat ...@@ -418,7 +418,7 @@ public class DownloadManagerUi implements OnMenuItemClickListener, SearchDelegat
for (Observer observer : mObservers) observer.onUrlChanged(url); for (Observer observer : mObservers) observer.onUrlChanged(url);
if (mNativePage != null) { if (mNativePage != null) {
mNativePage.onStateChange(DownloadFilter.getUrlForFilter(filter)); mNativePage.onStateChange(DownloadFilter.getUrlForFilter(filter), true);
} }
RecordHistogram.recordEnumeratedHistogram( RecordHistogram.recordEnumeratedHistogram(
......
...@@ -106,10 +106,14 @@ public abstract class BasicNativePage extends EmptyTabObserver implements Native ...@@ -106,10 +106,14 @@ public abstract class BasicNativePage extends EmptyTabObserver implements Native
/** /**
* Tells the native page framework that the url should be changed. * Tells the native page framework that the url should be changed.
* @param url The URL of the page.
* @param replaceLastUrl Whether the last navigation entry should be replaced with the new URL.
*/ */
public void onStateChange(String url) { public void onStateChange(String url, boolean replaceLastUrl) {
if (url.equals(mUrl)) return; if (url.equals(mUrl)) return;
mHost.loadUrl(new LoadUrlParams(url), /* incognito = */ false); LoadUrlParams params = new LoadUrlParams(url);
params.setShouldReplaceCurrentEntry(replaceLastUrl);
mHost.loadUrl(params, /* incognito = */ false);
} }
/** /**
......
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