Commit f9648891 authored by maniscalco's avatar maniscalco Committed by Commit bot

[Sync] Remove DropAttachments from AttachmentService and SyncData

AttachmentService and SyncData no longer need to provide a drop method
beecause model type code can drop attachments using its AttachmentStore.

BUG=

Review URL: https://codereview.chromium.org/973513002

Cr-Commit-Position: refs/heads/master@{#318981}
parent 718dcc5b
...@@ -218,10 +218,4 @@ void SyncDataRemote::GetOrDownloadAttachments( ...@@ -218,10 +218,4 @@ void SyncDataRemote::GetOrDownloadAttachments(
attachment_service_.GetOrDownloadAttachments(attachment_ids, callback); attachment_service_.GetOrDownloadAttachments(attachment_ids, callback);
} }
void SyncDataRemote::DropAttachments(
const AttachmentIdList& attachment_ids,
const AttachmentService::DropCallback& callback) {
attachment_service_.DropAttachments(attachment_ids, callback);
}
} // namespace syncer } // namespace syncer
...@@ -190,15 +190,6 @@ class SYNC_EXPORT SyncDataRemote : public SyncData { ...@@ -190,15 +190,6 @@ class SYNC_EXPORT SyncDataRemote : public SyncData {
void GetOrDownloadAttachments( void GetOrDownloadAttachments(
const AttachmentIdList& attachment_ids, const AttachmentIdList& attachment_ids,
const AttachmentService::GetOrDownloadCallback& callback); const AttachmentService::GetOrDownloadCallback& callback);
// Drop (delete from local storage) the attachments associated with this
// SyncData specified in |attachment_ids|. This method will not delete
// attachments from the server.
//
// |callback| will be invoked when the operation is complete (successfully
// or otherwise).
void DropAttachments(const AttachmentIdList& attachment_ids,
const AttachmentService::DropCallback& callback);
}; };
// gmock printer helper. // gmock printer helper.
......
...@@ -117,8 +117,8 @@ TEST_F(SyncDataTest, CreateRemoteData) { ...@@ -117,8 +117,8 @@ TEST_F(SyncDataTest, CreateRemoteData) {
EXPECT_TRUE(data.GetAttachmentIds().empty()); EXPECT_TRUE(data.GetAttachmentIds().empty());
} }
// TODO(maniscalco): Add test cases that verify GetLocalAttachmentsForUpload and // TODO(maniscalco): Add test cases that verify GetLocalAttachmentsForUpload
// DropAttachments calls are passed through to the underlying AttachmentService. // calls are passed through to the underlying AttachmentService.
} // namespace } // namespace
......
...@@ -176,16 +176,6 @@ void AttachmentServiceImpl::GetOrDownloadAttachments( ...@@ -176,16 +176,6 @@ void AttachmentServiceImpl::GetOrDownloadAttachments(
state)); state));
} }
void AttachmentServiceImpl::DropAttachments(
const AttachmentIdList& attachment_ids,
const DropCallback& callback) {
DCHECK(CalledOnValidThread());
attachment_store_->Drop(attachment_ids,
base::Bind(&AttachmentServiceImpl::DropDone,
weak_ptr_factory_.GetWeakPtr(),
callback));
}
void AttachmentServiceImpl::ReadDone( void AttachmentServiceImpl::ReadDone(
const scoped_refptr<GetOrDownloadState>& state, const scoped_refptr<GetOrDownloadState>& state,
const AttachmentStore::Result& result, const AttachmentStore::Result& result,
...@@ -234,18 +224,6 @@ void AttachmentServiceImpl::WriteDone( ...@@ -234,18 +224,6 @@ void AttachmentServiceImpl::WriteDone(
} }
} }
void AttachmentServiceImpl::DropDone(const DropCallback& callback,
const AttachmentStore::Result& result) {
AttachmentService::DropResult drop_result =
AttachmentService::DROP_UNSPECIFIED_ERROR;
if (result == AttachmentStore::SUCCESS) {
drop_result = AttachmentService::DROP_SUCCESS;
}
// TODO(maniscalco): Deal with case where an error occurred (bug 361251).
base::MessageLoop::current()->PostTask(FROM_HERE,
base::Bind(callback, drop_result));
}
void AttachmentServiceImpl::UploadDone( void AttachmentServiceImpl::UploadDone(
const AttachmentUploader::UploadResult& result, const AttachmentUploader::UploadResult& result,
const AttachmentId& attachment_id) { const AttachmentId& attachment_id) {
......
...@@ -26,15 +26,6 @@ void ProxyGetOrDownloadCallback( ...@@ -26,15 +26,6 @@ void ProxyGetOrDownloadCallback(
FROM_HERE, base::Bind(callback, result, base::Passed(&attachments))); FROM_HERE, base::Bind(callback, result, base::Passed(&attachments)));
} }
// Invokes |callback| with |result| and |attachments| in the |task_runner|
// thread.
void ProxyDropCallback(
const scoped_refptr<base::SequencedTaskRunner>& task_runner,
const AttachmentService::DropCallback& callback,
const AttachmentService::DropResult& result) {
task_runner->PostTask(FROM_HERE, base::Bind(callback, result));
}
} // namespace } // namespace
AttachmentServiceProxy::AttachmentServiceProxy() { AttachmentServiceProxy::AttachmentServiceProxy() {
...@@ -78,19 +69,6 @@ void AttachmentServiceProxy::GetOrDownloadAttachments( ...@@ -78,19 +69,6 @@ void AttachmentServiceProxy::GetOrDownloadAttachments(
proxy_callback)); proxy_callback));
} }
void AttachmentServiceProxy::DropAttachments(
const AttachmentIdList& attachment_ids,
const DropCallback& callback) {
DCHECK(wrapped_task_runner_.get());
DropCallback proxy_callback = base::Bind(
&ProxyDropCallback, base::ThreadTaskRunnerHandle::Get(), callback);
wrapped_task_runner_->PostTask(FROM_HERE,
base::Bind(&AttachmentService::DropAttachments,
core_,
attachment_ids,
proxy_callback));
}
void AttachmentServiceProxy::UploadAttachments( void AttachmentServiceProxy::UploadAttachments(
const AttachmentIdSet& attachment_ids) { const AttachmentIdSet& attachment_ids) {
DCHECK(wrapped_task_runner_.get()); DCHECK(wrapped_task_runner_.get());
...@@ -120,15 +98,6 @@ void AttachmentServiceProxy::Core::GetOrDownloadAttachments( ...@@ -120,15 +98,6 @@ void AttachmentServiceProxy::Core::GetOrDownloadAttachments(
wrapped_->GetOrDownloadAttachments(attachment_ids, callback); wrapped_->GetOrDownloadAttachments(attachment_ids, callback);
} }
void AttachmentServiceProxy::Core::DropAttachments(
const AttachmentIdList& attachment_ids,
const DropCallback& callback) {
if (!wrapped_) {
return;
}
wrapped_->DropAttachments(attachment_ids, callback);
}
void AttachmentServiceProxy::Core::UploadAttachments( void AttachmentServiceProxy::Core::UploadAttachments(
const AttachmentIdSet& attachment_ids) { const AttachmentIdSet& attachment_ids) {
if (!wrapped_) { if (!wrapped_) {
......
...@@ -48,14 +48,6 @@ class StubAttachmentService : public AttachmentService, ...@@ -48,14 +48,6 @@ class StubAttachmentService : public AttachmentService,
base::Passed(&attachments))); base::Passed(&attachments)));
} }
void DropAttachments(const AttachmentIdList& attachment_ids,
const DropCallback& callback) override {
CalledOnValidThread();
Increment();
base::MessageLoop::current()->PostTask(
FROM_HERE, base::Bind(callback, AttachmentService::DROP_SUCCESS));
}
void UploadAttachments(const AttachmentIdSet& attachments_ids) override { void UploadAttachments(const AttachmentIdSet& attachments_ids) override {
CalledOnValidThread(); CalledOnValidThread();
Increment(); Increment();
...@@ -101,10 +93,7 @@ class AttachmentServiceProxyTest : public testing::Test, ...@@ -101,10 +93,7 @@ class AttachmentServiceProxyTest : public testing::Test,
callback_get_or_download = callback_get_or_download =
base::Bind(&AttachmentServiceProxyTest::IncrementGetOrDownload, base::Bind(&AttachmentServiceProxyTest::IncrementGetOrDownload,
base::Unretained(this)); base::Unretained(this));
callback_drop = base::Bind(&AttachmentServiceProxyTest::IncrementDrop,
base::Unretained(this));
count_callback_get_or_download = 0; count_callback_get_or_download = 0;
count_callback_drop = 0;
} }
void TearDown() override { void TearDown() override {
...@@ -124,12 +113,6 @@ class AttachmentServiceProxyTest : public testing::Test, ...@@ -124,12 +113,6 @@ class AttachmentServiceProxyTest : public testing::Test,
++count_callback_get_or_download; ++count_callback_get_or_download;
} }
// a DropCallback
void IncrementDrop(const AttachmentService::DropResult&) {
CalledOnValidThread();
++count_callback_drop;
}
void WaitForStubThread() { void WaitForStubThread() {
base::WaitableEvent done(false, false); base::WaitableEvent done(false, false);
stub_thread->message_loop()->PostTask( stub_thread->message_loop()->PostTask(
...@@ -144,12 +127,9 @@ class AttachmentServiceProxyTest : public testing::Test, ...@@ -144,12 +127,9 @@ class AttachmentServiceProxyTest : public testing::Test,
scoped_ptr<AttachmentServiceProxy> proxy; scoped_ptr<AttachmentServiceProxy> proxy;
AttachmentService::GetOrDownloadCallback callback_get_or_download; AttachmentService::GetOrDownloadCallback callback_get_or_download;
AttachmentService::DropCallback callback_drop;
// number of times callback_get_or_download was invoked // number of times callback_get_or_download was invoked
int count_callback_get_or_download; int count_callback_get_or_download;
// number of times callback_drop was invoked
int count_callback_drop;
}; };
TEST_F(AttachmentServiceProxyTest, GetStore) { TEST_F(AttachmentServiceProxyTest, GetStore) {
...@@ -161,11 +141,10 @@ TEST_F(AttachmentServiceProxyTest, GetStore) { ...@@ -161,11 +141,10 @@ TEST_F(AttachmentServiceProxyTest, GetStore) {
// thread. // thread.
TEST_F(AttachmentServiceProxyTest, MethodsAreProxied) { TEST_F(AttachmentServiceProxyTest, MethodsAreProxied) {
proxy->GetOrDownloadAttachments(AttachmentIdList(), callback_get_or_download); proxy->GetOrDownloadAttachments(AttachmentIdList(), callback_get_or_download);
proxy->DropAttachments(AttachmentIdList(), callback_drop);
proxy->UploadAttachments(AttachmentIdSet()); proxy->UploadAttachments(AttachmentIdSet());
// Wait for the posted calls to execute in the stub thread. // Wait for the posted calls to execute in the stub thread.
WaitForStubThread(); WaitForStubThread();
EXPECT_EQ(3, stub->GetCallCount()); EXPECT_EQ(2, stub->GetCallCount());
// At this point the stub thread has finished executed the calls. However, the // At this point the stub thread has finished executed the calls. However, the
// result callbacks it has posted may not have executed yet. Wait a second // result callbacks it has posted may not have executed yet. Wait a second
// time to ensure the stub thread has executed the posted result callbacks. // time to ensure the stub thread has executed the posted result callbacks.
...@@ -173,7 +152,6 @@ TEST_F(AttachmentServiceProxyTest, MethodsAreProxied) { ...@@ -173,7 +152,6 @@ TEST_F(AttachmentServiceProxyTest, MethodsAreProxied) {
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_EQ(1, count_callback_get_or_download); EXPECT_EQ(1, count_callback_get_or_download);
EXPECT_EQ(1, count_callback_drop);
} }
// Verify that it's safe to use an AttachmentServiceProxy even after its wrapped // Verify that it's safe to use an AttachmentServiceProxy even after its wrapped
......
...@@ -36,15 +36,6 @@ class SYNC_EXPORT AttachmentService { ...@@ -36,15 +36,6 @@ class SYNC_EXPORT AttachmentService {
void(const GetOrDownloadResult&, scoped_ptr<AttachmentMap> attachments)> void(const GetOrDownloadResult&, scoped_ptr<AttachmentMap> attachments)>
GetOrDownloadCallback; GetOrDownloadCallback;
// The result of a DropAttachments operation.
enum DropResult {
DROP_SUCCESS, // No error, all attachments dropped.
DROP_UNSPECIFIED_ERROR, // An unspecified error occurred. Some or all
// attachments may not have been dropped.
};
typedef base::Callback<void(const DropResult&)> DropCallback;
// An interface that embedder code implements to be notified about different // An interface that embedder code implements to be notified about different
// events that originate from AttachmentService. // events that originate from AttachmentService.
// This interface will be called from the same thread AttachmentService was // This interface will be called from the same thread AttachmentService was
...@@ -71,10 +62,6 @@ class SYNC_EXPORT AttachmentService { ...@@ -71,10 +62,6 @@ class SYNC_EXPORT AttachmentService {
const AttachmentIdList& attachment_ids, const AttachmentIdList& attachment_ids,
const GetOrDownloadCallback& callback) = 0; const GetOrDownloadCallback& callback) = 0;
// See SyncData::DropAttachments.
virtual void DropAttachments(const AttachmentIdList& attachment_ids,
const DropCallback& callback) = 0;
// Schedules the attachments identified by |attachment_ids| to be uploaded to // Schedules the attachments identified by |attachment_ids| to be uploaded to
// the server. // the server.
// //
......
...@@ -62,8 +62,6 @@ class SYNC_EXPORT AttachmentServiceImpl ...@@ -62,8 +62,6 @@ class SYNC_EXPORT AttachmentServiceImpl
AttachmentStore* GetStore() override; AttachmentStore* GetStore() override;
void GetOrDownloadAttachments(const AttachmentIdList& attachment_ids, void GetOrDownloadAttachments(const AttachmentIdList& attachment_ids,
const GetOrDownloadCallback& callback) override; const GetOrDownloadCallback& callback) override;
void DropAttachments(const AttachmentIdList& attachment_ids,
const DropCallback& callback) override;
void UploadAttachments(const AttachmentIdSet& attachment_ids) override; void UploadAttachments(const AttachmentIdSet& attachment_ids) override;
// NetworkChangeObserver implementation. // NetworkChangeObserver implementation.
...@@ -85,8 +83,6 @@ class SYNC_EXPORT AttachmentServiceImpl ...@@ -85,8 +83,6 @@ class SYNC_EXPORT AttachmentServiceImpl
void WriteDone(const scoped_refptr<GetOrDownloadState>& state, void WriteDone(const scoped_refptr<GetOrDownloadState>& state,
const Attachment& attachment, const Attachment& attachment,
const AttachmentStore::Result& result); const AttachmentStore::Result& result);
void DropDone(const DropCallback& callback,
const AttachmentStore::Result& result);
void UploadDone(const AttachmentUploader::UploadResult& result, void UploadDone(const AttachmentUploader::UploadResult& result,
const AttachmentId& attachment_id); const AttachmentId& attachment_id);
void DownloadDone(const scoped_refptr<GetOrDownloadState>& state, void DownloadDone(const scoped_refptr<GetOrDownloadState>& state,
......
...@@ -56,8 +56,6 @@ class SYNC_EXPORT AttachmentServiceProxy : public AttachmentService { ...@@ -56,8 +56,6 @@ class SYNC_EXPORT AttachmentServiceProxy : public AttachmentService {
AttachmentStore* GetStore() override; AttachmentStore* GetStore() override;
void GetOrDownloadAttachments(const AttachmentIdList& attachment_ids, void GetOrDownloadAttachments(const AttachmentIdList& attachment_ids,
const GetOrDownloadCallback& callback) override; const GetOrDownloadCallback& callback) override;
void DropAttachments(const AttachmentIdList& attachment_ids,
const DropCallback& callback) override;
void UploadAttachments(const AttachmentIdSet& attachment_ids) override; void UploadAttachments(const AttachmentIdSet& attachment_ids) override;
protected: protected:
...@@ -85,8 +83,6 @@ class SYNC_EXPORT AttachmentServiceProxy : public AttachmentService { ...@@ -85,8 +83,6 @@ class SYNC_EXPORT AttachmentServiceProxy : public AttachmentService {
void GetOrDownloadAttachments( void GetOrDownloadAttachments(
const AttachmentIdList& attachment_ids, const AttachmentIdList& attachment_ids,
const GetOrDownloadCallback& callback) override; const GetOrDownloadCallback& callback) override;
void DropAttachments(const AttachmentIdList& attachment_ids,
const DropCallback& callback) override;
void UploadAttachments(const AttachmentIdSet& attachment_ids) override; void UploadAttachments(const AttachmentIdSet& attachment_ids) override;
protected: protected:
......
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