diff options
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(®istry))->AsWeakPtr(); + (new MockOperation(®istry))->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(®istry))->AsWeakPtr(); + (new MockOperation(®istry))->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(®istry))->AsWeakPtr(); + (new MockOperation(®istry))->AsWeakPtr(); base::WeakPtr<MockOperation> op2 = - (new MockDownloadOperation(®istry))->AsWeakPtr(); + (new MockOperation(®istry))->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(®istry))->AsWeakPtr(); - base::WeakPtr<MockOperation> op2 = - (new MockDownloadOperation(®istry))->AsWeakPtr(); - base::WeakPtr<MockOperation> op3 = - (new MockOtherOperation(®istry))->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(®istry))->AsWeakPtr(); + (new MockOperation(®istry))->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(®istry))->AsWeakPtr(); + (new MockOperation(®istry))->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(®istry))->AsWeakPtr(); + (new MockOperation(®istry))->AsWeakPtr(); EXPECT_CALL(*op2, DoCancel()).Times(1); op2->NotifyResume(); |