summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorkinaba@chromium.org <kinaba@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-04-25 15:10:30 +0000
committerkinaba@chromium.org <kinaba@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-04-25 15:10:30 +0000
commit5bb0d078246b270f2b79735a37ffa40395d5c1e4 (patch)
tree2cd7504f50a8ac262e8e73d26d1ab69b4276bd08
parent693c1830f2ad5536bda5deb6fae6c6de57416011 (diff)
downloadchromium_src-5bb0d078246b270f2b79735a37ffa40395d5c1e4.zip
chromium_src-5bb0d078246b270f2b79735a37ffa40395d5c1e4.tar.gz
chromium_src-5bb0d078246b270f2b79735a37ffa40395d5c1e4.tar.bz2
Get rid of the progress report generation part from OperationRegistry.
Now, all the clients of the feature has been converted to use DriveScheduler. Job cancellation feature of OperationRegistry is still used, so unfortunately we cannot remove all of the class at once. BUG=164098 Review URL: https://chromiumcodereview.appspot.com/14374004 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@196400 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--chrome/browser/google_apis/auth_service_interface.h1
-rw-r--r--chrome/browser/google_apis/base_operations.cc13
-rw-r--r--chrome/browser/google_apis/base_operations.h5
-rw-r--r--chrome/browser/google_apis/drive_api_operations.cc1
-rw-r--r--chrome/browser/google_apis/drive_api_service.cc17
-rw-r--r--chrome/browser/google_apis/drive_api_service.h8
-rw-r--r--chrome/browser/google_apis/drive_service_interface.h6
-rw-r--r--chrome/browser/google_apis/dummy_drive_service.cc4
-rw-r--r--chrome/browser/google_apis/dummy_drive_service.h1
-rw-r--r--chrome/browser/google_apis/fake_drive_service.cc5
-rw-r--r--chrome/browser/google_apis/fake_drive_service.h1
-rw-r--r--chrome/browser/google_apis/gdata_wapi_operations.cc1
-rw-r--r--chrome/browser/google_apis/gdata_wapi_service.cc17
-rw-r--r--chrome/browser/google_apis/gdata_wapi_service.h8
-rw-r--r--chrome/browser/google_apis/mock_drive_service.cc2
-rw-r--r--chrome/browser/google_apis/mock_drive_service.h2
-rw-r--r--chrome/browser/google_apis/operation_registry.cc178
-rw-r--r--chrome/browser/google_apis/operation_registry.h72
-rw-r--r--chrome/browser/google_apis/operation_registry_unittest.cc184
19 files changed, 26 insertions, 500 deletions
diff --git a/chrome/browser/google_apis/auth_service_interface.h b/chrome/browser/google_apis/auth_service_interface.h
index 6853059..4b2f006 100644
--- a/chrome/browser/google_apis/auth_service_interface.h
+++ b/chrome/browser/google_apis/auth_service_interface.h
@@ -15,7 +15,6 @@ class Profile;
namespace google_apis {
class AuthServiceObserver;
-class OperationRegistry;
// Called when fetching of access token is complete.
typedef base::Callback<void(GDataErrorCode error,
diff --git a/chrome/browser/google_apis/base_operations.cc b/chrome/browser/google_apis/base_operations.cc
index d42004dd..2767788 100644
--- a/chrome/browser/google_apis/base_operations.cc
+++ b/chrome/browser/google_apis/base_operations.cc
@@ -106,9 +106,8 @@ UrlFetchOperationBase::UrlFetchOperationBase(
UrlFetchOperationBase::UrlFetchOperationBase(
OperationRegistry* registry,
net::URLRequestContextGetter* url_request_context_getter,
- OperationType type,
const base::FilePath& path)
- : OperationRegistry::Operation(registry, type, path),
+ : OperationRegistry::Operation(registry, path),
url_request_context_getter_(url_request_context_getter),
re_authenticate_count_(0),
started_(false),
@@ -390,7 +389,6 @@ InitiateUploadOperationBase::InitiateUploadOperationBase(
int64 content_length)
: UrlFetchOperationBase(registry,
url_request_context_getter,
- OPERATION_UPLOAD,
drive_file_path),
callback_(callback),
drive_file_path_(drive_file_path),
@@ -469,7 +467,6 @@ UploadRangeOperationBase::UploadRangeOperationBase(
const GURL& upload_url)
: UrlFetchOperationBase(registry,
url_request_context_getter,
- OPERATION_UPLOAD,
drive_file_path),
upload_mode_(upload_mode),
drive_file_path_(drive_file_path),
@@ -642,12 +639,6 @@ bool ResumeUploadOperationBase::GetContentData(
return true;
}
-void ResumeUploadOperationBase::OnURLFetchUploadProgress(
- const URLFetcher* source, int64 current, int64 total) {
- // Adjust the progress values according to the range currently uploaded.
- NotifyProgress(start_position_ + current, content_length_);
-}
-
//============================ DownloadFileOperation ===========================
DownloadFileOperation::DownloadFileOperation(
@@ -661,7 +652,6 @@ DownloadFileOperation::DownloadFileOperation(
const base::FilePath& output_file_path)
: UrlFetchOperationBase(registry,
url_request_context_getter,
- OPERATION_DOWNLOAD,
drive_file_path),
download_action_callback_(download_action_callback),
get_content_callback_(get_content_callback),
@@ -687,7 +677,6 @@ GURL DownloadFileOperation::GetURL() const {
void DownloadFileOperation::OnURLFetchDownloadProgress(const URLFetcher* source,
int64 current,
int64 total) {
- NotifyProgress(current, total);
if (!progress_callback_.is_null())
progress_callback_.Run(current, total);
}
diff --git a/chrome/browser/google_apis/base_operations.h b/chrome/browser/google_apis/base_operations.h
index f92ce67..7d17db5 100644
--- a/chrome/browser/google_apis/base_operations.h
+++ b/chrome/browser/google_apis/base_operations.h
@@ -106,7 +106,6 @@ class UrlFetchOperationBase : public AuthenticatedOperationInterface,
UrlFetchOperationBase(
OperationRegistry* registry,
net::URLRequestContextGetter* url_request_context_getter,
- OperationType type,
const base::FilePath& drive_file_path);
virtual ~UrlFetchOperationBase();
@@ -428,10 +427,6 @@ class ResumeUploadOperationBase : public UploadRangeOperationBase {
virtual bool GetContentData(std::string* upload_content_type,
std::string* upload_content) OVERRIDE;
- // content::UrlFetcherDelegate overrides.
- virtual void OnURLFetchUploadProgress(const net::URLFetcher* source,
- int64 current, int64 total) OVERRIDE;
-
private:
// The parameters for the request. See ResumeUploadParams for the details.
const int64 start_position_;
diff --git a/chrome/browser/google_apis/drive_api_operations.cc b/chrome/browser/google_apis/drive_api_operations.cc
index 2382612..cd6ab78 100644
--- a/chrome/browser/google_apis/drive_api_operations.cc
+++ b/chrome/browser/google_apis/drive_api_operations.cc
@@ -568,7 +568,6 @@ void ResumeUploadOperation::OnRangeOperationComplete(
void ResumeUploadOperation::OnURLFetchUploadProgress(
const net::URLFetcher* source, int64 current, int64 total) {
- ResumeUploadOperationBase::OnURLFetchUploadProgress(source, current, total);
if (!progress_callback_.is_null())
progress_callback_.Run(current, total);
}
diff --git a/chrome/browser/google_apis/drive_api_service.cc b/chrome/browser/google_apis/drive_api_service.cc
index c2999f9..0f33120 100644
--- a/chrome/browser/google_apis/drive_api_service.cc
+++ b/chrome/browser/google_apis/drive_api_service.cc
@@ -230,10 +230,8 @@ DriveAPIService::DriveAPIService(
DriveAPIService::~DriveAPIService() {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- if (runner_.get()) {
- runner_->operation_registry()->RemoveObserver(this);
+ if (runner_.get())
runner_->auth_service()->RemoveObserver(this);
- }
}
void DriveAPIService::Initialize(Profile* profile) {
@@ -250,7 +248,6 @@ void DriveAPIService::Initialize(Profile* profile) {
runner_->Initialize();
runner_->auth_service()->AddObserver(this);
- runner_->operation_registry()->AddObserver(this);
}
void DriveAPIService::AddObserver(DriveServiceObserver* observer) {
@@ -277,11 +274,6 @@ bool DriveAPIService::CancelForFilePath(const base::FilePath& file_path) {
return operation_registry()->CancelForFilePath(file_path);
}
-OperationProgressStatusList DriveAPIService::GetProgressStatusList() const {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- return operation_registry()->GetProgressStatusList();
-}
-
std::string DriveAPIService::GetRootResourceId() const {
return kDriveApiRootDirectoryResourceId;
}
@@ -728,11 +720,4 @@ void DriveAPIService::OnOAuth2RefreshTokenChanged() {
}
}
-void DriveAPIService::OnProgressUpdate(
- const OperationProgressStatusList& list) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- FOR_EACH_OBSERVER(
- DriveServiceObserver, observers_, OnProgressUpdate(list));
-}
-
} // namespace google_apis
diff --git a/chrome/browser/google_apis/drive_api_service.h b/chrome/browser/google_apis/drive_api_service.h
index 9b69e8a..e1a8ffb 100644
--- a/chrome/browser/google_apis/drive_api_service.h
+++ b/chrome/browser/google_apis/drive_api_service.h
@@ -32,8 +32,7 @@ class OperationRunner;
// Details of API call are abstracted in each operation class and this class
// works as a thin wrapper for the API.
class DriveAPIService : public DriveServiceInterface,
- public AuthServiceObserver,
- public OperationRegistryObserver {
+ public AuthServiceObserver {
public:
// Instance is usually created by DriveSystemServiceFactory and owned by
// DriveFileSystem.
@@ -59,7 +58,6 @@ class DriveAPIService : public DriveServiceInterface,
virtual bool CanStartOperation() const OVERRIDE;
virtual void CancelAll() OVERRIDE;
virtual bool CancelForFilePath(const base::FilePath& file_path) OVERRIDE;
- virtual OperationProgressStatusList GetProgressStatusList() const OVERRIDE;
virtual bool HasAccessToken() const OVERRIDE;
virtual bool HasRefreshToken() const OVERRIDE;
virtual void ClearAccessToken() OVERRIDE;
@@ -164,10 +162,6 @@ class DriveAPIService : public DriveServiceInterface,
// AuthServiceObserver override.
virtual void OnOAuth2RefreshTokenChanged() OVERRIDE;
- // DriveServiceObserver Overrides
- virtual void OnProgressUpdate(
- const OperationProgressStatusList& list) OVERRIDE;
-
net::URLRequestContextGetter* url_request_context_getter_;
Profile* profile_;
scoped_ptr<OperationRunner> runner_;
diff --git a/chrome/browser/google_apis/drive_service_interface.h b/chrome/browser/google_apis/drive_service_interface.h
index 89102e6..5170c15 100644
--- a/chrome/browser/google_apis/drive_service_interface.h
+++ b/chrome/browser/google_apis/drive_service_interface.h
@@ -32,9 +32,6 @@ class DriveServiceObserver {
// Triggered when the service gets ready to perform operations.
virtual void OnReadyToPerformOperations() {}
- // Called when an operation started, made some progress, or finished.
- virtual void OnProgressUpdate(const OperationProgressStatusList& list) {}
-
// Called when the refresh token was found to be invalid.
virtual void OnRefreshTokenInvalid() {}
@@ -108,9 +105,6 @@ class DriveServiceInterface {
// the operation was found and canceled.
virtual bool CancelForFilePath(const base::FilePath& file_path) = 0;
- // Obtains the list of currently active operations.
- virtual OperationProgressStatusList GetProgressStatusList() const = 0;
-
// Authentication service:
// True if OAuth2 access token is retrieved and believed to be fresh.
diff --git a/chrome/browser/google_apis/dummy_drive_service.cc b/chrome/browser/google_apis/dummy_drive_service.cc
index 29ba179..b978c80 100644
--- a/chrome/browser/google_apis/dummy_drive_service.cc
+++ b/chrome/browser/google_apis/dummy_drive_service.cc
@@ -24,10 +24,6 @@ bool DummyDriveService::CancelForFilePath(const base::FilePath& file_path) {
return true;
}
-OperationProgressStatusList DummyDriveService::GetProgressStatusList() const {
- return OperationProgressStatusList();
-}
-
bool DummyDriveService::HasAccessToken() const { return true; }
bool DummyDriveService::HasRefreshToken() const { return true; }
diff --git a/chrome/browser/google_apis/dummy_drive_service.h b/chrome/browser/google_apis/dummy_drive_service.h
index b1d6ea8..cec7830 100644
--- a/chrome/browser/google_apis/dummy_drive_service.h
+++ b/chrome/browser/google_apis/dummy_drive_service.h
@@ -23,7 +23,6 @@ class DummyDriveService : public DriveServiceInterface {
virtual bool CanStartOperation() const OVERRIDE;
virtual void CancelAll() OVERRIDE;
virtual bool CancelForFilePath(const base::FilePath& file_path) OVERRIDE;
- virtual OperationProgressStatusList GetProgressStatusList() const OVERRIDE;
virtual bool HasAccessToken() const OVERRIDE;
virtual bool HasRefreshToken() const OVERRIDE;
virtual void ClearAccessToken() OVERRIDE;
diff --git a/chrome/browser/google_apis/fake_drive_service.cc b/chrome/browser/google_apis/fake_drive_service.cc
index 1c93dde..04cb7be 100644
--- a/chrome/browser/google_apis/fake_drive_service.cc
+++ b/chrome/browser/google_apis/fake_drive_service.cc
@@ -193,11 +193,6 @@ bool FakeDriveService::CancelForFilePath(const base::FilePath& file_path) {
return true;
}
-OperationProgressStatusList FakeDriveService::GetProgressStatusList() const {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- return OperationProgressStatusList();
-}
-
bool FakeDriveService::HasAccessToken() const {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
return true;
diff --git a/chrome/browser/google_apis/fake_drive_service.h b/chrome/browser/google_apis/fake_drive_service.h
index 4b6a0e9..6762ad6 100644
--- a/chrome/browser/google_apis/fake_drive_service.h
+++ b/chrome/browser/google_apis/fake_drive_service.h
@@ -81,7 +81,6 @@ class FakeDriveService : public DriveServiceInterface {
virtual bool CanStartOperation() const OVERRIDE;
virtual void CancelAll() OVERRIDE;
virtual bool CancelForFilePath(const base::FilePath& file_path) OVERRIDE;
- virtual OperationProgressStatusList GetProgressStatusList() const OVERRIDE;
virtual std::string GetRootResourceId() const OVERRIDE;
virtual bool HasAccessToken() const OVERRIDE;
virtual bool HasRefreshToken() const OVERRIDE;
diff --git a/chrome/browser/google_apis/gdata_wapi_operations.cc b/chrome/browser/google_apis/gdata_wapi_operations.cc
index 6c2c630..35af2f2 100644
--- a/chrome/browser/google_apis/gdata_wapi_operations.cc
+++ b/chrome/browser/google_apis/gdata_wapi_operations.cc
@@ -713,7 +713,6 @@ void ResumeUploadOperation::OnRangeOperationComplete(
void ResumeUploadOperation::OnURLFetchUploadProgress(
const URLFetcher* source, int64 current, int64 total) {
- ResumeUploadOperationBase::OnURLFetchUploadProgress(source, current, total);
if (!progress_callback_.is_null())
progress_callback_.Run(current, total);
}
diff --git a/chrome/browser/google_apis/gdata_wapi_service.cc b/chrome/browser/google_apis/gdata_wapi_service.cc
index 3e224b6..227704f 100644
--- a/chrome/browser/google_apis/gdata_wapi_service.cc
+++ b/chrome/browser/google_apis/gdata_wapi_service.cc
@@ -106,10 +106,8 @@ GDataWapiService::GDataWapiService(
GDataWapiService::~GDataWapiService() {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- if (runner_.get()) {
- runner_->operation_registry()->RemoveObserver(this);
+ if (runner_.get())
runner_->auth_service()->RemoveObserver(this);
- }
}
AuthService* GDataWapiService::auth_service_for_testing() {
@@ -132,7 +130,6 @@ void GDataWapiService::Initialize(Profile* profile) {
runner_->Initialize();
runner_->auth_service()->AddObserver(this);
- runner_->operation_registry()->AddObserver(this);
}
void GDataWapiService::AddObserver(DriveServiceObserver* observer) {
@@ -159,11 +156,6 @@ bool GDataWapiService::CancelForFilePath(const base::FilePath& file_path) {
return operation_registry()->CancelForFilePath(file_path);
}
-OperationProgressStatusList GDataWapiService::GetProgressStatusList() const {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- return operation_registry()->GetProgressStatusList();
-}
-
std::string GDataWapiService::GetRootResourceId() const {
return kWapiRootDirectoryResourceId;
}
@@ -598,11 +590,4 @@ void GDataWapiService::OnOAuth2RefreshTokenChanged() {
}
}
-void GDataWapiService::OnProgressUpdate(
- const OperationProgressStatusList& list) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- FOR_EACH_OBSERVER(
- DriveServiceObserver, observers_, OnProgressUpdate(list));
-}
-
} // namespace google_apis
diff --git a/chrome/browser/google_apis/gdata_wapi_service.h b/chrome/browser/google_apis/gdata_wapi_service.h
index 182e13d..421b3ce 100644
--- a/chrome/browser/google_apis/gdata_wapi_service.h
+++ b/chrome/browser/google_apis/gdata_wapi_service.h
@@ -35,8 +35,7 @@ class OperationRunner;
// Details of API call are abstracted in each operation class and this class
// works as a thin wrapper for the API.
class GDataWapiService : public DriveServiceInterface,
- public AuthServiceObserver,
- public OperationRegistryObserver {
+ public AuthServiceObserver {
public:
// Instance is usually created by DriveSystemServiceFactory and owned by
// DriveFileSystem.
@@ -59,7 +58,6 @@ class GDataWapiService : public DriveServiceInterface,
virtual bool CanStartOperation() const OVERRIDE;
virtual void CancelAll() OVERRIDE;
virtual bool CancelForFilePath(const base::FilePath& file_path) OVERRIDE;
- virtual OperationProgressStatusList GetProgressStatusList() const OVERRIDE;
virtual bool HasAccessToken() const OVERRIDE;
virtual bool HasRefreshToken() const OVERRIDE;
virtual void ClearAccessToken() OVERRIDE;
@@ -163,10 +161,6 @@ class GDataWapiService : public DriveServiceInterface,
// AuthService::Observer override.
virtual void OnOAuth2RefreshTokenChanged() OVERRIDE;
- // DriveServiceObserver Overrides
- virtual void OnProgressUpdate(
- const OperationProgressStatusList& list) OVERRIDE;
-
net::URLRequestContextGetter* url_request_context_getter_; // Not owned.
scoped_ptr<OperationRunner> runner_;
ObserverList<DriveServiceObserver> observers_;
diff --git a/chrome/browser/google_apis/mock_drive_service.cc b/chrome/browser/google_apis/mock_drive_service.cc
index 44bf0d2..13d1f55 100644
--- a/chrome/browser/google_apis/mock_drive_service.cc
+++ b/chrome/browser/google_apis/mock_drive_service.cc
@@ -23,8 +23,6 @@ using ::testing::Return;
namespace google_apis {
MockDriveService::MockDriveService() {
- ON_CALL(*this, GetProgressStatusList())
- .WillByDefault(Return(OperationProgressStatusList()));
ON_CALL(*this, GetChangeList(_, _))
.WillByDefault(Invoke(this, &MockDriveService::GetChangeListStub));
ON_CALL(*this, GetAccountMetadata(_))
diff --git a/chrome/browser/google_apis/mock_drive_service.h b/chrome/browser/google_apis/mock_drive_service.h
index 9e260c2..f73ce3d 100644
--- a/chrome/browser/google_apis/mock_drive_service.h
+++ b/chrome/browser/google_apis/mock_drive_service.h
@@ -34,8 +34,6 @@ class MockDriveService : public DriveServiceInterface {
MOCK_CONST_METHOD0(CanStartOperation, bool());
MOCK_METHOD0(CancelAll, void(void));
MOCK_METHOD1(CancelForFilePath, bool(const base::FilePath& file_path));
- MOCK_CONST_METHOD0(GetProgressStatusList,
- OperationProgressStatusList());
MOCK_CONST_METHOD0(GetRootResourceId, std::string());
MOCK_METHOD1(GetAllResourceList,
void(const GetResourceListCallback& callback));
diff --git a/chrome/browser/google_apis/operation_registry.cc b/chrome/browser/google_apis/operation_registry.cc
index e11a120..4b4a86c 100644
--- a/chrome/browser/google_apis/operation_registry.cc
+++ b/chrome/browser/google_apis/operation_registry.cc
@@ -4,80 +4,27 @@
#include "chrome/browser/google_apis/operation_registry.h"
-#include "base/strings/string_number_conversions.h"
#include "content/public/browser/browser_thread.h"
using content::BrowserThread;
-namespace {
-
-const int64 kNotificationFrequencyInMilliseconds = 1000;
-
-} // namespace
-
namespace google_apis {
-std::string OperationTypeToString(OperationType type) {
- switch (type) {
- case OPERATION_UPLOAD: return "upload";
- case OPERATION_DOWNLOAD: return "download";
- case OPERATION_OTHER: return "other";
- }
- NOTREACHED();
- return "unknown_transfer_state";
-}
-
-std::string OperationTransferStateToString(OperationTransferState state) {
- switch (state) {
- case OPERATION_NOT_STARTED: return "not_started";
- case OPERATION_STARTED: return "started";
- case OPERATION_IN_PROGRESS: return "in_progress";
- case OPERATION_COMPLETED: return "completed";
- case OPERATION_FAILED: return "failed";
- // Suspended state is opaque to users and looks as same as "in_progress".
- case OPERATION_SUSPENDED: return "in_progress";
- }
- NOTREACHED();
- return "unknown_transfer_state";
-}
-
-OperationProgressStatus::OperationProgressStatus(OperationType type,
- const base::FilePath& path)
+OperationProgressStatus::OperationProgressStatus(const base::FilePath& path)
: operation_id(-1),
- operation_type(type),
file_path(path),
- transfer_state(OPERATION_NOT_STARTED),
- progress_current(0),
- progress_total(-1) {
-}
-
-std::string OperationProgressStatus::DebugString() const {
- std::string str;
- str += "id=";
- str += base::IntToString(operation_id);
- str += " type=";
- str += OperationTypeToString(operation_type);
- str += " path=";
- str += file_path.AsUTF8Unsafe();
- str += " state=";
- str += OperationTransferStateToString(transfer_state);
- str += " progress=";
- str += base::Int64ToString(progress_current);
- str += "/";
- str += base::Int64ToString(progress_total);
- return str;
+ transfer_state(OPERATION_NOT_STARTED) {
}
OperationRegistry::Operation::Operation(OperationRegistry* registry)
: registry_(registry),
- progress_status_(OPERATION_OTHER, base::FilePath()) {
+ progress_status_(base::FilePath()) {
}
OperationRegistry::Operation::Operation(OperationRegistry* registry,
- OperationType type,
const base::FilePath& path)
: registry_(registry),
- progress_status_(type, path) {
+ progress_status_(path) {
}
OperationRegistry::Operation::~Operation() {
@@ -96,21 +43,10 @@ void OperationRegistry::Operation::NotifyStart() {
// Some operations may be restarted. Report only the first "start".
if (progress_status_.transfer_state == OPERATION_NOT_STARTED) {
progress_status_.transfer_state = OPERATION_STARTED;
- progress_status_.start_time = base::Time::Now();
registry_->OnOperationStart(this, &progress_status_.operation_id);
}
}
-void OperationRegistry::Operation::NotifyProgress(
- int64 current, int64 total) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- DCHECK(progress_status_.transfer_state >= OPERATION_STARTED);
- progress_status_.transfer_state = OPERATION_IN_PROGRESS;
- progress_status_.progress_current = current;
- progress_status_.progress_total = total;
- registry_->OnOperationProgress(progress_status().operation_id);
-}
-
void OperationRegistry::Operation::NotifyFinish(
OperationTransferState status) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -135,8 +71,7 @@ void OperationRegistry::Operation::NotifyResume() {
}
}
-OperationRegistry::OperationRegistry()
- : do_notification_frequency_control_(true) {
+OperationRegistry::OperationRegistry() {
in_flight_operations_.set_check_on_null_data(true);
}
@@ -144,18 +79,6 @@ OperationRegistry::~OperationRegistry() {
DCHECK(in_flight_operations_.IsEmpty());
}
-void OperationRegistry::AddObserver(OperationRegistryObserver* observer) {
- observer_list_.AddObserver(observer);
-}
-
-void OperationRegistry::RemoveObserver(OperationRegistryObserver* observer) {
- observer_list_.RemoveObserver(observer);
-}
-
-void OperationRegistry::DisableNotificationFrequencyControlForTest() {
- do_notification_frequency_control_ = false;
-}
-
void OperationRegistry::CancelAll() {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -205,20 +128,6 @@ void OperationRegistry::OnOperationStart(
*id = in_flight_operations_.Add(operation);
DVLOG(1) << "GDataOperation[" << *id << "] started.";
- if (IsFileTransferOperation(operation))
- NotifyStatusToObservers();
-}
-
-void OperationRegistry::OnOperationProgress(OperationID id) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
- Operation* operation = in_flight_operations_.Lookup(id);
- DCHECK(operation);
-
- DVLOG(1) << "GDataOperation[" << id << "] "
- << operation->progress_status().DebugString();
- if (IsFileTransferOperation(operation))
- NotifyStatusToObservers();
}
void OperationRegistry::OnOperationFinish(OperationID id) {
@@ -228,8 +137,6 @@ void OperationRegistry::OnOperationFinish(OperationID id) {
DCHECK(operation);
DVLOG(1) << "GDataOperation[" << id << "] finished.";
- if (IsFileTransferOperation(operation))
- NotifyStatusToObservers();
in_flight_operations_.Remove(id);
}
@@ -265,21 +172,13 @@ void OperationRegistry::OnOperationResume(
return;
}
- // Copy the progress status.
+ // Remove the old one and initiate the new operation.
const OperationProgressStatus& old_status = suspended->progress_status();
OperationID old_id = old_status.operation_id;
-
- new_status->progress_current = old_status.progress_current;
- new_status->progress_total = old_status.progress_total;
- new_status->start_time = old_status.start_time;
-
- // Remove the old one and initiate the new operation.
in_flight_operations_.Remove(old_id);
new_status->operation_id = in_flight_operations_.Add(operation);
DVLOG(1) << "GDataOperation[" << old_id << " -> " <<
new_status->operation_id << "] resumed.";
- if (IsFileTransferOperation(operation))
- NotifyStatusToObservers();
}
void OperationRegistry::OnOperationSuspend(OperationID id) {
@@ -289,71 +188,6 @@ void OperationRegistry::OnOperationSuspend(OperationID id) {
DCHECK(operation);
DVLOG(1) << "GDataOperation[" << id << "] suspended.";
- if (IsFileTransferOperation(operation))
- NotifyStatusToObservers();
-}
-
-bool OperationRegistry::IsFileTransferOperation(
- const Operation* operation) const {
- OperationType type = operation->progress_status().operation_type;
- return type == OPERATION_UPLOAD || type == OPERATION_DOWNLOAD;
-}
-
-OperationProgressStatusList OperationRegistry::GetProgressStatusList() {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
- OperationProgressStatusList status_list;
- for (OperationIDMap::const_iterator iter(&in_flight_operations_);
- !iter.IsAtEnd();
- iter.Advance()) {
- const Operation* operation = iter.GetCurrentValue();
- if (IsFileTransferOperation(operation))
- status_list.push_back(operation->progress_status());
- }
- return status_list;
-}
-
-bool OperationRegistry::ShouldNotifyStatusNow(
- const OperationProgressStatusList& list) {
- if (!do_notification_frequency_control_)
- return true;
-
- base::Time now = base::Time::Now();
-
- // If it is a first event, or some time abnormality is detected, we should
- // not skip this notification.
- if (last_notification_.is_null() || now < last_notification_) {
- last_notification_ = now;
- return true;
- }
-
- // If sufficiently long time has elapsed since the previous event, we should
- // not skip this notification.
- if ((now - last_notification_).InMilliseconds() >=
- kNotificationFrequencyInMilliseconds) {
- last_notification_ = now;
- return true;
- }
-
- // If important events (OPERATION_STARTED, COMPLETED, or FAILED) are there,
- // we should not skip this notification.
- for (size_t i = 0; i < list.size(); ++i) {
- if (list[i].transfer_state != OPERATION_IN_PROGRESS) {
- last_notification_ = now;
- return true;
- }
- }
-
- // Otherwise we can skip it.
- return false;
-}
-
-void OperationRegistry::NotifyStatusToObservers() {
- OperationProgressStatusList list(GetProgressStatusList());
- if (ShouldNotifyStatusNow(list))
- FOR_EACH_OBSERVER(OperationRegistryObserver,
- observer_list_,
- OnProgressUpdate(list));
}
} // namespace google_apis
diff --git a/chrome/browser/google_apis/operation_registry.h b/chrome/browser/google_apis/operation_registry.h
index 36a6373..f6ab834 100644
--- a/chrome/browser/google_apis/operation_registry.h
+++ b/chrome/browser/google_apis/operation_registry.h
@@ -5,30 +5,16 @@
#ifndef CHROME_BROWSER_GOOGLE_APIS_OPERATION_REGISTRY_H_
#define CHROME_BROWSER_GOOGLE_APIS_OPERATION_REGISTRY_H_
-#include <string>
-#include <vector>
-
#include "base/basictypes.h"
#include "base/files/file_path.h"
#include "base/id_map.h"
-#include "base/observer_list.h"
-#include "base/time.h"
#include "chrome/browser/google_apis/gdata_errorcode.h"
namespace google_apis {
-class OperationRegistryObserver;
-
// Unique ID to identify each operation.
typedef int32 OperationID;
-// Enumeration type for indicating the direction of the operation.
-enum OperationType {
- OPERATION_UPLOAD,
- OPERATION_DOWNLOAD,
- OPERATION_OTHER,
-};
-
// Enumeration type for indicating the state of the transfer.
enum OperationTransferState {
OPERATION_NOT_STARTED,
@@ -39,35 +25,20 @@ enum OperationTransferState {
OPERATION_SUSPENDED,
};
-// Returns string representations of the operation type and state, which are
-// exposed to the private extension API as in:
-// operation.chrome/common/extensions/api/file_browser_private.json
-std::string OperationTypeToString(OperationType type);
+// Returns string representations of the operation state.
std::string OperationTransferStateToString(OperationTransferState state);
// Structure that packs progress information of each operation.
struct OperationProgressStatus {
- OperationProgressStatus(OperationType type, const base::FilePath& file_path);
- // For debugging
- std::string DebugString() const;
+ explicit OperationProgressStatus(const base::FilePath& file_path);
OperationID operation_id;
- // Type of the operation: upload/download.
- OperationType operation_type;
// GData path of the file dealt with the current operation.
base::FilePath file_path;
// Current state of the transfer;
OperationTransferState transfer_state;
- // The time when the operation is initiated.
- base::Time start_time;
- // Current fraction of progress of the operation.
- int64 progress_current;
- // Expected total number of bytes to be transferred in the operation.
- // -1 if no expectation is available (yet).
- int64 progress_total;
};
-typedef std::vector<OperationProgressStatus> OperationProgressStatusList;
// This class tracks all the in-flight GData operation objects and manage their
// lifetime.
@@ -83,9 +54,7 @@ class OperationRegistry {
class Operation {
public:
explicit Operation(OperationRegistry* registry);
- Operation(OperationRegistry* registry,
- OperationType type,
- const base::FilePath& file_path);
+ Operation(OperationRegistry* registry, const base::FilePath& file_path);
virtual ~Operation();
// Cancels the ongoing operation. NotifyFinish() is called and the Operation
@@ -100,7 +69,6 @@ class OperationRegistry {
protected:
// Notifies the registry about current status.
void NotifyStart();
- void NotifyProgress(int64 current, int64 total);
void NotifyFinish(OperationTransferState status);
// Notifies suspend/resume, used for chunked upload operations.
// The initial upload operation should issue "start" "progress"* "suspend".
@@ -127,60 +95,26 @@ class OperationRegistry {
// the operation was found and canceled.
bool CancelForFilePath(const base::FilePath& file_path);
- // Obtains the list of currently active operations.
- OperationProgressStatusList GetProgressStatusList();
-
- // Sets an observer. The registry do NOT own observers; before destruction
- // they need to be removed from the registry.
- void AddObserver(OperationRegistryObserver* observer);
- void RemoveObserver(OperationRegistryObserver* observer);
-
- // Disables the notification suppression for testing purpose.
- void DisableNotificationFrequencyControlForTest();
-
private:
// Handlers for notifications from Operations.
friend class Operation;
// Notifies that an operation has started. This method passes the ownership of
// the operation to the registry. A fresh operation ID is returned to *id.
void OnOperationStart(Operation* operation, OperationID* id);
- void OnOperationProgress(OperationID operation);
void OnOperationFinish(OperationID operation);
void OnOperationSuspend(OperationID operation);
void OnOperationResume(Operation* operation,
OperationProgressStatus* new_status);
- bool IsFileTransferOperation(const Operation* operation) const;
-
- // Controls the frequency of notifications, not to flood the listeners with
- // too many events.
- bool ShouldNotifyStatusNow(const OperationProgressStatusList& list);
- // Sends notifications to the observers after checking that the frequency is
- // not too high by ShouldNotifyStatusNow.
- void NotifyStatusToObservers();
-
// Cancels the specified operation.
void CancelOperation(Operation* operation);
typedef IDMap<Operation, IDMapOwnPointer> OperationIDMap;
OperationIDMap in_flight_operations_;
- ObserverList<OperationRegistryObserver> observer_list_;
- base::Time last_notification_;
- bool do_notification_frequency_control_;
DISALLOW_COPY_AND_ASSIGN(OperationRegistry);
};
-// Observer interface for listening changes in the active set of operations.
-class OperationRegistryObserver {
- public:
- // Called when a GData operation started, made some progress, or finished.
- virtual void OnProgressUpdate(const OperationProgressStatusList& list) {}
-
- protected:
- virtual ~OperationRegistryObserver() {}
-};
-
} // namespace google_apis
#endif // CHROME_BROWSER_GOOGLE_APIS_OPERATION_REGISTRY_H_
diff --git a/chrome/browser/google_apis/operation_registry_unittest.cc b/chrome/browser/google_apis/operation_registry_unittest.cc
index 716d2ed..183d7ac 100644
--- a/chrome/browser/google_apis/operation_registry_unittest.cc
+++ b/chrome/browser/google_apis/operation_registry_unittest.cc
@@ -12,8 +12,6 @@
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
-using testing::ElementsAre;
-
namespace google_apis {
namespace {
@@ -21,102 +19,23 @@ namespace {
class MockOperation : public OperationRegistry::Operation,
public base::SupportsWeakPtr<MockOperation> {
public:
- MockOperation(OperationRegistry* registry,
- OperationType type,
- const base::FilePath& path)
- : OperationRegistry::Operation(registry, type, path) {}
+ explicit MockOperation(OperationRegistry* registry)
+ : OperationRegistry::Operation(
+ registry,
+ base::FilePath(FILE_PATH_LITERAL("/dummy/download"))) {
+ }
MOCK_METHOD0(DoCancel, void());
// Make them public so that they can be called from test cases.
using OperationRegistry::Operation::NotifyStart;
- using OperationRegistry::Operation::NotifyProgress;
using OperationRegistry::Operation::NotifyFinish;
using OperationRegistry::Operation::NotifySuspend;
using OperationRegistry::Operation::NotifyResume;
};
-class MockUploadOperation : public MockOperation {
- public:
- explicit MockUploadOperation(OperationRegistry* registry)
- : MockOperation(registry,
- OPERATION_UPLOAD,
- base::FilePath(FILE_PATH_LITERAL("/dummy/upload"))) {}
-};
-
-class MockDownloadOperation : public MockOperation {
- public:
- explicit MockDownloadOperation(OperationRegistry* registry)
- : MockOperation(registry,
- OPERATION_DOWNLOAD,
- base::FilePath(FILE_PATH_LITERAL("/dummy/download"))) {}
-};
-
-class MockOtherOperation : public MockOperation {
- public:
- explicit MockOtherOperation(OperationRegistry* registry)
- : MockOperation(registry,
- OPERATION_OTHER,
- base::FilePath(FILE_PATH_LITERAL("/dummy/other"))) {}
-};
-
-class TestObserver : public OperationRegistryObserver {
- public:
- virtual void OnProgressUpdate(
- const OperationProgressStatusList& list) OVERRIDE {
- status_ = list;
- }
-
- const OperationProgressStatusList& status() const {
- return status_;
- }
-
- private:
- OperationProgressStatusList status_;
-};
-
-class ProgressMatcher
- : public ::testing::MatcherInterface<const OperationProgressStatus&> {
- public:
- ProgressMatcher(int64 expected_current, int64 expected_total)
- : expected_current_(expected_current),
- expected_total_(expected_total) {}
-
- virtual bool MatchAndExplain(
- const OperationProgressStatus& status,
- testing::MatchResultListener* /* listener */) const OVERRIDE {
- return status.progress_current == expected_current_ &&
- status.progress_total == expected_total_;
- }
-
- virtual void DescribeTo(::std::ostream* os) const OVERRIDE {
- *os << "current / total equals " << expected_current_ << " / " <<
- expected_total_;
- }
-
- virtual void DescribeNegationTo(::std::ostream* os) const OVERRIDE {
- *os << "current / total does not equal " << expected_current_ << " / " <<
- expected_total_;
- }
-
- private:
- const int64 expected_current_;
- const int64 expected_total_;
-};
-
-testing::Matcher<const OperationProgressStatus&> Progress(
- int64 current, int64 total) {
- return testing::MakeMatcher(new ProgressMatcher(current, total));
-}
-
} // namespace
-// Pretty-prints OperationProgressStatus for testing purpose.
-std::ostream& operator<<(std::ostream& os,
- const OperationProgressStatus& status) {
- return os << status.DebugString();
-}
-
class OperationRegistryTest : public testing::Test {
protected:
OperationRegistryTest()
@@ -127,141 +46,62 @@ class OperationRegistryTest : public testing::Test {
};
TEST_F(OperationRegistryTest, OneSuccess) {
- TestObserver observer;
OperationRegistry registry;
- registry.DisableNotificationFrequencyControlForTest();
- registry.AddObserver(&observer);
base::WeakPtr<MockOperation> op1 =
- (new MockUploadOperation(&registry))->AsWeakPtr();
+ (new MockOperation(&registry))->AsWeakPtr();
EXPECT_CALL(*op1, DoCancel()).Times(0);
- EXPECT_EQ(0U, observer.status().size());
op1->NotifyStart();
- EXPECT_THAT(observer.status(), ElementsAre(Progress(0, -1)));
- op1->NotifyProgress(0, 100);
- EXPECT_THAT(observer.status(), ElementsAre(Progress(0, 100)));
- op1->NotifyProgress(100, 100);
- EXPECT_THAT(observer.status(), ElementsAre(Progress(100, 100)));
op1->NotifyFinish(OPERATION_COMPLETED);
- // Contains one "COMPLETED" notification.
- EXPECT_THAT(observer.status(), ElementsAre(Progress(100, 100)));
- // Then it is removed.
- EXPECT_EQ(0U, registry.GetProgressStatusList().size());
EXPECT_EQ(NULL, op1.get()); // deleted
}
TEST_F(OperationRegistryTest, OneCancel) {
- TestObserver observer;
OperationRegistry registry;
- registry.DisableNotificationFrequencyControlForTest();
- registry.AddObserver(&observer);
base::WeakPtr<MockOperation> op1 =
- (new MockUploadOperation(&registry))->AsWeakPtr();
+ (new MockOperation(&registry))->AsWeakPtr();
EXPECT_CALL(*op1, DoCancel());
- EXPECT_EQ(0U, observer.status().size());
op1->NotifyStart();
- EXPECT_THAT(observer.status(), ElementsAre(Progress(0, -1)));
- op1->NotifyProgress(0, 100);
- EXPECT_THAT(observer.status(), ElementsAre(Progress(0, 100)));
registry.CancelAll();
- EXPECT_THAT(observer.status(), ElementsAre(Progress(0, 100)));
- EXPECT_EQ(0U, registry.GetProgressStatusList().size());
EXPECT_EQ(NULL, op1.get()); // deleted
}
TEST_F(OperationRegistryTest, TwoSuccess) {
- TestObserver observer;
OperationRegistry registry;
- registry.DisableNotificationFrequencyControlForTest();
- registry.AddObserver(&observer);
base::WeakPtr<MockOperation> op1 =
- (new MockUploadOperation(&registry))->AsWeakPtr();
+ (new MockOperation(&registry))->AsWeakPtr();
base::WeakPtr<MockOperation> op2 =
- (new MockDownloadOperation(&registry))->AsWeakPtr();
+ (new MockOperation(&registry))->AsWeakPtr();
EXPECT_CALL(*op1, DoCancel()).Times(0);
EXPECT_CALL(*op2, DoCancel()).Times(0);
- EXPECT_EQ(0U, observer.status().size());
op1->NotifyStart();
- op1->NotifyProgress(0, 100);
- EXPECT_THAT(observer.status(), ElementsAre(Progress(0, 100)));
op2->NotifyStart();
- op2->NotifyProgress(0, 200);
- op1->NotifyProgress(50, 100);
- EXPECT_THAT(observer.status(), ElementsAre(Progress(50, 100),
- Progress(0, 200)));
op1->NotifyFinish(OPERATION_COMPLETED);
- EXPECT_THAT(observer.status(), ElementsAre(Progress(50, 100),
- Progress(0, 200)));
- EXPECT_EQ(1U, registry.GetProgressStatusList().size());
op2->NotifyFinish(OPERATION_COMPLETED);
- EXPECT_THAT(observer.status(), ElementsAre(Progress(0, 200)));
- EXPECT_EQ(0U, registry.GetProgressStatusList().size());
EXPECT_EQ(NULL, op1.get()); // deleted
EXPECT_EQ(NULL, op2.get()); // deleted
}
-TEST_F(OperationRegistryTest, ThreeCancel) {
- TestObserver observer;
- OperationRegistry registry;
- registry.DisableNotificationFrequencyControlForTest();
- registry.AddObserver(&observer);
-
- base::WeakPtr<MockOperation> op1 =
- (new MockUploadOperation(&registry))->AsWeakPtr();
- base::WeakPtr<MockOperation> op2 =
- (new MockDownloadOperation(&registry))->AsWeakPtr();
- base::WeakPtr<MockOperation> op3 =
- (new MockOtherOperation(&registry))->AsWeakPtr();
- EXPECT_CALL(*op1, DoCancel());
- EXPECT_CALL(*op2, DoCancel());
- EXPECT_CALL(*op3, DoCancel());
-
- EXPECT_EQ(0U, observer.status().size());
- op1->NotifyStart();
- EXPECT_EQ(1U, observer.status().size());
- op2->NotifyStart();
- EXPECT_EQ(2U, observer.status().size());
- op3->NotifyStart();
- EXPECT_EQ(2U, observer.status().size()); // only upload/download is reported.
- registry.CancelAll();
- EXPECT_EQ(1U, observer.status().size()); // holds the last one "COMPLETED"
- EXPECT_EQ(0U, registry.GetProgressStatusList().size());
- EXPECT_EQ(NULL, op1.get()); // deleted
- EXPECT_EQ(NULL, op2.get()); // deleted
- EXPECT_EQ(NULL, op3.get()); // deleted. CancelAll cares all operations.
-}
-
TEST_F(OperationRegistryTest, RestartOperation) {
- TestObserver observer;
OperationRegistry registry;
- registry.DisableNotificationFrequencyControlForTest();
- registry.AddObserver(&observer);
base::WeakPtr<MockOperation> op1 =
- (new MockUploadOperation(&registry))->AsWeakPtr();
+ (new MockOperation(&registry))->AsWeakPtr();
EXPECT_CALL(*op1, DoCancel()).Times(0);
op1->NotifyStart();
- EXPECT_EQ(1U, registry.GetProgressStatusList().size());
op1->NotifyStart(); // restart
- EXPECT_EQ(1U, registry.GetProgressStatusList().size());
- op1->NotifyProgress(0, 200);
op1->NotifyFinish(OPERATION_COMPLETED);
- EXPECT_EQ(0U, registry.GetProgressStatusList().size());
EXPECT_EQ(NULL, op1.get()); // deleted
}
-
TEST_F(OperationRegistryTest, SuspendCancel) {
- TestObserver observer;
OperationRegistry registry;
- registry.DisableNotificationFrequencyControlForTest();
- registry.AddObserver(&observer);
// Suspend-then-resume is a hack in OperationRegistry to tie physically
// split but logically single operation (= chunked uploading split into
@@ -274,7 +114,7 @@ TEST_F(OperationRegistryTest, SuspendCancel) {
// callback must not be called more than once.
base::WeakPtr<MockOperation> op1 =
- (new MockUploadOperation(&registry))->AsWeakPtr();
+ (new MockOperation(&registry))->AsWeakPtr();
EXPECT_CALL(*op1, DoCancel()).Times(0);
op1->NotifyStart();
@@ -283,7 +123,7 @@ TEST_F(OperationRegistryTest, SuspendCancel) {
EXPECT_EQ(NULL, op1.get()); // deleted
base::WeakPtr<MockOperation> op2 =
- (new MockUploadOperation(&registry))->AsWeakPtr();
+ (new MockOperation(&registry))->AsWeakPtr();
EXPECT_CALL(*op2, DoCancel()).Times(1);
op2->NotifyResume();