Commit c805e129 authored by dfalcantara's avatar dfalcantara Committed by Commit bot

[Download Home] More correctly track paused state

The Download Home frontend shouldn't be directly talking
to the Downloads backend because it's missing information
about the downloads that are stored only on the Java side.
Instead, consult the Java sort-of-backend-but-not-really
shared preferences to know whether or not a download is
actually paused or not.

BUG=678099,651408

Review-Url: https://codereview.chromium.org/2623373002
Cr-Commit-Position: refs/heads/master@{#443423}
parent 26ff4b57
...@@ -1540,12 +1540,14 @@ public class DownloadManagerService extends BroadcastReceiver implements ...@@ -1540,12 +1540,14 @@ public class DownloadManagerService extends BroadcastReceiver implements
@Override @Override
public void addDownloadHistoryAdapter(DownloadHistoryAdapter adapter) { public void addDownloadHistoryAdapter(DownloadHistoryAdapter adapter) {
mHistoryAdapters.addObserver(adapter); mHistoryAdapters.addObserver(adapter);
DownloadSharedPreferenceHelper.getInstance().addObserver(adapter);
} }
/** Removes a DownloadHistoryAdapter from the list. */ /** Removes a DownloadHistoryAdapter from the list. */
@Override @Override
public void removeDownloadHistoryAdapter(DownloadHistoryAdapter adapter) { public void removeDownloadHistoryAdapter(DownloadHistoryAdapter adapter) {
mHistoryAdapters.removeObserver(adapter); mHistoryAdapters.removeObserver(adapter);
DownloadSharedPreferenceHelper.getInstance().removeObserver(adapter);
} }
/** /**
......
...@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.download; ...@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.download;
import android.content.SharedPreferences; import android.content.SharedPreferences;
import org.chromium.base.ContextUtils; import org.chromium.base.ContextUtils;
import org.chromium.base.ObserverList;
import org.chromium.base.ThreadUtils; import org.chromium.base.ThreadUtils;
import org.chromium.base.VisibleForTesting; import org.chromium.base.VisibleForTesting;
...@@ -20,10 +21,18 @@ import java.util.Set; ...@@ -20,10 +21,18 @@ import java.util.Set;
* Class for maintaining all entries of DownloadSharedPreferenceEntry. * Class for maintaining all entries of DownloadSharedPreferenceEntry.
*/ */
public class DownloadSharedPreferenceHelper { public class DownloadSharedPreferenceHelper {
/** Observes modifications to the SharedPreferences for {@link DownloadItem}s. */
public interface Observer {
/** Called when a {@link DownloadSharedPreferenceEntry} has been updated. */
void onAddOrReplaceDownloadSharedPreferenceEntry(String guid);
}
@VisibleForTesting @VisibleForTesting
static final String KEY_PENDING_DOWNLOAD_NOTIFICATIONS = "PendingDownloadNotifications"; static final String KEY_PENDING_DOWNLOAD_NOTIFICATIONS = "PendingDownloadNotifications";
private final List<DownloadSharedPreferenceEntry> mDownloadSharedPreferenceEntries = private final List<DownloadSharedPreferenceEntry> mDownloadSharedPreferenceEntries =
new ArrayList<DownloadSharedPreferenceEntry>(); new ArrayList<DownloadSharedPreferenceEntry>();
private final ObserverList<Observer> mObservers = new ObserverList<>();
private SharedPreferences mSharedPrefs; private SharedPreferences mSharedPrefs;
// "Initialization on demand holder idiom" // "Initialization on demand holder idiom"
...@@ -64,6 +73,10 @@ public class DownloadSharedPreferenceHelper { ...@@ -64,6 +73,10 @@ public class DownloadSharedPreferenceHelper {
} }
mDownloadSharedPreferenceEntries.add(pendingEntry); mDownloadSharedPreferenceEntries.add(pendingEntry);
storeDownloadSharedPreferenceEntries(); storeDownloadSharedPreferenceEntries();
for (Observer observer : mObservers) {
observer.onAddOrReplaceDownloadSharedPreferenceEntry(pendingEntry.downloadGuid);
}
} }
/** /**
...@@ -129,6 +142,22 @@ public class DownloadSharedPreferenceHelper { ...@@ -129,6 +142,22 @@ public class DownloadSharedPreferenceHelper {
return null; return null;
} }
/**
* Adds the given {@link Observer}.
* @param observer Observer to notify about changes.
*/
public void addObserver(Observer observer) {
mObservers.addObserver(observer);
}
/**
* Removes the given {@link Observer}.
* @param observer Observer to stop notifying about changes.
*/
public void removeObserver(Observer observer) {
mObservers.removeObserver(observer);
}
/** /**
* Helper method to store all the SharedPreferences entries. * Helper method to store all the SharedPreferences entries.
*/ */
......
...@@ -45,6 +45,7 @@ import org.chromium.chrome.browser.tab.Tab; ...@@ -45,6 +45,7 @@ import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tabmodel.TabModel.TabLaunchType; import org.chromium.chrome.browser.tabmodel.TabModel.TabLaunchType;
import org.chromium.chrome.browser.tabmodel.document.TabDelegate; import org.chromium.chrome.browser.tabmodel.document.TabDelegate;
import org.chromium.chrome.browser.util.IntentUtils; import org.chromium.chrome.browser.util.IntentUtils;
import org.chromium.content_public.browser.DownloadState;
import org.chromium.content_public.browser.LoadUrlParams; import org.chromium.content_public.browser.LoadUrlParams;
import org.chromium.ui.base.DeviceFormFactor; import org.chromium.ui.base.DeviceFormFactor;
import org.chromium.ui.widget.Toast; import org.chromium.ui.widget.Toast;
...@@ -531,4 +532,54 @@ public class DownloadUtils { ...@@ -531,4 +532,54 @@ public class DownloadUtils {
return false; return false;
} }
/**
* Determine what String to show for a given download.
* @param item Download to check the status of.
* @return ID of a String resource to use, or 0 if the status couldn't be determined.
*/
public static int getStatusStringId(DownloadItem item) {
int state = item.getDownloadInfo().state();
if (state == DownloadState.COMPLETE) return R.string.download_notification_completed;
DownloadSharedPreferenceHelper helper = DownloadSharedPreferenceHelper.getInstance();
DownloadSharedPreferenceEntry entry = helper.getDownloadSharedPreferenceEntry(item.getId());
if (entry != null && state == DownloadState.INTERRUPTED && entry.isAutoResumable) {
return R.string.download_notification_pending;
} else if (isDownloadPaused(item)) {
return R.string.download_notification_paused;
} else if (state == DownloadState.IN_PROGRESS) {
return R.string.download_started;
} else {
assert false : "Unable to determine state of the download";
return 0;
}
}
/**
* Query the Download backends about whether a download is paused.
*
* The Java-side contains more information about the status of a download than is persisted
* by the native backend, so it is queried first.
*
* @param item Download to check the status of.
* @return Whether the download is paused or not.
*/
public static boolean isDownloadPaused(DownloadItem item) {
DownloadSharedPreferenceHelper helper = DownloadSharedPreferenceHelper.getInstance();
DownloadSharedPreferenceEntry entry = helper.getDownloadSharedPreferenceEntry(item.getId());
if (entry != null) {
// The Java downloads backend knows more about the download than the native backend.
return !entry.isAutoResumable;
} else {
// Only the native downloads backend knows about the download.
if (item.getDownloadInfo().state() == DownloadState.IN_PROGRESS) {
return item.getDownloadInfo().isPaused();
} else {
return item.getDownloadInfo().state() == DownloadState.INTERRUPTED;
}
}
}
} }
...@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.download.ui; ...@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.download.ui;
import android.content.ComponentName; import android.content.ComponentName;
import android.support.v7.widget.RecyclerView.ViewHolder; import android.support.v7.widget.RecyclerView.ViewHolder;
import android.text.TextUtils;
import android.view.LayoutInflater; import android.view.LayoutInflater;
import android.view.ViewGroup; import android.view.ViewGroup;
...@@ -14,6 +15,7 @@ import org.chromium.base.metrics.RecordHistogram; ...@@ -14,6 +15,7 @@ import org.chromium.base.metrics.RecordHistogram;
import org.chromium.base.metrics.RecordUserAction; import org.chromium.base.metrics.RecordUserAction;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.download.DownloadItem; import org.chromium.chrome.browser.download.DownloadItem;
import org.chromium.chrome.browser.download.DownloadSharedPreferenceHelper;
import org.chromium.chrome.browser.download.ui.BackendProvider.DownloadDelegate; import org.chromium.chrome.browser.download.ui.BackendProvider.DownloadDelegate;
import org.chromium.chrome.browser.download.ui.BackendProvider.OfflinePageDelegate; import org.chromium.chrome.browser.download.ui.BackendProvider.OfflinePageDelegate;
import org.chromium.chrome.browser.download.ui.DownloadHistoryItemWrapper.DownloadItemWrapper; import org.chromium.chrome.browser.download.ui.DownloadHistoryItemWrapper.DownloadItemWrapper;
...@@ -25,11 +27,13 @@ import org.chromium.chrome.browser.widget.DateDividedAdapter; ...@@ -25,11 +27,13 @@ import org.chromium.chrome.browser.widget.DateDividedAdapter;
import org.chromium.chrome.browser.widget.selection.SelectionDelegate; import org.chromium.chrome.browser.widget.selection.SelectionDelegate;
import org.chromium.content_public.browser.DownloadState; import org.chromium.content_public.browser.DownloadState;
import java.util.ArrayList;
import java.util.List; import java.util.List;
import java.util.Set; import java.util.Set;
/** Bridges the user's download history and the UI used to display it. */ /** Bridges the user's download history and the UI used to display it. */
public class DownloadHistoryAdapter extends DateDividedAdapter implements DownloadUiObserver { public class DownloadHistoryAdapter extends DateDividedAdapter
implements DownloadUiObserver, DownloadSharedPreferenceHelper.Observer {
/** Alerted about changes to internal state. */ /** Alerted about changes to internal state. */
static interface TestObserver { static interface TestObserver {
...@@ -71,6 +75,7 @@ public class DownloadHistoryAdapter extends DateDividedAdapter implements Downlo ...@@ -71,6 +75,7 @@ public class DownloadHistoryAdapter extends DateDividedAdapter implements Downlo
private final boolean mShowOffTheRecord; private final boolean mShowOffTheRecord;
private final LoadingStateDelegate mLoadingDelegate; private final LoadingStateDelegate mLoadingDelegate;
private final ObserverList<TestObserver> mObservers = new ObserverList<>(); private final ObserverList<TestObserver> mObservers = new ObserverList<>();
private final List<DownloadItemView> mViews = new ArrayList<>();
private BackendProvider mBackendProvider; private BackendProvider mBackendProvider;
private OfflinePageDownloadBridge.Observer mOfflinePageObserver; private OfflinePageDownloadBridge.Observer mOfflinePageObserver;
...@@ -204,6 +209,7 @@ public class DownloadHistoryAdapter extends DateDividedAdapter implements Downlo ...@@ -204,6 +209,7 @@ public class DownloadHistoryAdapter extends DateDividedAdapter implements Downlo
DownloadItemView v = (DownloadItemView) LayoutInflater.from(parent.getContext()).inflate( DownloadItemView v = (DownloadItemView) LayoutInflater.from(parent.getContext()).inflate(
R.layout.download_item_view, parent, false); R.layout.download_item_view, parent, false);
v.setSelectionDelegate(getSelectionDelegate()); v.setSelectionDelegate(getSelectionDelegate());
mViews.add(v);
return new DownloadHistoryItemViewHolder(v); return new DownloadHistoryItemViewHolder(v);
} }
...@@ -300,6 +306,16 @@ public class DownloadHistoryAdapter extends DateDividedAdapter implements Downlo ...@@ -300,6 +306,16 @@ public class DownloadHistoryAdapter extends DateDividedAdapter implements Downlo
sDeletedFileTracker.decrementInstanceCount(); sDeletedFileTracker.decrementInstanceCount();
} }
@Override
public void onAddOrReplaceDownloadSharedPreferenceEntry(final String guid) {
// Alert DownloadItemViews displaying information about the item that it has changed.
for (DownloadItemView view : mViews) {
if (TextUtils.equals(guid, view.getItem().getId())) {
view.displayItem(mBackendProvider, view.getItem());
}
}
}
/** Marks that certain items are about to be deleted. */ /** Marks that certain items are about to be deleted. */
void markItemsForDeletion(List<DownloadHistoryItemWrapper> items) { void markItemsForDeletion(List<DownloadHistoryItemWrapper> items) {
for (DownloadHistoryItemWrapper item : items) item.setIsDeletionPending(true); for (DownloadHistoryItemWrapper item : items) item.setIsDeletionPending(true);
......
...@@ -160,9 +160,6 @@ public abstract class DownloadHistoryItemWrapper extends TimedItem { ...@@ -160,9 +160,6 @@ public abstract class DownloadHistoryItemWrapper extends TimedItem {
/** @return Whether the download is currently paused. */ /** @return Whether the download is currently paused. */
abstract boolean isPaused(); abstract boolean isPaused();
/** @return Whether the download can be resumed. */
abstract boolean isResumable();
/** Called when the user wants to open the file. */ /** Called when the user wants to open the file. */
abstract void open(); abstract void open();
...@@ -305,19 +302,7 @@ public abstract class DownloadHistoryItemWrapper extends TimedItem { ...@@ -305,19 +302,7 @@ public abstract class DownloadHistoryItemWrapper extends TimedItem {
@Override @Override
public int getStatusString() { public int getStatusString() {
int state = mItem.getDownloadInfo().state(); return DownloadUtils.getStatusStringId(mItem);
if (mItem.getDownloadInfo().isPaused()) {
// TODO(dfalcantara): Account for paused downloads after restarting Chrome.
return R.string.download_notification_paused;
} else if (state == DownloadState.INTERRUPTED) {
return R.string.download_notification_pending;
} else if (state == DownloadState.COMPLETE) {
return R.string.download_notification_completed;
} else if (state == DownloadState.IN_PROGRESS) {
return R.string.download_started;
} else {
return 0;
}
} }
@Override @Override
...@@ -379,12 +364,7 @@ public abstract class DownloadHistoryItemWrapper extends TimedItem { ...@@ -379,12 +364,7 @@ public abstract class DownloadHistoryItemWrapper extends TimedItem {
@Override @Override
public boolean isPaused() { public boolean isPaused() {
return mItem.getDownloadInfo().isPaused(); return DownloadUtils.isDownloadPaused(mItem);
}
@Override
public boolean isResumable() {
return mItem.getDownloadInfo().isResumable();
} }
@Override @Override
...@@ -554,10 +534,5 @@ public abstract class DownloadHistoryItemWrapper extends TimedItem { ...@@ -554,10 +534,5 @@ public abstract class DownloadHistoryItemWrapper extends TimedItem {
public boolean isPaused() { public boolean isPaused() {
return false; return false;
} }
@Override
public boolean isResumable() {
return false;
}
} }
} }
...@@ -91,7 +91,7 @@ public class DownloadItemView extends SelectableItemView<DownloadHistoryItemWrap ...@@ -91,7 +91,7 @@ public class DownloadItemView extends SelectableItemView<DownloadHistoryItemWrap
mPauseResumeButton.setOnClickListener(new View.OnClickListener() { mPauseResumeButton.setOnClickListener(new View.OnClickListener() {
@Override @Override
public void onClick(View v) { public void onClick(View v) {
if (mItem.isResumable()) { if (mItem.isPaused()) {
mItem.resume(); mItem.resume();
} else if (!mItem.isComplete()) { } else if (!mItem.isComplete()) {
mItem.pause(); mItem.pause();
...@@ -133,14 +133,6 @@ public class DownloadItemView extends SelectableItemView<DownloadHistoryItemWrap ...@@ -133,14 +133,6 @@ public class DownloadItemView extends SelectableItemView<DownloadHistoryItemWrap
ThumbnailProvider thumbnailProvider = provider.getThumbnailProvider(); ThumbnailProvider thumbnailProvider = provider.getThumbnailProvider();
thumbnailProvider.cancelRetrieval(this); thumbnailProvider.cancelRetrieval(this);
Context context = mFilesizeView.getContext();
mFilenameCompletedView.setText(item.getDisplayFileName());
mFilenameInProgressView.setText(item.getDisplayFileName());
mHostnameView.setText(
UrlFormatter.formatUrlForSecurityDisplay(item.getUrl(), false));
mFilesizeView.setText(
Formatter.formatFileSize(context, item.getFileSize()));
// Asynchronously grab a thumbnail for the file if it might have one. // Asynchronously grab a thumbnail for the file if it might have one.
int fileType = item.getFilterType(); int fileType = item.getFilterType();
mThumbnailBitmap = null; mThumbnailBitmap = null;
...@@ -173,13 +165,21 @@ public class DownloadItemView extends SelectableItemView<DownloadHistoryItemWrap ...@@ -173,13 +165,21 @@ public class DownloadItemView extends SelectableItemView<DownloadHistoryItemWrap
updateIconView(); updateIconView();
Context context = mFilesizeView.getContext();
mFilenameCompletedView.setText(item.getDisplayFileName());
mFilenameInProgressView.setText(item.getDisplayFileName());
mHostnameView.setText(
UrlFormatter.formatUrlForSecurityDisplay(item.getUrl(), false));
mFilesizeView.setText(
Formatter.formatFileSize(context, item.getFileSize()));
if (item.isComplete()) { if (item.isComplete()) {
showLayout(mLayoutCompleted); showLayout(mLayoutCompleted);
} else { } else {
showLayout(mLayoutInProgress); showLayout(mLayoutInProgress);
mDownloadStatusView.setText(item.getStatusString()); mDownloadStatusView.setText(item.getStatusString());
if (item.isResumable() || item.isPaused()) { if (item.isPaused()) {
mPauseResumeButton.setImageResource(R.drawable.ic_media_control_play); mPauseResumeButton.setImageResource(R.drawable.ic_media_control_play);
mPauseResumeButton.setContentDescription( mPauseResumeButton.setContentDescription(
getContext().getString(R.string.download_notification_resume_button)); getContext().getString(R.string.download_notification_resume_button));
......
...@@ -93,7 +93,14 @@ public class DownloadHistoryAdapterTest extends NativeLibraryTestBase { ...@@ -93,7 +93,14 @@ public class DownloadHistoryAdapterTest extends NativeLibraryTestBase {
initializeAdapter(false); initializeAdapter(false);
assertEquals(0, mAdapter.getItemCount()); assertEquals(0, mAdapter.getItemCount());
assertEquals(0, mAdapter.getTotalDownloadSize()); assertEquals(0, mAdapter.getTotalDownloadSize());
ThreadUtils.runOnUiThreadBlocking(new Runnable() {
@Override
public void run() {
mAdapter.onManagerDestroyed(); mAdapter.onManagerDestroyed();
}
});
mDownloadDelegate.removeCallback.waitForCallback(0); mDownloadDelegate.removeCallback.waitForCallback(0);
} }
......
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