diff options
author | kareng <kareng@google.com> | 2014-10-20 05:28:28 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2014-10-20 12:28:49 +0000 |
commit | 10e58e62b3dff1340532cb5986315e7174186497 (patch) | |
tree | 03e9d11dabc4ebe5559c331cfe8d0b5ad990c428 | |
parent | 997d364ad30fe99c15c8246e2c638e9e55d7e8ad (diff) | |
download | chromium_src-10e58e62b3dff1340532cb5986315e7174186497.zip chromium_src-10e58e62b3dff1340532cb5986315e7174186497.tar.gz chromium_src-10e58e62b3dff1340532cb5986315e7174186497.tar.bz2 |
Revert of Added quota client for serviceworker. Enables 'clear past <time> data'. (patchset #7 id:300001 of https://codereview.chromium.org/633273002/)
Reason for revert:
causing crashes https://code.google.com/p/chromium/issues/detail?id=424831
Original issue's description:
> Added quota client for serviceworker. Enables 'clear past <time> data'.
>
> BUG=419287
>
> Committed: https://crrev.com/f6916df879bf23bacf994880359c980f4249e324
> Cr-Commit-Position: refs/heads/master@{#299918}
TBR=michaeln@chromium.org,jsbell@chromium.org,sky@chromium.org,dmurph@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=419287
Review URL: https://codereview.chromium.org/654323003
Cr-Commit-Position: refs/heads/master@{#300251}
12 files changed, 28 insertions, 342 deletions
diff --git a/content/browser/service_worker/service_worker_context_core.cc b/content/browser/service_worker/service_worker_context_core.cc index e253901..99131dd 100644 --- a/content/browser/service_worker/service_worker_context_core.cc +++ b/content/browser/service_worker/service_worker_context_core.cc @@ -4,9 +4,6 @@ #include "content/browser/service_worker/service_worker_context_core.h" -#include "base/barrier_closure.h" -#include "base/bind.h" -#include "base/bind_helpers.h" #include "base/files/file_path.h" #include "base/single_thread_task_runner.h" #include "base/strings/string_util.h" @@ -26,25 +23,6 @@ #include "url/gurl.h" namespace content { -namespace { -void SuccessCollectorCallback(const base::Closure& done_closure, - bool* overall_success, - ServiceWorkerStatusCode status) { - if (status != ServiceWorkerStatusCode::SERVICE_WORKER_OK) { - *overall_success = false; - } - done_closure.Run(); -} - -void SuccessReportingCallback( - const bool* success, - const ServiceWorkerContextCore::UnregistrationCallback& callback) { - DCHECK_CURRENTLY_ON(BrowserThread::IO); - bool result = *success; - callback.Run(result ? ServiceWorkerStatusCode::SERVICE_WORKER_OK - : ServiceWorkerStatusCode::SERVICE_WORKER_ERROR_FAILED); -} -} // namespace const base::FilePath::CharType ServiceWorkerContextCore::kServiceWorkerDirectory[] = @@ -236,45 +214,6 @@ void ServiceWorkerContextCore::UnregisterServiceWorker( callback)); } -void ServiceWorkerContextCore::UnregisterServiceWorkers( - const GURL& origin, - const UnregistrationCallback& callback) { - DCHECK_CURRENTLY_ON(BrowserThread::IO); - if (storage()->IsDisabled()) { - // Not posting as new task to match implementations above. - callback.Run(SERVICE_WORKER_ERROR_ABORT); - return; - } - - storage()->GetAllRegistrations(base::Bind( - &ServiceWorkerContextCore::DidGetAllRegistrationsForUnregisterForOrigin, - AsWeakPtr(), - callback, - origin)); -} - -void ServiceWorkerContextCore::DidGetAllRegistrationsForUnregisterForOrigin( - const UnregistrationCallback& result, - const GURL& origin, - const std::vector<ServiceWorkerRegistrationInfo>& registrations) { - std::set<GURL> scopes; - for (const auto& registration_info : registrations) { - if (origin == registration_info.pattern.GetOrigin()) { - scopes.insert(registration_info.pattern); - } - } - bool* overall_success = new bool(true); - base::Closure barrier = base::BarrierClosure( - scopes.size(), - base::Bind( - &SuccessReportingCallback, base::Owned(overall_success), result)); - - for (const GURL& scope : scopes) { - UnregisterServiceWorker( - scope, base::Bind(&SuccessCollectorCallback, barrier, overall_success)); - } -} - void ServiceWorkerContextCore::UpdateServiceWorker( ServiceWorkerRegistration* registration) { DCHECK_CURRENTLY_ON(BrowserThread::IO); diff --git a/content/browser/service_worker/service_worker_context_core.h b/content/browser/service_worker/service_worker_context_core.h index 87c6909..ed583c4 100644 --- a/content/browser/service_worker/service_worker_context_core.h +++ b/content/browser/service_worker/service_worker_context_core.h @@ -154,11 +154,6 @@ class CONTENT_EXPORT ServiceWorkerContextCore const RegistrationCallback& callback); void UnregisterServiceWorker(const GURL& pattern, const UnregistrationCallback& callback); - // Callback is called issued after all unregistrations occur. The Status - // is populated as SERVICE_WORKER_OK if all succeed, or SERVICE_WORKER_FAILED - // if any did not succeed. - void UnregisterServiceWorkers(const GURL& origin, - const UnregistrationCallback& callback); void UpdateServiceWorker(ServiceWorkerRegistration* registration); // This class maintains collections of live instances, this class @@ -208,11 +203,6 @@ class CONTENT_EXPORT ServiceWorkerContextCore const UnregistrationCallback& callback, ServiceWorkerStatusCode status); - void DidGetAllRegistrationsForUnregisterForOrigin( - const UnregistrationCallback& result, - const GURL& origin, - const std::vector<ServiceWorkerRegistrationInfo>& registrations); - base::WeakPtrFactory<ServiceWorkerContextCore> weak_factory_; // It's safe to store a raw pointer instead of a scoped_refptr to |wrapper_| // because the Wrapper::Shutdown call that hops threads to destroy |this| uses diff --git a/content/browser/service_worker/service_worker_context_unittest.cc b/content/browser/service_worker/service_worker_context_unittest.cc index 4341917..e437cc0 100644 --- a/content/browser/service_worker/service_worker_context_unittest.cc +++ b/content/browser/service_worker/service_worker_context_unittest.cc @@ -273,89 +273,6 @@ TEST_F(ServiceWorkerContextTest, Unregister) { base::RunLoop().RunUntilIdle(); } -// Make sure registrations are cleaned up when they are unregistered in bulk. -TEST_F(ServiceWorkerContextTest, UnregisterMultiple) { - GURL origin1_p1("http://www.example.com/test"); - GURL origin1_p2("http://www.example.com/hello"); - GURL origin2_p1("http://www.example.com:8080/again"); - GURL origin3_p1("http://www.other.com/"); - - bool called = false; - int64 registration_id1 = kInvalidServiceWorkerRegistrationId; - int64 registration_id2 = kInvalidServiceWorkerRegistrationId; - int64 registration_id3 = kInvalidServiceWorkerRegistrationId; - int64 registration_id4 = kInvalidServiceWorkerRegistrationId; - context()->RegisterServiceWorker( - origin1_p1, - GURL("http://www.example.com/service_worker.js"), - NULL, - MakeRegisteredCallback(&called, ®istration_id1)); - context()->RegisterServiceWorker( - origin1_p2, - GURL("http://www.example.com/service_worker2.js"), - NULL, - MakeRegisteredCallback(&called, ®istration_id2)); - context()->RegisterServiceWorker( - origin2_p1, - GURL("http://www.example.com:8080/service_worker3.js"), - NULL, - MakeRegisteredCallback(&called, ®istration_id3)); - context()->RegisterServiceWorker( - origin3_p1, - GURL("http://www.other.com/service_worker4.js"), - NULL, - MakeRegisteredCallback(&called, ®istration_id4)); - - ASSERT_FALSE(called); - base::RunLoop().RunUntilIdle(); - ASSERT_TRUE(called); - - EXPECT_NE(kInvalidServiceWorkerRegistrationId, registration_id1); - EXPECT_NE(kInvalidServiceWorkerRegistrationId, registration_id2); - EXPECT_NE(kInvalidServiceWorkerRegistrationId, registration_id3); - EXPECT_NE(kInvalidServiceWorkerRegistrationId, registration_id4); - - called = false; - context()->UnregisterServiceWorkers(origin1_p1.GetOrigin(), - MakeUnregisteredCallback(&called)); - - ASSERT_FALSE(called); - base::RunLoop().RunUntilIdle(); - ASSERT_TRUE(called); - - context()->storage()->FindRegistrationForId( - registration_id1, - origin1_p1.GetOrigin(), - base::Bind(&ExpectRegisteredWorkers, - SERVICE_WORKER_ERROR_NOT_FOUND, - false /* expect_waiting */, - false /* expect_active */)); - context()->storage()->FindRegistrationForId( - registration_id2, - origin1_p2.GetOrigin(), - base::Bind(&ExpectRegisteredWorkers, - SERVICE_WORKER_ERROR_NOT_FOUND, - false /* expect_waiting */, - false /* expect_active */)); - context()->storage()->FindRegistrationForId( - registration_id3, - origin2_p1.GetOrigin(), - base::Bind(&ExpectRegisteredWorkers, - SERVICE_WORKER_OK, - false /* expect_waiting */, - true /* expect_active */)); - - context()->storage()->FindRegistrationForId( - registration_id4, - origin3_p1.GetOrigin(), - base::Bind(&ExpectRegisteredWorkers, - SERVICE_WORKER_OK, - false /* expect_waiting */, - true /* expect_active */)); - - base::RunLoop().RunUntilIdle(); -} - // Make sure registering a new script shares an existing registration. TEST_F(ServiceWorkerContextTest, RegisterNewScript) { GURL pattern("http://www.example.com/"); diff --git a/content/browser/service_worker/service_worker_context_wrapper.cc b/content/browser/service_worker/service_worker_context_wrapper.cc index b5744d3..ec33d56 100644 --- a/content/browser/service_worker/service_worker_context_wrapper.cc +++ b/content/browser/service_worker/service_worker_context_wrapper.cc @@ -6,7 +6,6 @@ #include <map> -#include "base/barrier_closure.h" #include "base/files/file_path.h" #include "base/logging.h" #include "base/threading/sequenced_worker_pool.h" @@ -14,7 +13,6 @@ #include "content/browser/service_worker/service_worker_context_core.h" #include "content/browser/service_worker/service_worker_context_observer.h" #include "content/browser/service_worker/service_worker_process_manager.h" -#include "content/browser/service_worker/service_worker_quota_client.h" #include "content/public/browser/browser_context.h" #include "content/public/browser/browser_thread.h" #include "net/url_request/url_request_context_getter.h" @@ -185,26 +183,35 @@ void ServiceWorkerContextWrapper::DidGetAllRegistrationsForGetAllOrigins( } namespace { -void StatusCodeToBoolCallbackAdapter( - const ServiceWorkerContext::ResultCallback& callback, - ServiceWorkerStatusCode code) { - callback.Run(code == ServiceWorkerStatusCode::SERVICE_WORKER_OK); -} void EmptySuccessCallback(bool success) { } + } // namespace -void ServiceWorkerContextWrapper::DeleteForOrigin( - const GURL& origin_url, - const ResultCallback& result) { +void ServiceWorkerContextWrapper::DeleteForOrigin(const GURL& origin_url) { DCHECK_CURRENTLY_ON(BrowserThread::IO); - context_core_->UnregisterServiceWorkers( - origin_url, base::Bind(&StatusCodeToBoolCallbackAdapter, result)); + context_core_->storage()->GetAllRegistrations(base::Bind( + &ServiceWorkerContextWrapper::DidGetAllRegistrationsForDeleteForOrigin, + this, + origin_url)); } -void ServiceWorkerContextWrapper::DeleteForOrigin(const GURL& origin_url) { - DeleteForOrigin(origin_url, base::Bind(&EmptySuccessCallback)); +void ServiceWorkerContextWrapper::DidGetAllRegistrationsForDeleteForOrigin( + const GURL& origin, + const std::vector<ServiceWorkerRegistrationInfo>& registrations) { + DCHECK_CURRENTLY_ON(BrowserThread::IO); + + for (std::vector<ServiceWorkerRegistrationInfo>::const_iterator it = + registrations.begin(); + it != registrations.end(); + ++it) { + const ServiceWorkerRegistrationInfo& registration_info = *it; + if (origin == registration_info.pattern.GetOrigin()) { + UnregisterServiceWorker(registration_info.pattern, + base::Bind(&EmptySuccessCallback)); + } + } } void ServiceWorkerContextWrapper::AddObserver( @@ -251,9 +258,6 @@ void ServiceWorkerContextWrapper::InitInternal( return; } DCHECK(!context_core_); - if (quota_manager_proxy) { - quota_manager_proxy->RegisterClient(new ServiceWorkerQuotaClient(this)); - } context_core_.reset(new ServiceWorkerContextCore(user_data_directory, stores_task_runner, database_task_manager.Pass(), diff --git a/content/browser/service_worker/service_worker_context_wrapper.h b/content/browser/service_worker/service_worker_context_wrapper.h index de0646a..6238deb 100644 --- a/content/browser/service_worker/service_worker_context_wrapper.h +++ b/content/browser/service_worker/service_worker_context_wrapper.h @@ -71,18 +71,13 @@ class CONTENT_EXPORT ServiceWorkerContextWrapper const GURL& pattern, const GURL& script_url, const ResultCallback& continuation) override; - virtual void UnregisterServiceWorker( - const GURL& pattern, - const ResultCallback& continuation) override; + virtual void UnregisterServiceWorker(const GURL& pattern, + const ResultCallback& continuation) + override; virtual void Terminate() override; virtual void GetAllOriginsInfo(const GetUsageInfoCallback& callback) override; virtual void DeleteForOrigin(const GURL& origin_url) override; - // DeleteForOrigin with completion callback. Does not exit early, and returns - // false if one or more of the deletions fail. - virtual void DeleteForOrigin(const GURL& origin_url, - const ResultCallback& done); - void AddObserver(ServiceWorkerContextObserver* observer); void RemoveObserver(ServiceWorkerContextObserver* observer); @@ -102,7 +97,6 @@ class CONTENT_EXPORT ServiceWorkerContextWrapper friend class base::RefCountedThreadSafe<ServiceWorkerContextWrapper>; friend class EmbeddedWorkerTestHelper; friend class ServiceWorkerProcessManager; - virtual ~ServiceWorkerContextWrapper(); void InitInternal( @@ -119,6 +113,9 @@ class CONTENT_EXPORT ServiceWorkerContextWrapper void DidGetAllRegistrationsForGetAllOrigins( const GetUsageInfoCallback& callback, const std::vector<ServiceWorkerRegistrationInfo>& registrations); + void DidGetAllRegistrationsForDeleteForOrigin( + const GURL& origin, + const std::vector<ServiceWorkerRegistrationInfo>& registrations); const scoped_refptr<ObserverListThreadSafe<ServiceWorkerContextObserver> > observer_list_; diff --git a/content/browser/service_worker/service_worker_database.cc b/content/browser/service_worker/service_worker_database.cc index 6733fe6..856e30e 100644 --- a/content/browser/service_worker/service_worker_database.cc +++ b/content/browser/service_worker/service_worker_database.cc @@ -215,9 +215,6 @@ ServiceWorkerDatabase::Status ParseRegistrationData( if (!scope_url.is_valid() || !script_url.is_valid() || scope_url.GetOrigin() != script_url.GetOrigin()) { - DLOG(ERROR) << "Scope URL '" << data.scope_url() << "' and/or script url '" - << data.script_url() - << "' are invalid or have mismatching origins."; return ServiceWorkerDatabase::STATUS_ERROR_CORRUPTED; } diff --git a/content/browser/service_worker/service_worker_quota_client.cc b/content/browser/service_worker/service_worker_quota_client.cc deleted file mode 100644 index e2697e2..0000000 --- a/content/browser/service_worker/service_worker_quota_client.cc +++ /dev/null @@ -1,95 +0,0 @@ -// Copyright 2014 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. -#include "content/browser/service_worker/service_worker_quota_client.h" - -#include "base/bind.h" -#include "content/browser/service_worker/service_worker_context_wrapper.h" -#include "content/public/browser/browser_thread.h" - -using storage::QuotaClient; - -namespace content { -namespace { -void ReportOrigins(const QuotaClient::GetOriginsCallback& callback, - bool restrict_on_host, - const std::string host, - const std::vector<ServiceWorkerUsageInfo>& usage_info) { - std::set<GURL> origins; - for (const ServiceWorkerUsageInfo& info : usage_info) { - if (restrict_on_host && info.origin.host() != host) { - continue; - } - origins.insert(info.origin); - } - callback.Run(origins); -} - -void ReportToQuotaStatus(const QuotaClient::DeletionCallback& callback, - bool status) { - callback.Run(status ? storage::QuotaStatusCode::kQuotaStatusOk - : storage::QuotaStatusCode::kQuotaStatusUnknown); -} -} // namespace - -ServiceWorkerQuotaClient::ServiceWorkerQuotaClient( - ServiceWorkerContextWrapper* context) - : context_(context) { -} - -ServiceWorkerQuotaClient::~ServiceWorkerQuotaClient() { -} - -QuotaClient::ID ServiceWorkerQuotaClient::id() const { - return QuotaClient::kServiceWorker; -} - -void ServiceWorkerQuotaClient::OnQuotaManagerDestroyed() { - delete this; -} - -void ServiceWorkerQuotaClient::GetOriginUsage( - const GURL& origin, - storage::StorageType type, - const GetUsageCallback& callback) { - // TODO(dmurph): Add usage fetching when information is available. - callback.Run(0); -} - -void ServiceWorkerQuotaClient::GetOriginsForType( - storage::StorageType type, - const GetOriginsCallback& callback) { - if (type != storage::StorageType::kStorageTypeTemporary) { - callback.Run(std::set<GURL>()); - return; - } - context_->GetAllOriginsInfo(base::Bind(&ReportOrigins, callback, false, "")); -} - -void ServiceWorkerQuotaClient::GetOriginsForHost( - storage::StorageType type, - const std::string& host, - const GetOriginsCallback& callback) { - if (type != storage::StorageType::kStorageTypeTemporary) { - callback.Run(std::set<GURL>()); - return; - } - context_->GetAllOriginsInfo(base::Bind(&ReportOrigins, callback, true, host)); -} - -void ServiceWorkerQuotaClient::DeleteOriginData( - const GURL& origin, - storage::StorageType type, - const DeletionCallback& callback) { - if (type != storage::StorageType::kStorageTypeTemporary) { - callback.Run(storage::QuotaStatusCode::kQuotaStatusOk); - return; - } - context_->DeleteForOrigin(origin, base::Bind(&ReportToQuotaStatus, callback)); -} - -bool ServiceWorkerQuotaClient::DoesSupport(storage::StorageType type) const { - return type == storage::StorageType::kStorageTypeTemporary; -} - -} // namespace content diff --git a/content/browser/service_worker/service_worker_quota_client.h b/content/browser/service_worker/service_worker_quota_client.h deleted file mode 100644 index 7b69b45..0000000 --- a/content/browser/service_worker/service_worker_quota_client.h +++ /dev/null @@ -1,49 +0,0 @@ -// Copyright 2014 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_QUOTA_CLIENT_H_ -#define CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_QUOTA_CLIENT_H_ - -#include "base/memory/ref_counted.h" -#include "content/common/content_export.h" -#include "storage/browser/quota/quota_client.h" - -namespace content { -class ServiceWorkerContextWrapper; - -class ServiceWorkerQuotaClient : public storage::QuotaClient { - public: - virtual ~ServiceWorkerQuotaClient(); - - // QuotaClient method overrides - virtual ID id() const override; - virtual void OnQuotaManagerDestroyed() override; - virtual void GetOriginUsage(const GURL& origin, - storage::StorageType type, - const GetUsageCallback& callback) override; - virtual void GetOriginsForType(storage::StorageType type, - const GetOriginsCallback& callback) override; - virtual void GetOriginsForHost(storage::StorageType type, - const std::string& host, - const GetOriginsCallback& callback) override; - virtual void DeleteOriginData(const GURL& origin, - storage::StorageType type, - const DeletionCallback& callback) override; - virtual bool DoesSupport(storage::StorageType type) const override; - - private: - friend class ServiceWorkerContextWrapper; - friend class ServiceWorkerQuotaClientTest; - - CONTENT_EXPORT explicit ServiceWorkerQuotaClient( - ServiceWorkerContextWrapper* context); - - scoped_refptr<ServiceWorkerContextWrapper> context_; - - DISALLOW_COPY_AND_ASSIGN(ServiceWorkerQuotaClient); -}; - -} // namespace content - -#endif // CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_QUOTA_CLIENT_H_ diff --git a/content/browser/service_worker/service_worker_storage.cc b/content/browser/service_worker/service_worker_storage.cc index 82fd457..da891db 100644 --- a/content/browser/service_worker/service_worker_storage.cc +++ b/content/browser/service_worker/service_worker_storage.cc @@ -479,16 +479,6 @@ void ServiceWorkerStorage::StoreRegistration( callback))); registration->set_is_deleted(false); - - // TODO(dmurph): Add correct byte delta. - if (quota_manager_proxy_.get()) { - // Can be nullptr in tests. - quota_manager_proxy_->NotifyStorageModified( - storage::QuotaClient::kServiceWorker, - registration->pattern().GetOrigin(), - storage::StorageType::kStorageTypeTemporary, - 0); - } } void ServiceWorkerStorage::UpdateToActiveState( diff --git a/content/browser/storage_partition_impl.cc b/content/browser/storage_partition_impl.cc index 1e26255..531e638 100644 --- a/content/browser/storage_partition_impl.cc +++ b/content/browser/storage_partition_impl.cc @@ -233,8 +233,7 @@ int StoragePartitionImpl::GenerateQuotaClientMask(uint32 remove_mask) { quota_client_mask |= storage::QuotaClient::kAppcache; if (remove_mask & StoragePartition::REMOVE_DATA_MASK_INDEXEDDB) quota_client_mask |= storage::QuotaClient::kIndexedDatabase; - if (remove_mask & StoragePartition::REMOVE_DATA_MASK_SERVICE_WORKERS) - quota_client_mask |= storage::QuotaClient::kServiceWorker; + // TODO(jsbell): StoragePartition::REMOVE_DATA_MASK_SERVICE_WORKERS) return quota_client_mask; } diff --git a/content/content_browser.gypi b/content/content_browser.gypi index 02a1cf0..d5b2d65 100644 --- a/content/content_browser.gypi +++ b/content/content_browser.gypi @@ -1201,8 +1201,6 @@ 'browser/service_worker/service_worker_process_manager.h', 'browser/service_worker/service_worker_provider_host.cc', 'browser/service_worker/service_worker_provider_host.h', - 'browser/service_worker/service_worker_quota_client.cc', - 'browser/service_worker/service_worker_quota_client.h', 'browser/service_worker/service_worker_read_from_cache_job.cc', 'browser/service_worker/service_worker_read_from_cache_job.h', 'browser/service_worker/service_worker_register_job_base.h', diff --git a/storage/browser/quota/quota_client.h b/storage/browser/quota/quota_client.h index 5fb3c89..8a6a294 100644 --- a/storage/browser/quota/quota_client.h +++ b/storage/browser/quota/quota_client.h @@ -35,7 +35,6 @@ class STORAGE_EXPORT QuotaClient { kDatabase = 1 << 2, kAppcache = 1 << 3, kIndexedDatabase = 1 << 4, - kServiceWorker = 1 << 5, kAllClientsMask = -1, }; |