Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(159)

Unified Diff: content/browser/background_sync/background_sync_service_impl.cc

Issue 1282013004: BackgroundSyncManager tracks client registrations (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Address comments from PS12 Created 5 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: content/browser/background_sync/background_sync_service_impl.cc
diff --git a/content/browser/background_sync/background_sync_service_impl.cc b/content/browser/background_sync/background_sync_service_impl.cc
index 7c9507f685a8ee3188ec7d2262d93949c20e2ed8..cd3ffc0234d94472e24be797b8fae2510e94398c 100644
--- a/content/browser/background_sync/background_sync_service_impl.cc
+++ b/content/browser/background_sync/background_sync_service_impl.cc
@@ -5,8 +5,9 @@
#include "content/browser/background_sync/background_sync_service_impl.h"
#include "base/memory/weak_ptr.h"
+#include "base/stl_util.h"
#include "content/browser/background_sync/background_sync_context_impl.h"
-#include "content/browser/background_sync/background_sync_registration.h"
+#include "content/browser/background_sync/client_background_sync_registration.h"
#include "content/public/browser/browser_thread.h"
namespace content {
@@ -88,6 +89,9 @@ COMPILE_ASSERT_MATCHING_ENUM(BACKGROUND_SYNC_PERIODICITY_MAX,
BackgroundSyncServiceImpl::~BackgroundSyncServiceImpl() {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
+ BackgroundSyncManager* background_sync_manager =
+ background_sync_context_->background_sync_manager();
+ DCHECK(background_sync_manager);
}
BackgroundSyncServiceImpl::BackgroundSyncServiceImpl(
@@ -129,15 +133,22 @@ void BackgroundSyncServiceImpl::Register(content::SyncRegistrationPtr options,
void BackgroundSyncServiceImpl::Unregister(
BackgroundSyncPeriodicity periodicity,
int64_t id,
- const mojo::String& tag,
int64_t sw_registration_id,
const UnregisterCallback& callback) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
BackgroundSyncManager* background_sync_manager =
background_sync_context_->background_sync_manager();
DCHECK(background_sync_manager);
- background_sync_manager->Unregister(
- sw_registration_id, tag, content::SYNC_ONE_SHOT, id,
+
+ ClientBackgroundSyncRegistration* registration =
+ hosted_registrations_.Lookup(id);
+ if (!registration) {
+ callback.Run(BACKGROUND_SYNC_ERROR_NOT_ALLOWED);
+ return;
+ }
+
+ registration->Unregister(
+ sw_registration_id,
base::Bind(&BackgroundSyncServiceImpl::OnUnregisterResult,
weak_ptr_factory_.GetWeakPtr(), callback));
}
@@ -183,12 +194,63 @@ void BackgroundSyncServiceImpl::GetPermissionStatus(
callback.Run(BACKGROUND_SYNC_ERROR_NONE, PERMISSION_STATUS_GRANTED);
}
+void BackgroundSyncServiceImpl::TrackRegistration(
+ SyncRegistrationPtr sync_registration) {
+ // Registrations that the client acquires without the
+ // BackgroundSyncServiceImpl's knowing are registered here.
+ BackgroundSyncManager* background_sync_manager =
+ background_sync_context_->background_sync_manager();
+ DCHECK(background_sync_manager);
+
+ if (hosted_registrations_.Lookup(sync_registration->id)) {
+ // TODO(jkarlin): Abort client.
+ LOG(WARNING) << "Client attempted to track an already tracked registration";
+ return;
+ }
+
+ BackgroundSyncRegistrationOptions options =
+ ToBackgroundSyncRegistrationOptions(sync_registration);
+
+ scoped_ptr<ClientBackgroundSyncRegistration> registration(
+ new ClientBackgroundSyncRegistration(background_sync_manager));
+ *registration->options() = options;
+ registration->set_id(sync_registration->id);
+
+ if (!registration->IsValid()) {
+ // TODO(jkarlin): Abort client.
+ LOG(WARNING) << "Client attempted to track a non-existant registration";
+ return;
+ }
+
+ hosted_registrations_.AddWithID(registration.release(),
+ sync_registration->id);
+}
+
+void BackgroundSyncServiceImpl::ReleaseRegistration(int64_t sync_id) {
+ if (!hosted_registrations_.Lookup(sync_id)) {
+ // TODO(jkarlin): Abort client.
+ LOG(WARNING) << "Client attempted to release non-existing registration";
+ return;
+ }
+
+ hosted_registrations_.Remove(sync_id);
+}
+
void BackgroundSyncServiceImpl::OnRegisterResult(
const RegisterCallback& callback,
BackgroundSyncStatus status,
- const BackgroundSyncRegistration& result) {
+ scoped_ptr<ClientBackgroundSyncRegistration> result) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
- SyncRegistrationPtr mojoResult = ToMojoRegistration(result);
+ ClientBackgroundSyncRegistration* result_ptr = result.get();
+
+ if (status != BACKGROUND_SYNC_STATUS_OK) {
+ callback.Run(static_cast<content::BackgroundSyncError>(status),
+ SyncRegistrationPtr(content::SyncRegistration::New()));
+ return;
+ }
+
+ hosted_registrations_.AddWithID(result.release(), result_ptr->id());
+ SyncRegistrationPtr mojoResult = ToMojoRegistration(*result_ptr);
callback.Run(static_cast<content::BackgroundSyncError>(status),
mojoResult.Pass());
}
@@ -203,11 +265,17 @@ void BackgroundSyncServiceImpl::OnUnregisterResult(
void BackgroundSyncServiceImpl::OnGetRegistrationsResult(
const GetRegistrationsCallback& callback,
BackgroundSyncStatus status,
- const std::vector<BackgroundSyncRegistration>& result_registrations) {
+ scoped_ptr<ScopedVector<ClientBackgroundSyncRegistration>>
+ result_registrations) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
mojo::Array<content::SyncRegistrationPtr> mojo_registrations(0);
- for (const auto& registration : result_registrations)
- mojo_registrations.push_back(ToMojoRegistration(registration));
+ for (ClientBackgroundSyncRegistration* registration : *result_registrations) {
+ hosted_registrations_.AddWithID(registration, registration->id());
michaeln 2015/08/19 01:56:01 Does this leak (for the lifetime of the renderer p
jkarlin 2015/08/19 12:47:06 Yes. See https://codereview.chromium.org/129990300
+ mojo_registrations.push_back(ToMojoRegistration(*registration));
+ }
+
+ result_registrations->weak_clear();
+
callback.Run(static_cast<content::BackgroundSyncError>(status),
mojo_registrations.Pass());
}

Powered by Google App Engine
This is Rietveld 408576698