Skip to content

Commit

Permalink
Download later: Plumb ChangeSchedule from download core to UI.
Browse files Browse the repository at this point in the history
DownloadItem::ChangeSchedule needs to plumb through offline item code
stack and the legacy download manager service code path, so download
home UI and infobar call use them.

TBR=dtrainor@chromium.org, qinmin@chromium.org

Bug: 1101869,1078454
Change-Id: I7e63bb837a123cce9ad045113bfbf3e87b729bb4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2280398
Reviewed-by: Xing Liu <xingliu@chromium.org>
Reviewed-by: Hesen Zhang <hesen@chromium.org>
Commit-Queue: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#785528}
  • Loading branch information
Xing Liu authored and Commit Bot committed Jul 6, 2020
1 parent e81c6b9 commit b141af9
Show file tree
Hide file tree
Showing 30 changed files with 252 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
import org.chromium.components.offline_items_collection.LegacyHelpers;
import org.chromium.components.offline_items_collection.OfflineContentProvider;
import org.chromium.components.offline_items_collection.OfflineItem;
import org.chromium.components.offline_items_collection.OfflineItemSchedule;
import org.chromium.components.offline_items_collection.OfflineItemState;
import org.chromium.components.offline_items_collection.PendingState;
import org.chromium.components.offline_items_collection.UpdateDelta;
Expand Down Expand Up @@ -1380,6 +1381,20 @@ public void renameDownload(ContentId id, String name,
DownloadManagerService.this, id.id, name, callback, isOffTheRecord);
}

/**
* Change the download schedule to start the download in a different condition.
* @param id The id of the {@link OfflineItem} that requests the change.
* @param schedule The download schedule that defines when to start the download.
* @param isOffTheRecord Whether the download is for off the record profile.
*/
public void changeSchedule(
final ContentId id, final OfflineItemSchedule schedule, boolean isOffTheRecord) {
boolean onlyOnWifi = (schedule == null) ? false : schedule.onlyOnWifi;
long startTimeMs = (schedule == null) ? -1 : schedule.startTimeMs;
DownloadManagerServiceJni.get().changeSchedule(getNativeDownloadManagerService(),
DownloadManagerService.this, id.id, onlyOnWifi, startTimeMs, isOffTheRecord);
}

/**
* Add an Intent extra for StateAtCancel UMA to know the state of a request prior to a
* user-initated cancel.
Expand Down Expand Up @@ -1816,6 +1831,8 @@ void removeDownload(long nativeDownloadManagerService, DownloadManagerService ca
void renameDownload(long nativeDownloadManagerService, DownloadManagerService caller,
String downloadGuid, String targetName, Callback</*RenameResult*/ Integer> callback,
boolean isOffTheRecord);
void changeSchedule(long nativeDownloadManagerService, DownloadManagerService caller,
String downloadGuid, boolean onlyOnWifi, long startTimeMs, boolean isOffTheRecord);
void getAllDownloads(long nativeDownloadManagerService, DownloadManagerService caller,
boolean isOffTheRecord);
void checkForExternallyRemovedDownloads(long nativeDownloadManagerService,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.chromium.components.offline_items_collection.OfflineContentProvider;
import org.chromium.components.offline_items_collection.OfflineContentProvider.Observer;
import org.chromium.components.offline_items_collection.OfflineItem;
import org.chromium.components.offline_items_collection.OfflineItemSchedule;
import org.chromium.components.offline_items_collection.OfflineItemShareInfo;
import org.chromium.components.offline_items_collection.ShareCallback;
import org.chromium.components.offline_items_collection.VisualsCallback;
Expand Down Expand Up @@ -203,6 +204,12 @@ public void renameItem(
item.id, name, callback, item.isOffTheRecord);
}

@Override
public void changeSchedule(final OfflineItem item, final OfflineItemSchedule schedule) {
DownloadManagerService.getDownloadManagerService().changeSchedule(
item.id, schedule, item.isOffTheRecord);
}

/**
* There could be some situations where we can't visually represent this download in the UI.
* This should be handled in native/be more generic, but it's here in the glue for now.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import org.chromium.components.offline_items_collection.LegacyHelpers;
import org.chromium.components.offline_items_collection.OfflineContentProvider;
import org.chromium.components.offline_items_collection.OfflineItem;
import org.chromium.components.offline_items_collection.OfflineItemSchedule;
import org.chromium.components.offline_items_collection.OpenParams;
import org.chromium.components.offline_items_collection.ShareCallback;
import org.chromium.components.offline_items_collection.UpdateDelta;
Expand Down Expand Up @@ -113,6 +114,15 @@ public void resumeDownload(OfflineItem item, boolean hasUserGesture) {
}
}

/** @see OfflineContentProvider#changeSchedule */
public void changeSchedule(final OfflineItem item, final OfflineItemSchedule schedule) {
if (mLegacyProvider != null && LegacyHelpers.isLegacyDownload(item.id)) {
mLegacyProvider.changeSchedule(item, schedule);
} else {
mProvider.changeSchedule(item.id, schedule);
}
}

/** @see OfflineContentProvider#getItemById(ContentId, Callback) */
public void getItemById(ContentId id, Callback<OfflineItem> callback) {
if (mLegacyProvider != null && LegacyHelpers.isLegacyDownload(id)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,8 @@ private void onRenameItem(OfflineItem item) {

private void onChangeItem(OfflineItem item) {
UmaUtils.recordItemAction(ViewAction.MENU_CHANGE);
// TODO(xingliu): Implement this by wiring to download later dialog.
// TODO(xingliu): Hook to download later dialog to generate the schedule.
mProvider.changeSchedule(item, null);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import org.chromium.components.offline_items_collection.LegacyHelpers;
import org.chromium.components.offline_items_collection.OfflineContentProvider;
import org.chromium.components.offline_items_collection.OfflineItem;
import org.chromium.components.offline_items_collection.OfflineItemSchedule;
import org.chromium.components.offline_items_collection.OpenParams;
import org.chromium.components.offline_items_collection.ShareCallback;
import org.chromium.components.offline_items_collection.UpdateDelta;
Expand Down Expand Up @@ -63,6 +64,12 @@ public void resumeDownload(ContentId id, boolean hasUserGesture) {
mProvider.resumeDownload(id, hasUserGesture);
}

@Override
public void changeSchedule(final ContentId id, final OfflineItemSchedule schedule) {
assert !LegacyHelpers.isLegacyDownload(id);
mProvider.changeSchedule(id, schedule);
}

@Override
public void getItemById(ContentId id, Callback<OfflineItem> callback) {
assert !LegacyHelpers.isLegacyDownload(id);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import org.chromium.components.offline_items_collection.ContentId;
import org.chromium.components.offline_items_collection.OfflineContentProvider;
import org.chromium.components.offline_items_collection.OfflineItem;
import org.chromium.components.offline_items_collection.OfflineItemSchedule;
import org.chromium.components.offline_items_collection.OpenParams;
import org.chromium.components.offline_items_collection.RenameResult;
import org.chromium.components.offline_items_collection.ShareCallback;
Expand Down Expand Up @@ -105,6 +106,9 @@ public void pauseDownload(ContentId id) {}
@Override
public void resumeDownload(ContentId id, boolean hasUserGesture) {}

@Override
public void changeSchedule(final ContentId id, final OfflineItemSchedule schedule) {}

@Override
public void cancelDownload(ContentId id) {}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.chromium.components.offline_items_collection.OfflineContentProvider;
import org.chromium.components.offline_items_collection.OfflineItem;
import org.chromium.components.offline_items_collection.OfflineItemFilter;
import org.chromium.components.offline_items_collection.OfflineItemSchedule;
import org.chromium.components.offline_items_collection.OfflineItemState;
import org.chromium.components.offline_items_collection.OpenParams;
import org.chromium.components.offline_items_collection.RenameResult;
Expand Down Expand Up @@ -79,6 +80,10 @@ public void openItem(OpenParams openParams, ContentId id) {}
public void pauseDownload(ContentId id) {}
@Override
public void resumeDownload(ContentId id, boolean hasUserGesture) {}

@Override
public void changeSchedule(final ContentId id, final OfflineItemSchedule schedule) {}

@Override
public void cancelDownload(ContentId id) {}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -816,6 +816,12 @@ void BackgroundFetchDelegateImpl::RenameItem(
NOTIMPLEMENTED();
}

void BackgroundFetchDelegateImpl::ChangeSchedule(
const offline_items_collection::ContentId& id,
base::Optional<offline_items_collection::OfflineItemSchedule> schedule) {
NOTIMPLEMENTED();
}

void BackgroundFetchDelegateImpl::AddObserver(Observer* observer) {
DCHECK(!observers_.count(observer));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,10 @@ class BackgroundFetchDelegateImpl
void RenameItem(const offline_items_collection::ContentId& id,
const std::string& name,
RenameCallback callback) override;
void ChangeSchedule(
const offline_items_collection::ContentId& id,
base::Optional<offline_items_collection::OfflineItemSchedule> schedule)
override;
void AddObserver(Observer* observer) override;
void RemoveObserver(Observer* observer) override;

Expand Down
8 changes: 7 additions & 1 deletion chrome/browser/content_index/content_index_provider_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ using offline_items_collection::ContentId;
using offline_items_collection::LaunchLocation;
using offline_items_collection::OfflineItem;
using offline_items_collection::OfflineItemFilter;

using offline_items_collection::OfflineItemSchedule;

namespace {

Expand Down Expand Up @@ -396,6 +396,12 @@ void ContentIndexProviderImpl::RenameItem(const ContentId& id,
NOTREACHED();
}

void ContentIndexProviderImpl::ChangeSchedule(
const ContentId& id,
base::Optional<OfflineItemSchedule> schedule) {
NOTREACHED();
}

void ContentIndexProviderImpl::AddObserver(Observer* observer) {
observers_.AddObserver(observer);
}
Expand Down
4 changes: 4 additions & 0 deletions chrome/browser/content_index/content_index_provider_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ class ContentIndexProviderImpl
void RenameItem(const offline_items_collection::ContentId& id,
const std::string& name,
RenameCallback callback) override;
void ChangeSchedule(
const offline_items_collection::ContentId& id,
base::Optional<offline_items_collection::OfflineItemSchedule> schedule)
override;
void AddObserver(Observer* observer) override;
void RemoveObserver(Observer* observer) override;

Expand Down
24 changes: 24 additions & 0 deletions chrome/browser/download/android/download_manager_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "base/bind.h"
#include "base/location.h"
#include "base/metrics/field_trial_params.h"
#include "base/optional.h"
#include "base/single_thread_task_runner.h"
#include "base/strings/string_number_conversions.h"
#include "base/threading/thread_task_runner_handle.h"
Expand Down Expand Up @@ -741,6 +742,29 @@ void DownloadManagerService::RenameDownload(
item->Rename(base::FilePath(target_name), std::move(callback));
}

void DownloadManagerService::ChangeSchedule(JNIEnv* env,
const JavaParamRef<jobject>& obj,
const JavaParamRef<jstring>& id,
jboolean only_on_wifi,
jlong start_time,
jboolean is_off_the_record) {
std::string download_guid = ConvertJavaStringToUTF8(id);
download::DownloadItem* item = GetDownload(download_guid, is_off_the_record);
if (!item)
return;

base::Optional<DownloadSchedule> download_schedule;
if (only_on_wifi) {
download_schedule = base::make_optional<DownloadSchedule>(
true /*only_on_wifi*/, base::nullopt);
} else if (start_time > 0) {
download_schedule = base::make_optional<DownloadSchedule>(
false /*only_on_wifi*/, base::Time::FromJavaTime(start_time));
}

item->OnDownloadScheduleChanged(std::move(download_schedule));
}

void DownloadManagerService::CreateInterruptedDownloadForTest(
JNIEnv* env,
jobject obj,
Expand Down
9 changes: 9 additions & 0 deletions chrome/browser/download/android/download_manager_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,15 @@ class DownloadManagerService
const JavaParamRef<jobject>& callback,
bool is_off_the_record);

// Called to change the download schedule of a download item that has GUID
// equal to |id|.
void ChangeSchedule(JNIEnv* env,
const JavaParamRef<jobject>& obj,
const JavaParamRef<jstring>& id,
jboolean only_on_wifi,
jlong start_time,
jboolean is_off_the_record);

// Returns whether or not the given download can be opened by the browser.
bool IsDownloadOpenableInBrowser(JNIEnv* env,
jobject obj,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import org.chromium.components.offline_items_collection.ContentId;
import org.chromium.components.offline_items_collection.OfflineContentProvider;
import org.chromium.components.offline_items_collection.OfflineItem;
import org.chromium.components.offline_items_collection.OfflineItemSchedule;
import org.chromium.components.offline_items_collection.ShareCallback;
import org.chromium.components.offline_items_collection.VisualsCallback;

Expand Down Expand Up @@ -58,4 +59,7 @@ public interface LegacyDownloadProvider {

/** @see OfflineContentProvider#renameItem(ContentId, String, Callback)*/
void renameItem(OfflineItem item, String name, Callback</*RenameResult*/ Integer> callback);
}

/** @see OfflineContentProvider#changeSchedule(ContentId, OfflineItemSchedule) */
void changeSchedule(final OfflineItem item, final OfflineItemSchedule schedule);
}
20 changes: 20 additions & 0 deletions chrome/browser/download/download_offline_content_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ using OfflineItemFilter = offline_items_collection::OfflineItemFilter;
using OfflineItemState = offline_items_collection::OfflineItemState;
using OfflineItemProgressUnit =
offline_items_collection::OfflineItemProgressUnit;
using offline_items_collection::OfflineItemSchedule;
using offline_items_collection::OfflineItemShareInfo;
using OfflineItemVisuals = offline_items_collection::OfflineItemVisuals;
using UpdateDelta = offline_items_collection::UpdateDelta;
Expand Down Expand Up @@ -392,6 +393,25 @@ void DownloadOfflineContentProvider::RenameItem(const ContentId& id,
item->Rename(base::FilePath(filename), std::move(download_callback));
}

void DownloadOfflineContentProvider::ChangeSchedule(
const ContentId& id,
base::Optional<OfflineItemSchedule> schedule) {
EnsureDownloadCoreServiceStarted();
if (state_ != State::HISTORY_LOADED) {
pending_actions_for_full_browser_.push_back(base::BindOnce(
&DownloadOfflineContentProvider::ChangeSchedule,
weak_ptr_factory_.GetWeakPtr(), id, std::move(schedule)));
return;
}

DownloadItem* item = GetDownload(id.id);
if (!item)
return;

item->OnDownloadScheduleChanged(
OfflineItemUtils::ToDownloadSchedule(schedule));
}

void DownloadOfflineContentProvider::OnRenameDownloadCallbackDone(
RenameCallback callback,
DownloadItem* item,
Expand Down
5 changes: 5 additions & 0 deletions chrome/browser/download/download_offline_content_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "components/offline_items_collection/core/offline_content_provider.h"
#include "components/offline_items_collection/core/offline_item.h"

// TODO(xingliu): Remove using in the header files.
using DownloadItem = download::DownloadItem;
using SimpleDownloadManagerCoordinator =
download::SimpleDownloadManagerCoordinator;
Expand Down Expand Up @@ -75,6 +76,10 @@ class DownloadOfflineContentProvider
void RenameItem(const ContentId& id,
const std::string& name,
RenameCallback callback) override;
void ChangeSchedule(
const offline_items_collection::ContentId& id,
base::Optional<offline_items_collection::OfflineItemSchedule> schedule)
override;

// Methods that can be run in reduced mode.
void CancelDownload(const ContentId& id) override;
Expand Down
27 changes: 22 additions & 5 deletions chrome/browser/download/offline_item_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#endif

using DownloadItem = download::DownloadItem;
using DownloadSchedule = download::DownloadSchedule;
using ContentId = offline_items_collection::ContentId;
using OfflineItem = offline_items_collection::OfflineItem;
using OfflineItemFilter = offline_items_collection::OfflineItemFilter;
Expand Down Expand Up @@ -132,11 +133,7 @@ OfflineItem OfflineItemUtils::CreateOfflineItem(const std::string& name_space,
item.fail_state =
ConvertDownloadInterruptReasonToFailState(download_item->GetLastReason());
item.can_rename = download_item->GetState() == DownloadItem::COMPLETE;
const auto& download_schedule = download_item->GetDownloadSchedule();
if (download_schedule.has_value()) {
item.schedule = base::make_optional<OfflineItemSchedule>(
download_schedule->only_on_wifi(), download_schedule->start_time());
}
item.schedule = ToOfflineItemSchedule(download_item->GetDownloadSchedule());

switch (download_item->GetState()) {
case DownloadItem::IN_PROGRESS:
Expand Down Expand Up @@ -347,3 +344,23 @@ RenameResult OfflineItemUtils::ConvertDownloadRenameResultToRenameResult(
return RenameResult::FAILURE_UNKNOWN;
}
}

// static
base::Optional<DownloadSchedule> OfflineItemUtils::ToDownloadSchedule(
base::Optional<OfflineItemSchedule> offline_item_schedule) {
if (!offline_item_schedule)
return base::nullopt;

return base::make_optional<DownloadSchedule>(
offline_item_schedule->only_on_wifi, offline_item_schedule->start_time);
}

// static
base::Optional<OfflineItemSchedule> OfflineItemUtils::ToOfflineItemSchedule(
base::Optional<DownloadSchedule> download_schedule) {
if (!download_schedule)
return base::nullopt;

return base::make_optional<OfflineItemSchedule>(
download_schedule->only_on_wifi(), download_schedule->start_time());
}

0 comments on commit b141af9

Please sign in to comment.