content/browser/background_sync/background_sync_manager.cc - Issue 1763123002: [BackgroundSync] Remove BackgroundSyncRegistrationHandle

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: Remove default rvalue functions due to msvc 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') | content/browser/background_sync/background_sync_registration.h » ('J')
Expand Comments ('e') | Collapse Comments ('c') | 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..291faf130711a0ad3011f3fd6a2f09bc5887e00c 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)

@@ -502,33 +469,32 @@ void BackgroundSyncManager::RegisterImpl(

BackgroundSyncState::REREGISTERED_WHILE_FIRING);

}

+ scoped_ptr<BackgroundSyncRegistration> out_registration(

+ new BackgroundSyncRegistration(*existing_registration));

base::ThreadTaskRunnerHandle::Get()->PostTask(

- FROM_HERE,

- base::Bind(

- callback, BACKGROUND_SYNC_STATUS_OK,

- base::Passed(CreateRegistrationHandle(existing_registration_ref))));

+ FROM_HERE, base::Bind(callback, BACKGROUND_SYNC_STATUS_OK,

+ base::Passed(std::move(out_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 +553,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 +572,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 +589,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 +610,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 +629,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 +644,13 @@ void BackgroundSyncManager::RegisterDidStore(

BackgroundSyncMetrics::REGISTRATION_IS_NOT_DUPLICATE);

FireReadyEvents();

+ scoped_ptr<BackgroundSyncRegistration> out_registration(

+ new BackgroundSyncRegistration(new_registration));

base::ThreadTaskRunnerHandle::Get()->PostTask(

- FROM_HERE,

- base::Bind(

- callback, BACKGROUND_SYNC_STATUS_OK,

- base::Passed(CreateRegistrationHandle(new_registration_ref.get()))));

+ FROM_HERE, base::Bind(callback, BACKGROUND_SYNC_STATUS_OK,

+ base::Passed(std::move(out_registration))));

}

void BackgroundSyncManager::RemoveActiveRegistration(

@@ -704,15 +668,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 +703,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 +713,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 +727,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 +744,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 +765,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);

iclelland 2016/03/08 16:48:14 These are allocated here, but never released in th

jkarlin 2016/03/08 19:37:36 That weak_clear had to go, thanks!

+ out_registrations->push_back(out_registration);

}

@@ -876,7 +805,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 +871,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 +907,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 +915,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 +937,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 +953,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 +978,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 +991,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);

@@ -1090,7 +1009,13 @@ void BackgroundSyncManager::EventCompleteImpl(

}

BackgroundSyncRegistration* registration =

- registration_handle->registration();

+ LookupActiveRegistration(service_worker_id, registration_key);

+ if (!registration) {

iclelland 2016/03/08 16:48:14 Is this a failure case?

jkarlin 2016/03/08 19:37:36 This is what happens if the ServiceWorkerRegistrat

+ base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,

+ base::Bind(callback));

+ return;

+ }

DCHECK(registration);

iclelland 2016/03/08 16:48:13 No longer necessary

jkarlin 2016/03/08 19:37:36 Done.

DCHECK_NE(BackgroundSyncState::PENDING, registration->sync_state());

@@ -1130,10 +1055,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 +1154,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>> registration) {

DCHECK_CURRENTLY_ON(BrowserThread::IO);

- callback.Run(status, std::move(registration_handles));

+ callback.Run(status, std::move(registration));

op_scheduler_.CompleteOperationAndRunNext();

}