Commit 156d3a1b authored by Kenichi Ishibashi's avatar Kenichi Ishibashi Committed by Commit Bot

Stop using ServiceWorkerStorage::IsDisabled() from ServiceWorkerRegistry

ServiceWorkerStorage will be encapsulated in a mojo interface and will
be moved outside content/. This means that it can't expose any public
method which returns a value synchronously. IsDisable() returns
a boolean value synchronously so we need to hide it.

ServiceWorkerRegistry needs to know if ServiceWorkerStorage is
disabled without IsDisabled(). This CL introduces a new field that
manages the state. It is updated when a storage operation is failed.
This isn't exactly the same, but should have almost the same effect.

Bug: 1055677
Change-Id: I1aab48eb4bf624c3d4c077d8e13472b188f8607d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2079715
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: default avatarHiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#745374}
parent 211f9265
...@@ -138,7 +138,7 @@ void ServiceWorkerRegistry::FindRegistrationForClientUrl( ...@@ -138,7 +138,7 @@ void ServiceWorkerRegistry::FindRegistrationForClientUrl(
void ServiceWorkerRegistry::FindRegistrationForScope( void ServiceWorkerRegistry::FindRegistrationForScope(
const GURL& scope, const GURL& scope,
FindRegistrationCallback callback) { FindRegistrationCallback callback) {
if (storage()->IsDisabled()) { if (is_storage_disabled_) {
RunSoon( RunSoon(
FROM_HERE, FROM_HERE,
base::BindOnce(std::move(callback), base::BindOnce(std::move(callback),
...@@ -165,7 +165,7 @@ void ServiceWorkerRegistry::FindRegistrationForId( ...@@ -165,7 +165,7 @@ void ServiceWorkerRegistry::FindRegistrationForId(
const GURL& origin, const GURL& origin,
FindRegistrationCallback callback) { FindRegistrationCallback callback) {
// Registration lookup is expected to abort when storage is disabled. // Registration lookup is expected to abort when storage is disabled.
if (storage()->IsDisabled()) { if (is_storage_disabled_) {
CompleteFindNow(nullptr, blink::ServiceWorkerStatusCode::kErrorAbort, CompleteFindNow(nullptr, blink::ServiceWorkerStatusCode::kErrorAbort,
std::move(callback)); std::move(callback));
return; return;
...@@ -194,7 +194,7 @@ void ServiceWorkerRegistry::FindRegistrationForIdOnly( ...@@ -194,7 +194,7 @@ void ServiceWorkerRegistry::FindRegistrationForIdOnly(
int64_t registration_id, int64_t registration_id,
FindRegistrationCallback callback) { FindRegistrationCallback callback) {
// Registration lookup is expected to abort when storage is disabled. // Registration lookup is expected to abort when storage is disabled.
if (storage()->IsDisabled()) { if (is_storage_disabled_) {
CompleteFindNow(nullptr, blink::ServiceWorkerStatusCode::kErrorAbort, CompleteFindNow(nullptr, blink::ServiceWorkerStatusCode::kErrorAbort,
std::move(callback)); std::move(callback));
return; return;
...@@ -266,7 +266,7 @@ void ServiceWorkerRegistry::StoreRegistration( ...@@ -266,7 +266,7 @@ void ServiceWorkerRegistry::StoreRegistration(
DCHECK(registration); DCHECK(registration);
DCHECK(version); DCHECK(version);
if (storage()->IsDisabled()) { if (is_storage_disabled_) {
RunSoon(FROM_HERE, RunSoon(FROM_HERE,
base::BindOnce(std::move(callback), base::BindOnce(std::move(callback),
blink::ServiceWorkerStatusCode::kErrorAbort)); blink::ServiceWorkerStatusCode::kErrorAbort));
...@@ -323,7 +323,7 @@ void ServiceWorkerRegistry::DeleteRegistration( ...@@ -323,7 +323,7 @@ void ServiceWorkerRegistry::DeleteRegistration(
scoped_refptr<ServiceWorkerRegistration> registration, scoped_refptr<ServiceWorkerRegistration> registration,
const GURL& origin, const GURL& origin,
StatusCallback callback) { StatusCallback callback) {
if (storage()->IsDisabled()) { if (is_storage_disabled_) {
RunSoon(FROM_HERE, RunSoon(FROM_HERE,
base::BindOnce(std::move(callback), base::BindOnce(std::move(callback),
blink::ServiceWorkerStatusCode::kErrorAbort)); blink::ServiceWorkerStatusCode::kErrorAbort));
...@@ -626,6 +626,7 @@ void ServiceWorkerRegistry::GetUserDataForAllRegistrationsByKeyPrefix( ...@@ -626,6 +626,7 @@ void ServiceWorkerRegistry::GetUserDataForAllRegistrationsByKeyPrefix(
void ServiceWorkerRegistry::PrepareForDeleteAndStarOver() { void ServiceWorkerRegistry::PrepareForDeleteAndStarOver() {
should_schedule_delete_and_start_over_ = false; should_schedule_delete_and_start_over_ = false;
storage()->Disable(); storage()->Disable();
is_storage_disabled_ = true;
} }
void ServiceWorkerRegistry::DeleteAndStartOver(StatusCallback callback) { void ServiceWorkerRegistry::DeleteAndStartOver(StatusCallback callback) {
...@@ -636,6 +637,7 @@ void ServiceWorkerRegistry::DeleteAndStartOver(StatusCallback callback) { ...@@ -636,6 +637,7 @@ void ServiceWorkerRegistry::DeleteAndStartOver(StatusCallback callback) {
void ServiceWorkerRegistry::DisableDeleteAndStartOverForTesting() { void ServiceWorkerRegistry::DisableDeleteAndStartOverForTesting() {
DCHECK(should_schedule_delete_and_start_over_); DCHECK(should_schedule_delete_and_start_over_);
should_schedule_delete_and_start_over_ = false; should_schedule_delete_and_start_over_ = false;
is_storage_disabled_ = true;
} }
ServiceWorkerRegistration* ServiceWorkerRegistration*
...@@ -1181,6 +1183,7 @@ void ServiceWorkerRegistry::ScheduleDeleteAndStartOver() { ...@@ -1181,6 +1183,7 @@ void ServiceWorkerRegistry::ScheduleDeleteAndStartOver() {
context_->ScheduleDeleteAndStartOver(); context_->ScheduleDeleteAndStartOver();
// ServiceWorkerContextCore should call PrepareForDeleteAndStartOver(). // ServiceWorkerContextCore should call PrepareForDeleteAndStartOver().
DCHECK(!should_schedule_delete_and_start_over_); DCHECK(!should_schedule_delete_and_start_over_);
DCHECK(is_storage_disabled_);
} }
} // namespace content } // namespace content
...@@ -332,6 +332,7 @@ class CONTENT_EXPORT ServiceWorkerRegistry { ...@@ -332,6 +332,7 @@ class CONTENT_EXPORT ServiceWorkerRegistry {
ServiceWorkerContextCore* const context_; ServiceWorkerContextCore* const context_;
std::unique_ptr<ServiceWorkerStorage> storage_; std::unique_ptr<ServiceWorkerStorage> storage_;
bool is_storage_disabled_ = false;
// For finding registrations being installed or uninstalled. // For finding registrations being installed or uninstalled.
using RegistrationRefsById = using RegistrationRefsById =
......
...@@ -154,7 +154,6 @@ void ServiceWorkerStorage::FindRegistrationForScope( ...@@ -154,7 +154,6 @@ void ServiceWorkerStorage::FindRegistrationForScope(
case STORAGE_STATE_DISABLED: case STORAGE_STATE_DISABLED:
RunSoon(FROM_HERE, RunSoon(FROM_HERE,
base::BindOnce(std::move(callback), base::BindOnce(std::move(callback),
/*data=*/nullptr, /*resources=*/nullptr, /*data=*/nullptr, /*resources=*/nullptr,
ServiceWorkerDatabase::Status::kErrorDisabled)); ServiceWorkerDatabase::Status::kErrorDisabled));
return; return;
...@@ -189,9 +188,9 @@ void ServiceWorkerStorage::FindRegistrationForId( ...@@ -189,9 +188,9 @@ void ServiceWorkerStorage::FindRegistrationForId(
FindRegistrationDataCallback callback) { FindRegistrationDataCallback callback) {
switch (state_) { switch (state_) {
case STORAGE_STATE_DISABLED: case STORAGE_STATE_DISABLED:
NOTREACHED() std::move(callback).Run(
<< "FindRegistrationForId() should not be called when storage " /*data=*/nullptr, /*resources=*/nullptr,
"is disabled"; ServiceWorkerDatabase::Status::kErrorDisabled);
return; return;
case STORAGE_STATE_INITIALIZING: // Fall-through. case STORAGE_STATE_INITIALIZING: // Fall-through.
case STORAGE_STATE_UNINITIALIZED: case STORAGE_STATE_UNINITIALIZED:
...@@ -223,9 +222,9 @@ void ServiceWorkerStorage::FindRegistrationForIdOnly( ...@@ -223,9 +222,9 @@ void ServiceWorkerStorage::FindRegistrationForIdOnly(
FindRegistrationDataCallback callback) { FindRegistrationDataCallback callback) {
switch (state_) { switch (state_) {
case STORAGE_STATE_DISABLED: case STORAGE_STATE_DISABLED:
NOTREACHED() std::move(callback).Run(
<< "FindRegistrationForIdOnly() should not be called when storage " /*data=*/nullptr, /*resources=*/nullptr,
"is disabled"; ServiceWorkerDatabase::Status::kErrorDisabled);
return; return;
case STORAGE_STATE_INITIALIZING: // Fall-through. case STORAGE_STATE_INITIALIZING: // Fall-through.
case STORAGE_STATE_UNINITIALIZED: case STORAGE_STATE_UNINITIALIZED:
...@@ -854,10 +853,6 @@ void ServiceWorkerStorage::Disable() { ...@@ -854,10 +853,6 @@ void ServiceWorkerStorage::Disable() {
disk_cache_->Disable(); disk_cache_->Disable();
} }
bool ServiceWorkerStorage::IsDisabled() const {
return state_ == STORAGE_STATE_DISABLED;
}
void ServiceWorkerStorage::PurgeResources(const ResourceList& resources) { void ServiceWorkerStorage::PurgeResources(const ResourceList& resources) {
if (!has_checked_for_stale_resources_) if (!has_checked_for_stale_resources_)
DeleteStaleResources(); DeleteStaleResources();
......
...@@ -277,7 +277,6 @@ class CONTENT_EXPORT ServiceWorkerStorage { ...@@ -277,7 +277,6 @@ class CONTENT_EXPORT ServiceWorkerStorage {
void GetNewResourceId(base::OnceCallback<void(int64_t resource_id)> callback); void GetNewResourceId(base::OnceCallback<void(int64_t resource_id)> callback);
void Disable(); void Disable();
bool IsDisabled() const;
// Schedules deleting |resources| from the disk cache and removing their keys // Schedules deleting |resources| from the disk cache and removing their keys
// as purgeable resources from the service worker database. It's OK to call // as purgeable resources from the service worker database. It's OK to call
...@@ -431,6 +430,8 @@ class CONTENT_EXPORT ServiceWorkerStorage { ...@@ -431,6 +430,8 @@ class CONTENT_EXPORT ServiceWorkerStorage {
int64_t NewVersionId(); int64_t NewVersionId();
int64_t NewResourceId(); int64_t NewResourceId();
bool IsDisabled() const { return state_ == STORAGE_STATE_DISABLED; }
// Static cross-thread helpers. // Static cross-thread helpers.
static void CollectStaleResourcesFromDB( static void CollectStaleResourcesFromDB(
ServiceWorkerDatabase* database, ServiceWorkerDatabase* database,
......
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