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

Issues Search
    My Issues | Starred     Open | Closed | All

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

Issue 1763123002: [BackgroundSync] Remove BackgroundSyncRegistrationHandle (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Address comments from PS7 Created 4 years, 9 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
« no previous file with comments | « content/browser/background_sync/background_sync_manager.h ('k') | content/browser/background_sync/background_sync_manager_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: content/browser/background_sync/background_sync_manager.cc
diff --git a/content/browser/background_sync/background_sync_manager.cc b/content/browser/background_sync/background_sync_manager.cc
index a973793ea7c79f5ddc3e9391465e7475eda5958f..79f2c78460fdbc02b519de94b16159b2c892d1aa 100644
--- a/content/browser/background_sync/background_sync_manager.cc
+++ b/content/browser/background_sync/background_sync_manager.cc
@@ -15,7 +15,6 @@
#include "build/build_config.h"
#include "content/browser/background_sync/background_sync_metrics.h"
#include "content/browser/background_sync/background_sync_network_observer.h"
-#include "content/browser/background_sync/background_sync_registration_handle.h"
#include "content/browser/background_sync/background_sync_registration_options.h"
#include "content/browser/service_worker/service_worker_context_wrapper.h"
#include "content/browser/service_worker/service_worker_storage.h"
@@ -32,19 +31,6 @@
namespace content {
-class BackgroundSyncManager::RefCountedRegistration
- : public base::RefCounted<RefCountedRegistration> {
- public:
- BackgroundSyncRegistration* value() { return &registration_; }
- const BackgroundSyncRegistration* value() const { return &registration_; }
-
- private:
- friend class base::RefCounted<RefCountedRegistration>;
- ~RefCountedRegistration() = default;
-
- BackgroundSyncRegistration registration_;
-};
-
namespace {
// The key used to index the background sync data in ServiceWorkerStorage.
@@ -56,7 +42,7 @@ void PostErrorResponse(
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
base::Bind(callback, status,
- base::Passed(scoped_ptr<BackgroundSyncRegistrationHandle>())));
+ base::Passed(scoped_ptr<BackgroundSyncRegistration>())));
}
// Returns nullptr if the controller cannot be accessed for any reason.
@@ -216,9 +202,8 @@ void BackgroundSyncManager::GetRegistrations(
FROM_HERE,
base::Bind(
callback, BACKGROUND_SYNC_STATUS_STORAGE_ERROR,
- base::Passed(
- scoped_ptr<ScopedVector<BackgroundSyncRegistrationHandle>>(
- new ScopedVector<BackgroundSyncRegistrationHandle>()))));
+ base::Passed(scoped_ptr<ScopedVector<BackgroundSyncRegistration>>(
+ new ScopedVector<BackgroundSyncRegistration>()))));
return;
}
@@ -228,20 +213,6 @@ void BackgroundSyncManager::GetRegistrations(
MakeStatusAndRegistrationsCompletion(callback)));
}
-// Given a HandleId |handle_id|, return a new handle for the same
-// registration.
-scoped_ptr<BackgroundSyncRegistrationHandle>
-BackgroundSyncManager::DuplicateRegistrationHandle(
- BackgroundSyncRegistrationHandle::HandleId handle_id) {
- DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
- scoped_refptr<RefCountedRegistration>* ref_registration =
- registration_handle_ids_.Lookup(handle_id);
- if (!ref_registration)
- return scoped_ptr<BackgroundSyncRegistrationHandle>();
- return CreateRegistrationHandle(ref_registration->get());
-}
-
void BackgroundSyncManager::OnRegistrationDeleted(int64_t sw_registration_id,
const GURL& pattern) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
@@ -378,10 +349,8 @@ void BackgroundSyncManager::InitDidGetDataFromBackend(
RegistrationKey registration_key(registration_proto.tag());
- scoped_refptr<RefCountedRegistration> ref_registration(
- new RefCountedRegistration());
- registrations->registration_map[registration_key] = ref_registration;
- BackgroundSyncRegistration* registration = ref_registration->value();
+ BackgroundSyncRegistration* registration =
+ &registrations->registration_map[registration_key];
BackgroundSyncRegistrationOptions* options = registration->options();
options->tag = registration_proto.tag();
@@ -482,12 +451,10 @@ void BackgroundSyncManager::RegisterImpl(
service_worker_context_,
sw_registration->pattern().GetOrigin()));
- RefCountedRegistration* existing_registration_ref =
+ BackgroundSyncRegistration* existing_registration =
LookupActiveRegistration(sw_registration_id, RegistrationKey(options));
- if (existing_registration_ref) {
- DCHECK(existing_registration_ref->value()->options()->Equals(options));
- BackgroundSyncRegistration* existing_registration =
- existing_registration_ref->value();
+ if (existing_registration) {
+ DCHECK(existing_registration->options()->Equals(options));
BackgroundSyncMetrics::RegistrationCouldFire registration_could_fire =
AreOptionConditionsMet(options)
@@ -504,31 +471,29 @@ void BackgroundSyncManager::RegisterImpl(
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
- base::Bind(
- callback, BACKGROUND_SYNC_STATUS_OK,
- base::Passed(CreateRegistrationHandle(existing_registration_ref))));
+ base::Bind(callback, BACKGROUND_SYNC_STATUS_OK,
+ base::Passed(make_scoped_ptr(new BackgroundSyncRegistration(
+ *existing_registration)))));
return;
}
- scoped_refptr<RefCountedRegistration> new_ref_registration(
- new RefCountedRegistration());
- BackgroundSyncRegistration* new_registration = new_ref_registration->value();
+ BackgroundSyncRegistration new_registration;
- *new_registration->options() = options;
+ *new_registration.options() = options;
BackgroundSyncRegistrations* registrations =
&active_registrations_[sw_registration_id];
- new_registration->set_id(registrations->next_id++);
+ new_registration.set_id(registrations->next_id++);
AddActiveRegistration(sw_registration_id,
sw_registration->pattern().GetOrigin(),
- new_ref_registration);
+ new_registration);
StoreRegistrations(
sw_registration_id,
base::Bind(&BackgroundSyncManager::RegisterDidStore,
weak_ptr_factory_.GetWeakPtr(), sw_registration_id,
- new_ref_registration, callback));
+ new_registration, callback));
}
void BackgroundSyncManager::DisableAndClearManager(
@@ -587,8 +552,7 @@ void BackgroundSyncManager::DisableAndClearManagerClearedOne(
base::Bind(barrier_closure));
}
-BackgroundSyncManager::RefCountedRegistration*
-BackgroundSyncManager::LookupActiveRegistration(
+BackgroundSyncRegistration* BackgroundSyncManager::LookupActiveRegistration(
int64_t sw_registration_id,
const RegistrationKey& registration_key) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
@@ -607,7 +571,7 @@ BackgroundSyncManager::LookupActiveRegistration(
if (key_and_registration_iter == registrations.registration_map.end())
return nullptr;
- return key_and_registration_iter->second.get();
+ return &key_and_registration_iter->second;
}
void BackgroundSyncManager::StoreRegistrations(
@@ -624,7 +588,7 @@ void BackgroundSyncManager::StoreRegistrations(
for (const auto& key_and_registration : registrations.registration_map) {
const BackgroundSyncRegistration& registration =
- *key_and_registration.second->value();
+ key_and_registration.second;
BackgroundSyncRegistrationProto* registration_proto =
registrations_proto.add_registration();
registration_proto->set_id(registration.id());
@@ -645,14 +609,11 @@ void BackgroundSyncManager::StoreRegistrations(
void BackgroundSyncManager::RegisterDidStore(
int64_t sw_registration_id,
- const scoped_refptr<RefCountedRegistration>& new_registration_ref,
+ const BackgroundSyncRegistration& new_registration,
const StatusAndRegistrationCallback& callback,
ServiceWorkerStatusCode status) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
- const BackgroundSyncRegistration* new_registration =
- new_registration_ref->value();
-
if (status == SERVICE_WORKER_ERROR_NOT_FOUND) {
// The service worker registration is gone.
BackgroundSyncMetrics::CountRegisterFailure(
@@ -667,14 +628,14 @@ void BackgroundSyncManager::RegisterDidStore(
"failure.";
BackgroundSyncMetrics::CountRegisterFailure(
BACKGROUND_SYNC_STATUS_STORAGE_ERROR);
- DisableAndClearManager(base::Bind(
- callback, BACKGROUND_SYNC_STATUS_STORAGE_ERROR,
- base::Passed(scoped_ptr<BackgroundSyncRegistrationHandle>())));
+ DisableAndClearManager(
+ base::Bind(callback, BACKGROUND_SYNC_STATUS_STORAGE_ERROR,
+ base::Passed(scoped_ptr<BackgroundSyncRegistration>())));
return;
}
BackgroundSyncMetrics::RegistrationCouldFire registration_could_fire =
- AreOptionConditionsMet(*new_registration->options())
+ AreOptionConditionsMet(*new_registration.options())
? BackgroundSyncMetrics::REGISTRATION_COULD_FIRE
: BackgroundSyncMetrics::REGISTRATION_COULD_NOT_FIRE;
BackgroundSyncMetrics::CountRegisterSuccess(
@@ -682,11 +643,12 @@ void BackgroundSyncManager::RegisterDidStore(
BackgroundSyncMetrics::REGISTRATION_IS_NOT_DUPLICATE);
FireReadyEvents();
+
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
- base::Bind(
- callback, BACKGROUND_SYNC_STATUS_OK,
- base::Passed(CreateRegistrationHandle(new_registration_ref.get()))));
+ base::Bind(callback, BACKGROUND_SYNC_STATUS_OK,
+ base::Passed(make_scoped_ptr(
+ new BackgroundSyncRegistration(new_registration)))));
}
void BackgroundSyncManager::RemoveActiveRegistration(
@@ -704,15 +666,15 @@ void BackgroundSyncManager::RemoveActiveRegistration(
void BackgroundSyncManager::AddActiveRegistration(
int64_t sw_registration_id,
const GURL& origin,
- const scoped_refptr<RefCountedRegistration>& sync_registration) {
+ const BackgroundSyncRegistration& sync_registration) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
- DCHECK(sync_registration->value()->IsValid());
+ DCHECK(sync_registration.IsValid());
BackgroundSyncRegistrations* registrations =
&active_registrations_[sw_registration_id];
registrations->origin = origin;
- RegistrationKey registration_key(*sync_registration->value());
+ RegistrationKey registration_key(sync_registration);
registrations->registration_map[registration_key] = sync_registration;
}
@@ -739,7 +701,7 @@ void BackgroundSyncManager::GetDataFromBackend(
}
void BackgroundSyncManager::DispatchSyncEvent(
- BackgroundSyncRegistrationHandle::HandleId handle_id,
+ const std::string& tag,
const scoped_refptr<ServiceWorkerVersion>& active_version,
BackgroundSyncEventLastChance last_chance,
const ServiceWorkerVersion::StatusCallback& callback) {
@@ -749,7 +711,7 @@ void BackgroundSyncManager::DispatchSyncEvent(
if (active_version->running_status() != ServiceWorkerVersion::RUNNING) {
active_version->RunAfterStartWorker(
base::Bind(&BackgroundSyncManager::DispatchSyncEvent,
- weak_ptr_factory_.GetWeakPtr(), handle_id, active_version,
+ weak_ptr_factory_.GetWeakPtr(), tag, active_version,
last_chance, callback),
callback);
return;
@@ -763,12 +725,8 @@ void BackgroundSyncManager::DispatchSyncEvent(
active_version->GetMojoServiceForRequest<BackgroundSyncServiceClient>(
request_id);
- // The ServiceWorkerVersion doesn't know when the client (javascript) is done
- // with the registration so don't give it a BackgroundSyncRegistrationHandle.
- // Once the render process gets the handle_id it can create its own handle
- // (with a new unique handle id).
client->Sync(
- handle_id, last_chance,
+ tag, last_chance,
base::Bind(&OnSyncEventFinished, active_version, request_id, callback));
}
@@ -784,46 +742,13 @@ void BackgroundSyncManager::HasMainFrameProviderHost(
service_worker_context_->HasMainFrameProviderHost(origin, callback);
}
-scoped_ptr<BackgroundSyncRegistrationHandle>
-BackgroundSyncManager::CreateRegistrationHandle(
- const scoped_refptr<RefCountedRegistration>& registration) {
- scoped_refptr<RefCountedRegistration>* ptr =
- new scoped_refptr<RefCountedRegistration>(registration);
-
- // Registration handles have unique handle ids. The handle id maps to an
- // internal RefCountedRegistration (which has the persistent registration id)
- // via
- // registration_reference_ids_.
- BackgroundSyncRegistrationHandle::HandleId handle_id =
- registration_handle_ids_.Add(ptr);
-
- return make_scoped_ptr(new BackgroundSyncRegistrationHandle(
- weak_ptr_factory_.GetWeakPtr(), handle_id));
-}
-
-BackgroundSyncRegistration* BackgroundSyncManager::GetRegistrationForHandle(
- BackgroundSyncRegistrationHandle::HandleId handle_id) const {
- scoped_refptr<RefCountedRegistration>* ref_registration =
- registration_handle_ids_.Lookup(handle_id);
- if (!ref_registration)
- return nullptr;
- return (*ref_registration)->value();
-}
-
-void BackgroundSyncManager::ReleaseRegistrationHandle(
- BackgroundSyncRegistrationHandle::HandleId handle_id) {
- DCHECK_CURRENTLY_ON(BrowserThread::IO);
- DCHECK(registration_handle_ids_.Lookup(handle_id));
- registration_handle_ids_.Remove(handle_id);
-}
-
void BackgroundSyncManager::GetRegistrationsImpl(
int64_t sw_registration_id,
const StatusAndRegistrationsCallback& callback) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
- scoped_ptr<ScopedVector<BackgroundSyncRegistrationHandle>> out_registrations(
- new ScopedVector<BackgroundSyncRegistrationHandle>());
+ scoped_ptr<ScopedVector<BackgroundSyncRegistration>> out_registrations(
+ new ScopedVector<BackgroundSyncRegistration>());
if (disabled_) {
base::ThreadTaskRunnerHandle::Get()->PostTask(
@@ -838,9 +763,11 @@ void BackgroundSyncManager::GetRegistrationsImpl(
if (it != active_registrations_.end()) {
const BackgroundSyncRegistrations& registrations = it->second;
for (const auto& tag_and_registration : registrations.registration_map) {
- RefCountedRegistration* registration = tag_and_registration.second.get();
- out_registrations->push_back(
- CreateRegistrationHandle(registration).release());
+ const BackgroundSyncRegistration& registration =
+ tag_and_registration.second;
+ BackgroundSyncRegistration* out_registration =
+ new BackgroundSyncRegistration(registration);
+ out_registrations->push_back(out_registration);
}
}
@@ -876,7 +803,7 @@ void BackgroundSyncManager::RunInBackgroundIfNecessary() {
for (const auto& key_and_registration :
sw_id_and_registrations.second.registration_map) {
const BackgroundSyncRegistration& registration =
- *key_and_registration.second->value();
+ key_and_registration.second;
if (registration.sync_state() == BackgroundSyncState::PENDING) {
if (clock_->Now() >= registration.delay_until()) {
soonest_wakeup_delta = base::TimeDelta();
@@ -942,8 +869,7 @@ void BackgroundSyncManager::FireReadyEventsImpl(const base::Closure& callback) {
const int64_t service_worker_id = sw_id_and_registrations.first;
for (auto& key_and_registration :
sw_id_and_registrations.second.registration_map) {
- BackgroundSyncRegistration* registration =
- key_and_registration.second->value();
+ BackgroundSyncRegistration* registration = &key_and_registration.second;
if (IsRegistrationReadyToFire(*registration)) {
sw_id_and_keys_to_fire.push_back(
std::make_pair(service_worker_id, key_and_registration.first));
@@ -979,7 +905,7 @@ void BackgroundSyncManager::FireReadyEventsImpl(const base::Closure& callback) {
for (const auto& sw_id_and_key : sw_id_and_keys_to_fire) {
int64_t service_worker_id = sw_id_and_key.first;
- const RefCountedRegistration* registration =
+ const BackgroundSyncRegistration* registration =
LookupActiveRegistration(service_worker_id, sw_id_and_key.second);
DCHECK(registration);
@@ -987,7 +913,7 @@ void BackgroundSyncManager::FireReadyEventsImpl(const base::Closure& callback) {
service_worker_id, active_registrations_[service_worker_id].origin,
base::Bind(&BackgroundSyncManager::FireReadyEventsDidFindRegistration,
weak_ptr_factory_.GetWeakPtr(), sw_id_and_key.second,
- registration->value()->id(), events_fired_barrier_closure,
+ registration->id(), events_fired_barrier_closure,
events_completed_barrier_closure));
}
}
@@ -1009,23 +935,14 @@ void BackgroundSyncManager::FireReadyEventsDidFindRegistration(
return;
}
- RefCountedRegistration* registration = LookupActiveRegistration(
+ BackgroundSyncRegistration* registration = LookupActiveRegistration(
service_worker_registration->id(), registration_key);
DCHECK(registration);
- // Create a handle and keep it until the sync event completes. The client can
- // acquire its own handle for longer-term use.
- scoped_ptr<BackgroundSyncRegistrationHandle> registration_handle =
- CreateRegistrationHandle(registration);
-
num_firing_registrations_ += 1;
- BackgroundSyncRegistrationHandle::HandleId handle_id =
- registration_handle->handle_id();
-
BackgroundSyncEventLastChance last_chance =
- registration->value()->num_attempts() ==
- parameters_->max_sync_attempts - 1
+ registration->num_attempts() == parameters_->max_sync_attempts - 1
? BackgroundSyncEventLastChance::IS_LAST_CHANCE
: BackgroundSyncEventLastChance::IS_NOT_LAST_CHANCE;
@@ -1034,11 +951,11 @@ void BackgroundSyncManager::FireReadyEventsDidFindRegistration(
base::Bind(&BackgroundSyncMetrics::RecordEventStarted));
DispatchSyncEvent(
- handle_id, service_worker_registration->active_version(), last_chance,
+ registration->options()->tag,
+ service_worker_registration->active_version(), last_chance,
base::Bind(&BackgroundSyncManager::EventComplete,
weak_ptr_factory_.GetWeakPtr(), service_worker_registration,
- service_worker_registration->id(),
- base::Passed(std::move(registration_handle)),
+ service_worker_registration->id(), registration_key,
event_completed_callback));
base::ThreadTaskRunnerHandle::Get()->PostTask(
@@ -1059,7 +976,7 @@ void BackgroundSyncManager::FireReadyEventsAllEventsFiring(
void BackgroundSyncManager::EventComplete(
const scoped_refptr<ServiceWorkerRegistration>& service_worker_registration,
int64_t service_worker_id,
- scoped_ptr<BackgroundSyncRegistrationHandle> registration_handle,
+ const RegistrationKey& registration_key,
const base::Closure& callback,
ServiceWorkerStatusCode status_code) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
@@ -1072,13 +989,13 @@ void BackgroundSyncManager::EventComplete(
op_scheduler_.ScheduleOperation(base::Bind(
&BackgroundSyncManager::EventCompleteImpl, weak_ptr_factory_.GetWeakPtr(),
- service_worker_id, base::Passed(std::move(registration_handle)),
- status_code, MakeClosureCompletion(callback)));
+ service_worker_id, registration_key, status_code,
+ MakeClosureCompletion(callback)));
}
void BackgroundSyncManager::EventCompleteImpl(
int64_t service_worker_id,
- scoped_ptr<BackgroundSyncRegistrationHandle> registration_handle,
+ const RegistrationKey& registration_key,
ServiceWorkerStatusCode status_code,
const base::Closure& callback) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
@@ -1089,15 +1006,20 @@ void BackgroundSyncManager::EventCompleteImpl(
return;
}
+ num_firing_registrations_ -= 1;
+
BackgroundSyncRegistration* registration =
- registration_handle->registration();
- DCHECK(registration);
+ LookupActiveRegistration(service_worker_id, registration_key);
+ if (!registration) {
+ base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
+ base::Bind(callback));
+ return;
+ }
+
DCHECK_NE(BackgroundSyncState::PENDING, registration->sync_state());
registration->set_num_attempts(registration->num_attempts() + 1);
- num_firing_registrations_ -= 1;
-
// The event ran to completion, we should count it, no matter what happens
// from here.
ServiceWorkerRegistration* sw_registration =
@@ -1130,10 +1052,10 @@ void BackgroundSyncManager::EventCompleteImpl(
if (registration_completed) {
RegistrationKey key(*registration);
- RefCountedRegistration* active_registration =
+ BackgroundSyncRegistration* active_registration =
LookupActiveRegistration(service_worker_id, key);
if (active_registration &&
- active_registration->value()->id() == registration->id()) {
+ active_registration->id() == registration->id()) {
RemoveActiveRegistration(service_worker_id, key);
}
}
@@ -1229,21 +1151,20 @@ void BackgroundSyncManager::CompleteOperationCallback(const CallbackT& callback,
void BackgroundSyncManager::CompleteStatusAndRegistrationCallback(
StatusAndRegistrationCallback callback,
BackgroundSyncStatus status,
- scoped_ptr<BackgroundSyncRegistrationHandle> registration_handle) {
+ scoped_ptr<BackgroundSyncRegistration> registration) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
- callback.Run(status, std::move(registration_handle));
+ callback.Run(status, std::move(registration));
op_scheduler_.CompleteOperationAndRunNext();
}
void BackgroundSyncManager::CompleteStatusAndRegistrationsCallback(
StatusAndRegistrationsCallback callback,
BackgroundSyncStatus status,
- scoped_ptr<ScopedVector<BackgroundSyncRegistrationHandle>>
- registration_handles) {
+ scoped_ptr<ScopedVector<BackgroundSyncRegistration>> registrations) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
- callback.Run(status, std::move(registration_handles));
+ callback.Run(status, std::move(registrations));
op_scheduler_.CompleteOperationAndRunNext();
}
« no previous file with comments | « content/browser/background_sync/background_sync_manager.h ('k') | content/browser/background_sync/background_sync_manager_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698