Commit 95d8be7f authored by Xing Liu's avatar Xing Liu Committed by Commit Bot

Fixed the upper guid issue in driver.

Currently content download only supports upper case guid. This CL
limits guid in download service to be upper case ascii chars.

In the future, we should consider the two guid approach to decouple
from content layer guid details.



Bug: 734818
Change-Id: Id03a234c0edb14c9ee7a66d5bb486cb1503e1840
Reviewed-on: https://chromium-review.googlesource.com/540679Reviewed-by: default avatarDavid Trainor <dtrainor@chromium.org>
Commit-Queue: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481004}
parent 4f4375e3
...@@ -634,6 +634,8 @@ void ControllerImpl::HandleStartDownloadResponse( ...@@ -634,6 +634,8 @@ void ControllerImpl::HandleStartDownloadResponse(
model_->Remove(guid); model_->Remove(guid);
} }
if (callback.is_null())
return;
base::ThreadTaskRunnerHandle::Get()->PostTask( base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::Bind(callback, guid, result)); FROM_HERE, base::Bind(callback, guid, result));
} }
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#include "components/download/internal/download_service_impl.h" #include "components/download/internal/download_service_impl.h"
#include "base/strings/string_util.h"
#include "components/download/internal/controller.h" #include "components/download/internal/controller.h"
#include "components/download/internal/startup_status.h" #include "components/download/internal/startup_status.h"
#include "components/download/internal/stats.h" #include "components/download/internal/stats.h"
...@@ -47,24 +48,28 @@ DownloadService::ServiceStatus DownloadServiceImpl::GetStatus() { ...@@ -47,24 +48,28 @@ DownloadService::ServiceStatus DownloadServiceImpl::GetStatus() {
void DownloadServiceImpl::StartDownload(const DownloadParams& download_params) { void DownloadServiceImpl::StartDownload(const DownloadParams& download_params) {
stats::LogServiceApiAction(download_params.client, stats::LogServiceApiAction(download_params.client,
stats::ServiceApiAction::START_DOWNLOAD); stats::ServiceApiAction::START_DOWNLOAD);
DCHECK_EQ(download_params.guid, base::ToUpperASCII(download_params.guid));
controller_->StartDownload(download_params); controller_->StartDownload(download_params);
} }
void DownloadServiceImpl::PauseDownload(const std::string& guid) { void DownloadServiceImpl::PauseDownload(const std::string& guid) {
stats::LogServiceApiAction(controller_->GetOwnerOfDownload(guid), stats::LogServiceApiAction(controller_->GetOwnerOfDownload(guid),
stats::ServiceApiAction::PAUSE_DOWNLOAD); stats::ServiceApiAction::PAUSE_DOWNLOAD);
DCHECK_EQ(guid, base::ToUpperASCII(guid));
controller_->PauseDownload(guid); controller_->PauseDownload(guid);
} }
void DownloadServiceImpl::ResumeDownload(const std::string& guid) { void DownloadServiceImpl::ResumeDownload(const std::string& guid) {
stats::LogServiceApiAction(controller_->GetOwnerOfDownload(guid), stats::LogServiceApiAction(controller_->GetOwnerOfDownload(guid),
stats::ServiceApiAction::RESUME_DOWNLOAD); stats::ServiceApiAction::RESUME_DOWNLOAD);
DCHECK_EQ(guid, base::ToUpperASCII(guid));
controller_->ResumeDownload(guid); controller_->ResumeDownload(guid);
} }
void DownloadServiceImpl::CancelDownload(const std::string& guid) { void DownloadServiceImpl::CancelDownload(const std::string& guid) {
stats::LogServiceApiAction(controller_->GetOwnerOfDownload(guid), stats::LogServiceApiAction(controller_->GetOwnerOfDownload(guid),
stats::ServiceApiAction::CANCEL_DOWNLOAD); stats::ServiceApiAction::CANCEL_DOWNLOAD);
DCHECK_EQ(guid, base::ToUpperASCII(guid));
controller_->CancelDownload(guid); controller_->CancelDownload(guid);
} }
...@@ -73,6 +78,7 @@ void DownloadServiceImpl::ChangeDownloadCriteria( ...@@ -73,6 +78,7 @@ void DownloadServiceImpl::ChangeDownloadCriteria(
const SchedulingParams& params) { const SchedulingParams& params) {
stats::LogServiceApiAction(controller_->GetOwnerOfDownload(guid), stats::LogServiceApiAction(controller_->GetOwnerOfDownload(guid),
stats::ServiceApiAction::CHANGE_CRITERIA); stats::ServiceApiAction::CHANGE_CRITERIA);
DCHECK_EQ(guid, base::ToUpperASCII(guid));
controller_->ChangeDownloadCriteria(guid, params); controller_->ChangeDownloadCriteria(guid, params);
} }
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
#include "base/strings/string_util.h"
#include "components/download/internal/startup_status.h" #include "components/download/internal/startup_status.h"
#include "components/download/internal/test/download_params_utils.h" #include "components/download/internal/test/download_params_utils.h"
#include "components/download/internal/test/mock_controller.h" #include "components/download/internal/test/mock_controller.h"
...@@ -63,6 +64,9 @@ TEST_F(DownloadServiceImplTest, TestGetStatus) { ...@@ -63,6 +64,9 @@ TEST_F(DownloadServiceImplTest, TestGetStatus) {
TEST_F(DownloadServiceImplTest, TestApiPassThrough) { TEST_F(DownloadServiceImplTest, TestApiPassThrough) {
DownloadParams params = test::BuildBasicDownloadParams(); DownloadParams params = test::BuildBasicDownloadParams();
// TODO(xingliu): Remove the limitation of upper case guid in
// |download_params|, see http://crbug.com/734818.
params.guid = base::ToUpperASCII(params.guid);
SchedulingParams scheduling_params; SchedulingParams scheduling_params;
scheduling_params.priority = SchedulingParams::Priority::UI; scheduling_params.priority = SchedulingParams::Priority::UI;
......
...@@ -41,7 +41,6 @@ SchedulerImpl::SchedulerImpl(TaskScheduler* task_scheduler, ...@@ -41,7 +41,6 @@ SchedulerImpl::SchedulerImpl(TaskScheduler* task_scheduler,
config_(config), config_(config),
download_clients_(clients), download_clients_(clients),
current_client_index_(0) { current_client_index_(0) {
DCHECK(!download_clients_.empty());
DCHECK(task_scheduler_); DCHECK(task_scheduler_);
} }
......
...@@ -131,6 +131,8 @@ struct DownloadParams { ...@@ -131,6 +131,8 @@ struct DownloadParams {
DownloadClient client; DownloadClient client;
// A unique GUID that represents this download. See |base::GenerateGUID()|. // A unique GUID that represents this download. See |base::GenerateGUID()|.
// TODO(xingliu): guid in content download must be upper case, see
// http://crbug.com/734818.
std::string guid; std::string guid;
// A callback that will be notified if this download has been accepted and // A callback that will be notified if this download has been accepted and
......
...@@ -75,7 +75,9 @@ class DownloadService : public KeyedService { ...@@ -75,7 +75,9 @@ class DownloadService : public KeyedService {
// Sends the download to the service. A callback to // Sends the download to the service. A callback to
// |DownloadParams::callback| will be triggered once the download has been // |DownloadParams::callback| will be triggered once the download has been
// persisted and saved in the service // persisted and saved in the service.
// TODO(xingliu): Remove the limitation of upper case guid in
// |download_params|, see http://crbug.com/734818.
virtual void StartDownload(const DownloadParams& download_params) = 0; virtual void StartDownload(const DownloadParams& download_params) = 0;
// Allows any feature to pause or resume downloads at will. Paused downloads // Allows any feature to pause or resume downloads at will. Paused downloads
......
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