Commit c0f0cb93 authored by David Trainor's avatar David Trainor Committed by Commit Bot

Add testability requirements for DHv2

In order to properly test download home V2 from a Coordinator
standpoint, we need to support building/passing in the following
parameters:

- Whether or not to use the legacy download path (which queries
DownloadManagerService/native code that we don't want).
- Whether or not to use the legacy download thumbnail path (which
queries disk/native code that we don't want).
- Letting tests mock out the OfflineContentProvider for a Profile.

Getting rid of the legacy code path for these tests is okay because
production code will soon do the same once the old code path is fully
deprecated.

Change-Id: I59784bc53c76b22a81fe6c4f7d43ec43ff7da298
Reviewed-on: https://chromium-review.googlesource.com/c/1277915Reviewed-by: default avatarXing Liu <xingliu@chromium.org>
Commit-Queue: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600487}
parent fc2f116c
...@@ -22,6 +22,16 @@ public class DownloadManagerUiConfig { ...@@ -22,6 +22,16 @@ public class DownloadManagerUiConfig {
/** Whether showing full width images should be supported. */ /** Whether showing full width images should be supported. */
public final boolean supportFullWidthImages; public final boolean supportFullWidthImages;
/** Whether or not to use the legacy download path or use the new OfflineContentProvider. */
public final boolean useNewDownloadPath;
/**
* Whether or not to use the legacy download thumbnail path or use the new
* OfflineContentProvider.
*/
public final boolean useNewDownloadPathThumbnails;
/** /**
* The time interval during which a download update is considered recent enough to show * The time interval during which a download update is considered recent enough to show
* in Just Now section. * in Just Now section.
...@@ -34,6 +44,8 @@ public class DownloadManagerUiConfig { ...@@ -34,6 +44,8 @@ public class DownloadManagerUiConfig {
isSeparateActivity = builder.mIsSeparateActivity; isSeparateActivity = builder.mIsSeparateActivity;
useGenericViewTypes = builder.mUseGenericViewTypes; useGenericViewTypes = builder.mUseGenericViewTypes;
supportFullWidthImages = builder.mSupportFullWidthImages; supportFullWidthImages = builder.mSupportFullWidthImages;
useNewDownloadPath = builder.mUseNewDownloadPath;
useNewDownloadPathThumbnails = builder.mUseNewDownloadPathThumbnails;
justNowThresholdSeconds = builder.mJustNowThresholdSeconds; justNowThresholdSeconds = builder.mJustNowThresholdSeconds;
} }
...@@ -48,6 +60,8 @@ public class DownloadManagerUiConfig { ...@@ -48,6 +60,8 @@ public class DownloadManagerUiConfig {
private boolean mIsSeparateActivity; private boolean mIsSeparateActivity;
private boolean mUseGenericViewTypes; private boolean mUseGenericViewTypes;
private boolean mSupportFullWidthImages; private boolean mSupportFullWidthImages;
private boolean mUseNewDownloadPath;
private boolean mUseNewDownloadPathThumbnails;
private long mJustNowThresholdSeconds; private long mJustNowThresholdSeconds;
public Builder() { public Builder() {
...@@ -77,6 +91,16 @@ public class DownloadManagerUiConfig { ...@@ -77,6 +91,16 @@ public class DownloadManagerUiConfig {
return this; return this;
} }
public Builder setUseNewDownloadPath(boolean useNewDownloadPath) {
mUseNewDownloadPath = useNewDownloadPath;
return this;
}
public Builder setUseNewDownloadPathThumbnails(boolean useNewDownloadPathThumbnails) {
mUseNewDownloadPathThumbnails = useNewDownloadPathThumbnails;
return this;
}
public DownloadManagerUiConfig build() { public DownloadManagerUiConfig build() {
return new DownloadManagerUiConfig(this); return new DownloadManagerUiConfig(this);
} }
......
...@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.download.home.glue; ...@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.download.home.glue;
import org.chromium.base.Callback; import org.chromium.base.Callback;
import org.chromium.base.ObserverList; import org.chromium.base.ObserverList;
import org.chromium.chrome.browser.download.home.DownloadManagerUiConfig;
import org.chromium.chrome.browser.widget.ThumbnailProvider; import org.chromium.chrome.browser.widget.ThumbnailProvider;
import org.chromium.components.offline_items_collection.ContentId; import org.chromium.components.offline_items_collection.ContentId;
import org.chromium.components.offline_items_collection.LaunchLocation; import org.chromium.components.offline_items_collection.LaunchLocation;
...@@ -29,16 +30,19 @@ public class OfflineContentProviderGlue implements OfflineContentProvider.Observ ...@@ -29,16 +30,19 @@ public class OfflineContentProviderGlue implements OfflineContentProvider.Observ
private final OfflineContentProvider mProvider; private final OfflineContentProvider mProvider;
private final boolean mIncludeOffTheRecord; private final boolean mIncludeOffTheRecord;
private final boolean mUseNewDownloadPathThumbnails;
private final DownloadGlue mDownloadProvider; private final DownloadGlue mDownloadProvider;
private Query mOutstandingQuery; private Query mOutstandingQuery;
/** Creates an {@link OfflineContentProviderGlue} instance. */ /** Creates an {@link OfflineContentProviderGlue} instance. */
public OfflineContentProviderGlue( public OfflineContentProviderGlue(
OfflineContentProvider provider, boolean includeOffTheRecord) { OfflineContentProvider provider, DownloadManagerUiConfig config) {
mProvider = provider; mProvider = provider;
mIncludeOffTheRecord = includeOffTheRecord; mIncludeOffTheRecord = config.isOffTheRecord;
mDownloadProvider = new DownloadGlue(this); mDownloadProvider = config.useNewDownloadPath ? null : new DownloadGlue(this);
mUseNewDownloadPathThumbnails = config.useNewDownloadPathThumbnails;
mProvider.addObserver(this); mProvider.addObserver(this);
} }
...@@ -48,7 +52,7 @@ public class OfflineContentProviderGlue implements OfflineContentProvider.Observ ...@@ -48,7 +52,7 @@ public class OfflineContentProviderGlue implements OfflineContentProvider.Observ
* is no longer in use. * is no longer in use.
*/ */
public void destroy() { public void destroy() {
mDownloadProvider.destroy(); if (mDownloadProvider != null) mDownloadProvider.destroy();
mProvider.removeObserver(this); mProvider.removeObserver(this);
} }
...@@ -57,7 +61,7 @@ public class OfflineContentProviderGlue implements OfflineContentProvider.Observ ...@@ -57,7 +61,7 @@ public class OfflineContentProviderGlue implements OfflineContentProvider.Observ
// easy use without this layer (we would only pass ID through). // easy use without this layer (we would only pass ID through).
/** @see OfflineContentProvider#openItem(ContentId) */ /** @see OfflineContentProvider#openItem(ContentId) */
public void openItem(OfflineItem item) { public void openItem(OfflineItem item) {
if (LegacyHelpers.isLegacyDownload(item.id)) { if (mDownloadProvider != null && LegacyHelpers.isLegacyDownload(item.id)) {
mDownloadProvider.openItem(item); mDownloadProvider.openItem(item);
} else { } else {
mProvider.openItem(LaunchLocation.DOWNLOAD_HOME, item.id); mProvider.openItem(LaunchLocation.DOWNLOAD_HOME, item.id);
...@@ -66,7 +70,7 @@ public class OfflineContentProviderGlue implements OfflineContentProvider.Observ ...@@ -66,7 +70,7 @@ public class OfflineContentProviderGlue implements OfflineContentProvider.Observ
/** @see OfflineContentProvider#removeItem(ContentId) */ /** @see OfflineContentProvider#removeItem(ContentId) */
public void removeItem(OfflineItem item) { public void removeItem(OfflineItem item) {
if (LegacyHelpers.isLegacyDownload(item.id)) { if (mDownloadProvider != null && LegacyHelpers.isLegacyDownload(item.id)) {
mDownloadProvider.removeItem(item); mDownloadProvider.removeItem(item);
} else { } else {
mProvider.removeItem(item.id); mProvider.removeItem(item.id);
...@@ -75,7 +79,7 @@ public class OfflineContentProviderGlue implements OfflineContentProvider.Observ ...@@ -75,7 +79,7 @@ public class OfflineContentProviderGlue implements OfflineContentProvider.Observ
/** @see OfflineContentProvider#cancelDownload(ContentId) */ /** @see OfflineContentProvider#cancelDownload(ContentId) */
public void cancelDownload(OfflineItem item) { public void cancelDownload(OfflineItem item) {
if (LegacyHelpers.isLegacyDownload(item.id)) { if (mDownloadProvider != null && LegacyHelpers.isLegacyDownload(item.id)) {
mDownloadProvider.cancelDownload(item); mDownloadProvider.cancelDownload(item);
} else { } else {
mProvider.cancelDownload(item.id); mProvider.cancelDownload(item.id);
...@@ -84,7 +88,7 @@ public class OfflineContentProviderGlue implements OfflineContentProvider.Observ ...@@ -84,7 +88,7 @@ public class OfflineContentProviderGlue implements OfflineContentProvider.Observ
/** @see OfflineContentProvider#pauseDownload(ContentId) */ /** @see OfflineContentProvider#pauseDownload(ContentId) */
public void pauseDownload(OfflineItem item) { public void pauseDownload(OfflineItem item) {
if (LegacyHelpers.isLegacyDownload(item.id)) { if (mDownloadProvider != null && LegacyHelpers.isLegacyDownload(item.id)) {
mDownloadProvider.pauseDownload(item); mDownloadProvider.pauseDownload(item);
} else { } else {
mProvider.pauseDownload(item.id); mProvider.pauseDownload(item.id);
...@@ -93,7 +97,7 @@ public class OfflineContentProviderGlue implements OfflineContentProvider.Observ ...@@ -93,7 +97,7 @@ public class OfflineContentProviderGlue implements OfflineContentProvider.Observ
/** @see OfflineContentProvider#resumeDownload(ContentId) */ /** @see OfflineContentProvider#resumeDownload(ContentId) */
public void resumeDownload(OfflineItem item, boolean hasUserGesture) { public void resumeDownload(OfflineItem item, boolean hasUserGesture) {
if (LegacyHelpers.isLegacyDownload(item.id)) { if (mDownloadProvider != null && LegacyHelpers.isLegacyDownload(item.id)) {
mDownloadProvider.resumeDownload(item, hasUserGesture); mDownloadProvider.resumeDownload(item, hasUserGesture);
} else { } else {
mProvider.resumeDownload(item.id, hasUserGesture); mProvider.resumeDownload(item.id, hasUserGesture);
...@@ -102,7 +106,7 @@ public class OfflineContentProviderGlue implements OfflineContentProvider.Observ ...@@ -102,7 +106,7 @@ public class OfflineContentProviderGlue implements OfflineContentProvider.Observ
/** @see OfflineContentProvider#getItemById(ContentId, Callback) */ /** @see OfflineContentProvider#getItemById(ContentId, Callback) */
public void getItemById(ContentId id, Callback<OfflineItem> callback) { public void getItemById(ContentId id, Callback<OfflineItem> callback) {
if (LegacyHelpers.isLegacyDownload(id)) { if (mDownloadProvider != null && LegacyHelpers.isLegacyDownload(id)) {
mDownloadProvider.getItemById(id, callback); mDownloadProvider.getItemById(id, callback);
} else { } else {
mProvider.getItemById(id, callback); mProvider.getItemById(id, callback);
...@@ -120,7 +124,7 @@ public class OfflineContentProviderGlue implements OfflineContentProvider.Observ ...@@ -120,7 +124,7 @@ public class OfflineContentProviderGlue implements OfflineContentProvider.Observ
* @see OfflineContentProvider#getVisualsForItem(ContentId, VisualsCallback) * @see OfflineContentProvider#getVisualsForItem(ContentId, VisualsCallback)
*/ */
public boolean getVisualsForItem(ContentId id, VisualsCallback callback) { public boolean getVisualsForItem(ContentId id, VisualsCallback callback) {
if (LegacyHelpers.isLegacyDownload(id)) return false; if (!mUseNewDownloadPathThumbnails && LegacyHelpers.isLegacyDownload(id)) return false;
mProvider.getVisualsForItem(id, callback); mProvider.getVisualsForItem(id, callback);
return true; return true;
} }
...@@ -131,13 +135,13 @@ public class OfflineContentProviderGlue implements OfflineContentProvider.Observ ...@@ -131,13 +135,13 @@ public class OfflineContentProviderGlue implements OfflineContentProvider.Observ
* layer needs to determine what to do with downloads that have externally managed thumbnails. * layer needs to determine what to do with downloads that have externally managed thumbnails.
*/ */
public void removeVisualsForItem(ThumbnailProvider provider, ContentId id) { public void removeVisualsForItem(ThumbnailProvider provider, ContentId id) {
if (!LegacyHelpers.isLegacyDownload(id)) return; if (mUseNewDownloadPathThumbnails || !LegacyHelpers.isLegacyDownload(id)) return;
provider.removeThumbnailsFromDisk(id.id); provider.removeThumbnailsFromDisk(id.id);
} }
/** @see OfflineContentProvider#getShareInfoForItem(ContentId, ShareCallback) */ /** @see OfflineContentProvider#getShareInfoForItem(ContentId, ShareCallback) */
public void getShareInfoForItem(OfflineItem item, ShareCallback callback) { public void getShareInfoForItem(OfflineItem item, ShareCallback callback) {
if (LegacyHelpers.isLegacyDownload(item.id)) { if (mDownloadProvider != null && LegacyHelpers.isLegacyDownload(item.id)) {
mDownloadProvider.getShareInfoForItem(item, callback); mDownloadProvider.getShareInfoForItem(item, callback);
} else { } else {
mProvider.getShareInfoForItem(item.id, callback); mProvider.getShareInfoForItem(item.id, callback);
...@@ -188,11 +192,17 @@ public class OfflineContentProviderGlue implements OfflineContentProvider.Observ ...@@ -188,11 +192,17 @@ public class OfflineContentProviderGlue implements OfflineContentProvider.Observ
public Query() { public Query() {
mDownloadProviderOffTheRecordResponded = !mIncludeOffTheRecord; mDownloadProviderOffTheRecordResponded = !mIncludeOffTheRecord;
if (mIncludeOffTheRecord) { if (mDownloadProvider == null) {
mDownloadProviderResponded = true;
mDownloadProviderOffTheRecordResponded = true;
} else {
if (mIncludeOffTheRecord) {
mDownloadProvider.getAllItems(
items -> addOffTheRecordDownloads(items), true /* offTheRecord */);
}
mDownloadProvider.getAllItems( mDownloadProvider.getAllItems(
items -> addOffTheRecordDownloads(items), true /* offTheRecord */); items -> addDownloads(items), false /* offTheRecord */);
} }
mDownloadProvider.getAllItems(items -> addDownloads(items), false /* offTheRecord */);
mProvider.getAllItems(items -> addOfflineItems(items)); mProvider.getAllItems(items -> addOfflineItems(items));
} }
......
...@@ -132,7 +132,7 @@ class DateOrderedListMediator { ...@@ -132,7 +132,7 @@ class DateOrderedListMediator {
// [DateOrderedListMutator] -> // [DateOrderedListMutator] ->
// [ListItemModel] // [ListItemModel]
mProvider = new OfflineContentProviderGlue(provider, config.isOffTheRecord); mProvider = new OfflineContentProviderGlue(provider, config);
mShareController = shareController; mShareController = shareController;
mModel = model; mModel = model;
mDeleteController = deleteController; mDeleteController = deleteController;
......
...@@ -4,6 +4,9 @@ ...@@ -4,6 +4,9 @@
package org.chromium.chrome.browser.download.items; package org.chromium.chrome.browser.download.items;
import android.support.annotation.Nullable;
import android.support.annotation.VisibleForTesting;
import org.chromium.chrome.browser.profiles.Profile; import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.components.offline_items_collection.OfflineContentProvider; import org.chromium.components.offline_items_collection.OfflineContentProvider;
...@@ -18,6 +21,23 @@ public class OfflineContentAggregatorFactory { ...@@ -18,6 +21,23 @@ public class OfflineContentAggregatorFactory {
private OfflineContentAggregatorFactory() {} private OfflineContentAggregatorFactory() {}
/**
* Allows tests to push a custom {@link OfflineContentProvider} to be used instead of the one
* pulled from a {@link Profile}.
* @param provider The {@link OfflineContentProvider} to return. If {@code null}, will revert
* to the non-overriding behavior and pull a {link OfflineContentProvider} from
* {@link Profile}.
*/
@VisibleForTesting
public static void setOfflineContentProviderForTests(
@Nullable OfflineContentProvider provider) {
if (provider == null) {
sBlockedProvider = null;
} else {
sBlockedProvider = new DownloadBlockedOfflineContentProvider(provider);
}
}
/** /**
* Used to get access to the {@link OfflineContentProvider} associated with {@code profile}. * Used to get access to the {@link OfflineContentProvider} associated with {@code profile}.
* The same {@link OfflineContentProvider} will be returned for the same {@link Profile}. * The same {@link OfflineContentProvider} will be returned for the same {@link Profile}.
......
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