Skip to content

GitLab

T
tangled
Commit 83882ed9 authored by Shakti Sahu's avatar Shakti Sahu Committed by Commit Bot
Browse files
Options

Deprecating some usages of download snackbar in favor of infobar 

This CL removes some usages of download snack bar since essentially download
infobar has replaced it for showing download progress. However there are
certain failure messages that are not covered which are not exactly related
to an active download. For those messages, snack bar will continue to show.

Bug: 1003153
Change-Id: I97d626ca69b77cacb2bfc52390c9b1dadf49a65e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1799924Reviewed-by: default avatarShakti Sahu <shaktisahu@chromium.org>
Reviewed-by: default avatarMin Qin <qinmin@chromium.org>
Commit-Queue: Shakti Sahu <shaktisahu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#696961}
parent 974f14a7
Hide whitespace changes
Inline Side-by-side
Showing with 25 additions and 180 deletions
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadInfoBarController.java
View file @ 83882ed9
......@@ -361,6 +361,11 @@ public class DownloadInfoBarController implements OfflineContentProvider.Observe
computeNextStepForUpdate(item);
}
/** @return Whether the infobar is currently showing. */
public boolean isShowing() {
return mCurrentInfoBar != null;
}
/**
* Helper method to get the parameters for showing accelerated download infobar IPH.
* @return The UI parameters to show IPH, if an IPH should be shown, null otherwise.
......
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java
View file @ 83882ed9
......@@ -317,8 +317,7 @@ public class DownloadManagerService
mUpdateDelayInMillis = updateDelayInMillis;
mHandler = handler;
mDownloadSnackbarController = new DownloadSnackbarController();
mOMADownloadHandler =
new OMADownloadHandler(applicationContext, mDownloadSnackbarController);
mOMADownloadHandler = new OMADownloadHandler(applicationContext);
// Note that this technically leaks the native object, however, DownloadManagerService
// is a singleton that lives forever and there's no clean shutdown of Chrome on Android.
init();
......@@ -553,7 +552,6 @@ public class DownloadManagerService
// TODO(cmsy): Use correct FailState.
mDownloadNotifier.notifyDownloadFailed(info);
Log.w(TAG, "Download failed: " + info.getFilePath());
onDownloadFailed(item, DownloadManager.ERROR_UNKNOWN);
break;
case DownloadStatus.IN_PROGRESS:
if (info.isPaused()) {
......@@ -629,7 +627,6 @@ public class DownloadManagerService
.build();
mDownloadNotifier.notifyDownloadFailed(info);
// TODO(qinmin): get the failure message from native.
onDownloadFailed(item, DownloadManager.ERROR_UNKNOWN);
maybeRecordBackgroundDownload(
UmaBackgroundDownload.FAILED, info.getDownloadGuid());
}
......@@ -833,13 +830,7 @@ public class DownloadManagerService
return;
}
// TODO(shaktisahu): We should show this on infobar instead.
if (FeatureUtilities.isDownloadProgressInfoBarEnabled()) {
getInfoBarController(downloadItem.getDownloadInfo().isOffTheRecord())
.onDownloadStarted();
} else {
DownloadUtils.showDownloadStartToast(ContextUtils.getApplicationContext());
}
getInfoBarController(downloadItem.getDownloadInfo().isOffTheRecord()).onDownloadStarted();
addUmaStatsEntry(new DownloadUmaStatsEntry(String.valueOf(response.downloadId),
downloadItem.getStartTime(), 0, false, true, 0, 0));
}
......@@ -1023,8 +1014,6 @@ public class DownloadManagerService
protected void onDownloadFailed(DownloadItem item, int reason) {
String failureMessage =
getDownloadFailureMessage(item.getDownloadInfo().getFileName(), reason);
// TODO(shaktisahu): Notify infobar controller of the failure.
if (FeatureUtilities.isDownloadProgressInfoBarEnabled()) return;
if (mDownloadSnackbarController.getSnackbarManager() != null) {
mDownloadSnackbarController.onDownloadFailed(
......@@ -1286,8 +1275,6 @@ public class DownloadManagerService
if (infobarController != null) {
infobarController.onNotificationShown(info.getContentId(), notificationId);
}
mDownloadSnackbarController.onDownloadSucceeded(
info, notificationId, systemDownloadId, canResolve, false);
}
} else {
if (getInfoBarController(info.isOffTheRecord()) != null) {
......@@ -1425,10 +1412,6 @@ public class DownloadManagerService
.onItemUpdated(DownloadInfo.createOfflineItem(
item.getDownloadInfo()),
null);
mDownloadSnackbarController.onDownloadSucceeded(
item.getDownloadInfo(),
DownloadSnackbarController.INVALID_NOTIFICATION_ID,
item.getSystemDownloadId(), canResolve, true);
}
}
}.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
......
chrome/android/java/src/org/chromium/chrome/browser/download/DownloadSnackbarController.java
View file @ 83882ed9
......@@ -6,18 +6,15 @@ package org.chromium.chrome.browser.download;
import android.app.Activity;
import android.app.DownloadManager;
import android.content.Context;
import android.content.Intent;
import org.chromium.base.ApplicationStatus;
import org.chromium.base.BuildInfo;
import org.chromium.base.ContextUtils;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.customtabs.CustomTabActivity;
import org.chromium.chrome.browser.download.items.OfflineContentAggregatorNotificationBridgeUiFactory;
import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.snackbar.Snackbar;
import org.chromium.chrome.browser.snackbar.SnackbarManager;
import org.chromium.chrome.browser.util.FeatureUtilities;
import org.chromium.components.offline_items_collection.LegacyHelpers;
/**
......@@ -76,46 +73,6 @@ public class DownloadSnackbarController implements SnackbarManager.SnackbarContr
public void onDismissNoAction(Object actionData) {
}
/**
* Called to display the download succeeded snackbar.
*
* @param downloadInfo Info of the download.
* @param notificationId Notification Id of the successful download.
* @param downloadId Id of the download from Android DownloadManager.
* @param canBeResolved Whether the download can be resolved to any activity.
* @param usesAndroidDownloadManager Whether the download uses Android DownloadManager.
*/
public void onDownloadSucceeded(
DownloadInfo downloadInfo, int notificationId, long downloadId, boolean canBeResolved,
boolean usesAndroidDownloadManager) {
Context appContext = ContextUtils.getApplicationContext();
if (FeatureUtilities.isDownloadProgressInfoBarEnabled()) return;
if (getSnackbarManager() == null) return;
Snackbar snackbar;
if (getActivity() instanceof CustomTabActivity) {
String packageLabel = BuildInfo.getInstance().hostPackageLabel;
snackbar = Snackbar.make(appContext.getString(R.string.download_succeeded_message,
downloadInfo.getFileName(), packageLabel),
this, Snackbar.TYPE_NOTIFICATION, Snackbar.UMA_DOWNLOAD_SUCCEEDED);
} else {
snackbar =
Snackbar.make(appContext.getString(R.string.download_succeeded_message_default,
downloadInfo.getFileName()),
this, Snackbar.TYPE_NOTIFICATION, Snackbar.UMA_DOWNLOAD_SUCCEEDED);
}
// TODO(qinmin): Coalesce snackbars if multiple downloads finish at the same time.
snackbar.setDuration(SNACKBAR_DURATION_MS).setSingleLine(false);
ActionDataInfo info = null;
if (canBeResolved || !LegacyHelpers.isLegacyDownload(downloadInfo.getContentId())
|| usesAndroidDownloadManager) {
info = new ActionDataInfo(downloadInfo, notificationId, downloadId,
usesAndroidDownloadManager);
}
// Show downloads app if the download cannot be resolved to any activity.
snackbar.setAction(appContext.getString(R.string.open_downloaded_label), info);
getSnackbarManager().showSnackbar(snackbar);
}
/**
* Called to display the download failed snackbar.
*
......@@ -124,7 +81,7 @@ public class DownloadSnackbarController implements SnackbarManager.SnackbarContr
* duplicated files.
*/
public void onDownloadFailed(String errorMessage, boolean showAllDownloads) {
if (FeatureUtilities.isDownloadProgressInfoBarEnabled()) return;
if (isShowingDownloadInfoBar()) return;
if (getSnackbarManager() == null) return;
// TODO(qinmin): Coalesce snackbars if multiple downloads finish at the same time.
Snackbar snackbar = Snackbar.make(errorMessage, this, Snackbar.TYPE_NOTIFICATION,
......@@ -170,4 +127,10 @@ public class DownloadSnackbarController implements SnackbarManager.SnackbarContr
}
return null;
}
private boolean isShowingDownloadInfoBar() {
return DownloadManagerService.getDownloadManagerService()
.getInfoBarController(Profile.getLastUsedProfile().isOffTheRecord())
.isShowing();
}
}
chrome/android/java/src/org/chromium/chrome/browser/download/OMADownloadHandler.java
View file @ 83882ed9
......@@ -42,7 +42,6 @@ import org.chromium.chrome.browser.content.ContentUtils;
import org.chromium.chrome.browser.download.DownloadManagerBridge.DownloadEnqueueRequest;
import org.chromium.chrome.browser.download.DownloadManagerBridge.DownloadEnqueueResponse;
import org.chromium.chrome.browser.download.items.OfflineContentAggregatorFactory;
import org.chromium.chrome.browser.util.FeatureUtilities;
import org.chromium.chrome.download.R;
import org.chromium.components.download.DownloadCollectionBridge;
import org.chromium.components.offline_items_collection.LegacyHelpers;
......@@ -131,7 +130,6 @@ public class OMADownloadHandler extends BroadcastReceiver {
new LongSparseArray<DownloadItem>();
private final LongSparseArray<OMAInfo> mPendingOMADownloads =
new LongSparseArray<OMAInfo>();
private final DownloadSnackbarController mDownloadSnackbarController;
private final ObserverList<TestObserver> mObservers = new ObserverList<>();
/**
......@@ -257,11 +255,10 @@ public class OMADownloadHandler extends BroadcastReceiver {
}
}
public OMADownloadHandler(
Context context, DownloadSnackbarController downloadSnackbarController) {
/** Constructor. */
public OMADownloadHandler(Context context) {
mContext = context;
mSharedPrefs = ContextUtils.getAppSharedPreferences();
mDownloadSnackbarController = downloadSnackbarController;
}
/**
......@@ -863,25 +860,13 @@ public class OMADownloadHandler extends BroadcastReceiver {
if (result.downloadStatus == DownloadManagerService.DownloadStatus.COMPLETE) {
onDownloadCompleted(item.getDownloadInfo(), downloadId, installNotifyURI);
removeOMADownloadFromSharedPrefs(downloadId);
if (FeatureUtilities.isDownloadProgressInfoBarEnabled()) {
showDownloadOnInfoBar(item, result.downloadStatus);
} else {
mDownloadSnackbarController.onDownloadSucceeded(item.getDownloadInfo(),
DownloadSnackbarController.INVALID_NOTIFICATION_ID, downloadId,
canResolve, true);
}
showDownloadOnInfoBar(item, result.downloadStatus);
} else if (result.downloadStatus == DownloadManagerService.DownloadStatus.FAILED) {
onDownloadFailed(item.getDownloadInfo(), downloadId, result.failureReason,
installNotifyURI);
removeOMADownloadFromSharedPrefs(downloadId);
if (FeatureUtilities.isDownloadProgressInfoBarEnabled()) {
// TODO(shaktisahu): Find a way to pass the failure reason.
showDownloadOnInfoBar(item, result.downloadStatus);
} else {
DownloadManagerService.getDownloadManagerService().onDownloadFailed(
item, result.failureReason);
}
// TODO(shaktisahu): Find a way to pass the failure reason.
showDownloadOnInfoBar(item, result.downloadStatus);
}
}
}.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
......
chrome/android/java/strings/android_chrome_strings.grd
View file @ 83882ed9
......@@ -2408,12 +2408,6 @@ To change this setting, <ph name="BEGIN_LINK">&lt;resetlink&gt;</ph>reset sync<p
<message name="IDS_PREFETCH_BADGE_NEW" desc="Message on download home to show that there are new prefetched contents">
New
</message>
<message name="IDS_DOWNLOAD_SUCCEEDED_MESSAGE" desc="App-based transient message shown when a file download has succeeded." meaning="Android">
<ph name="FILE_NAME">%1$s<ex>http://abc.com/test.pdf</ex></ph> downloaded in <ph name="PRODUCT_NAME">%2$s<ex>Chrome</ex></ph>
</message>
<message name="IDS_DOWNLOAD_SUCCEEDED_MESSAGE_DEFAULT" desc="Transient message shown when a file download has succeeded." meaning="Android">
<ph name="FILE_NAME">%1$s<ex>http://abc.com/test.pdf</ex></ph> downloaded
</message>
<message name="IDS_FILE_SIZE_DOWNLOADED_KB" desc="Notification message showing how many KBs have been downloaded.">
Downloaded <ph name="KBS">%1$.1f<ex>10.1</ex></ph> KB
</message>
......
chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadManagerServiceTest.java
View file @ 83882ed9
......@@ -175,36 +175,6 @@ public class DownloadManagerServiceTest {
public void resumePendingDownloads() {}
}
/**
* Mock implementation of the DownloadSnackbarController.
*/
static class MockDownloadSnackbarController extends DownloadSnackbarController {
private boolean mSucceeded;
private boolean mFailed;
public void waitForSnackbarControllerToFinish(final boolean success) {
CriteriaHelper.pollInstrumentationThread(
new Criteria("Failed while waiting for all calls to complete.") {
@Override
public boolean isSatisfied() {
return success ? mSucceeded : mFailed;
}
});
}
@Override
public void onDownloadSucceeded(
DownloadInfo downloadInfo, int notificationId, long downloadId,
boolean canBeResolved, boolean usesAndroidDownloadManager) {
mSucceeded = true;
}
@Override
public void onDownloadFailed(String errorMessage, boolean showAllDownloads) {
mFailed = true;
}
}
/**
* A set that each object can be matched ^only^ once. Once matched, the object
* will be removed from the set. This is useful to write expectations
......@@ -251,7 +221,6 @@ public class DownloadManagerServiceTest {
private static class DownloadManagerServiceForTest extends DownloadManagerService {
boolean mResumed;
DownloadSnackbarController mDownloadSnackbarController;
public DownloadManagerServiceForTest(
MockDownloadNotifier mockNotifier, long updateDelayInMillis) {
......@@ -271,18 +240,6 @@ public class DownloadManagerServiceTest {
TestThreadUtils.runOnUiThreadBlocking(
() -> DownloadManagerServiceForTest.super.scheduleUpdateIfNeeded());
}
@Override
protected void setDownloadSnackbarController(
DownloadSnackbarController downloadSnackbarController) {
mDownloadSnackbarController = downloadSnackbarController;
super.setDownloadSnackbarController(downloadSnackbarController);
}
@Override
protected void onDownloadFailed(DownloadItem downloadItem, int reason) {
mDownloadSnackbarController.onDownloadFailed("", false);
}
}
private DownloadManagerServiceForTest mService;
......@@ -402,18 +359,16 @@ public class DownloadManagerServiceTest {
public void testDownloadCompletedIsCalled() throws InterruptedException {
if (useDownloadOfflineContentProvider()) return;
MockDownloadNotifier notifier = new MockDownloadNotifier();
MockDownloadSnackbarController snackbarController = new MockDownloadSnackbarController();
createDownloadManagerService(notifier, UPDATE_DELAY_FOR_TEST);
TestThreadUtils.runOnUiThreadBlocking(
(Runnable) () -> DownloadManagerService.setDownloadManagerService(mService));
mService.setDownloadSnackbarController(snackbarController);
DownloadInfoBarController infoBarController = mService.getInfoBarController(false);
// Try calling download completed directly.
DownloadInfo successful = getDownloadInfo();
notifier.expect(MethodID.DOWNLOAD_SUCCESSFUL, successful);
mService.onDownloadCompleted(successful);
notifier.waitTillExpectedCallsComplete();
snackbarController.waitForSnackbarControllerToFinish(true);
// Now check that a successful notification appears after a download progress.
DownloadInfo progress = getDownloadInfo();
......@@ -423,7 +378,6 @@ public class DownloadManagerServiceTest {
Thread.sleep(DELAY_BETWEEN_CALLS);
mService.onDownloadCompleted(progress);
notifier.waitTillExpectedCallsComplete();
snackbarController.waitForSnackbarControllerToFinish(true);
}
@Test
......@@ -431,11 +385,9 @@ public class DownloadManagerServiceTest {
@Feature({"Download"})
public void testDownloadFailedIsCalled() throws InterruptedException {
MockDownloadNotifier notifier = new MockDownloadNotifier();
MockDownloadSnackbarController snackbarController = new MockDownloadSnackbarController();
createDownloadManagerService(notifier, UPDATE_DELAY_FOR_TEST);
TestThreadUtils.runOnUiThreadBlocking(
(Runnable) () -> DownloadManagerService.setDownloadManagerService(mService));
mService.setDownloadSnackbarController(snackbarController);
// Check that if an interrupted download cannot be resumed, it will trigger a download
// failure.
DownloadInfo failure =
......@@ -443,7 +395,6 @@ public class DownloadManagerServiceTest {
notifier.expect(MethodID.DOWNLOAD_FAILED, failure);
mService.onDownloadInterrupted(failure, false);
notifier.waitTillExpectedCallsComplete();
snackbarController.waitForSnackbarControllerToFinish(false);
}
@Test
......
chrome/android/javatests/src/org/chromium/chrome/browser/download/OMADownloadHandlerTest.java
View file @ 83882ed9
......@@ -95,42 +95,12 @@ public class OMADownloadHandlerTest {
}
}
/**
* Mock implementation of the DownloadSnackbarController.
*/
static class MockDownloadSnackbarController extends DownloadSnackbarController {
public boolean mSucceeded;
public boolean mFailed;
public void waitForSnackbarControllerToFinish(final boolean success) {
CriteriaHelper.pollInstrumentationThread(
new Criteria("Failed while waiting for all calls to complete.") {
@Override
public boolean isSatisfied() {
return success ? mSucceeded : mFailed;
}
});
}
@Override
public void onDownloadSucceeded(DownloadInfo downloadInfo, int notificationId,
long downloadId, boolean canBeResolved, boolean usesAndroidDownloadManager) {
mSucceeded = true;
}
@Override
public void onDownloadFailed(String errorMessage, boolean showAllDownloads) {
mFailed = true;
}
}
private static class OMADownloadHandlerForTest extends OMADownloadHandler {
public String mNofityURI;
public long mDownloadId;
public OMADownloadHandlerForTest(
Context context, DownloadSnackbarController downloadSnackbarController) {
super(context, downloadSnackbarController);
public OMADownloadHandlerForTest(Context context) {
super(context);
addObserverForTest(downloadId -> { mDownloadId = downloadId; });
}
......@@ -350,10 +320,7 @@ public class OMADownloadHandlerTest {
UrlUtils.getIsolatedTestFilePath("chrome/test/data/android/download/download.txt"),
4, true);
final MockDownloadSnackbarController snackbarController =
new MockDownloadSnackbarController();
final OMADownloadHandlerForTest omaHandler =
new OMADownloadHandlerForTest(context, snackbarController);
final OMADownloadHandlerForTest omaHandler = new OMADownloadHandlerForTest(context);
// Write a few pending downloads into shared preferences.
Set<String> pendingOmaDownloads = new HashSet<>();
......@@ -406,10 +373,7 @@ public class OMADownloadHandlerTest {
try {
DownloadInfo info = new DownloadInfo.Builder().build();
final MockDownloadSnackbarController snackbarController =
new MockDownloadSnackbarController();
final OMADownloadHandlerForTest omaHandler = new OMADownloadHandlerForTest(
context, snackbarController) {
final OMADownloadHandlerForTest omaHandler = new OMADownloadHandlerForTest(context) {
@Override
public void onReceive(Context context, Intent intent) {
// Ignore all the broadcasts.
......
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