diff options
| author | iclelland <iclelland@chromium.org> | 2015-07-23 12:23:26 -0700 |
|---|---|---|
| committer | Commit bot <commit-bot@chromium.org> | 2015-07-23 19:24:54 +0000 |
| commit | 213183fe6e8958d2ef43543c806dc3d34ce3509e (patch) | |
| tree | 5b3a72d280fc2562770aaee7cc3f653536113d35 /content | |
| parent | c28691dacdd323cebbdddc5e92526cb2623429b8 (diff) | |
| download | chromium_src-213183fe6e8958d2ef43543c806dc3d34ce3509e.zip chromium_src-213183fe6e8958d2ef43543c806dc3d34ce3509e.tar.gz chromium_src-213183fe6e8958d2ef43543c806dc3d34ce3509e.tar.bz2 | |
[Background Sync] Restrict sync registration to service workers with window clients.
BUG=479658
Review URL: https://codereview.chromium.org/1234353004
Cr-Commit-Position: refs/heads/master@{#340141}
Diffstat (limited to 'content')
9 files changed, 142 insertions, 31 deletions
diff --git a/content/browser/background_sync/background_sync_manager.cc b/content/browser/background_sync/background_sync_manager.cc index 2a79df5..6c33d51 100644 --- a/content/browser/background_sync/background_sync_manager.cc +++ b/content/browser/background_sync/background_sync_manager.cc @@ -320,6 +320,26 @@ void BackgroundSyncManager::RegisterImpl( return; } + ServiceWorkerRegistration* sw_registration = + service_worker_context_->GetLiveRegistration(sw_registration_id); + if (!sw_registration || !sw_registration->active_version()) { + BackgroundSyncMetrics::CountRegister( + options.periodicity, registration_could_fire, + BackgroundSyncMetrics::REGISTRATION_IS_NOT_DUPLICATE, + ERROR_TYPE_NO_SERVICE_WORKER); + base::ThreadTaskRunnerHandle::Get()->PostTask( + FROM_HERE, base::Bind(callback, ERROR_TYPE_NO_SERVICE_WORKER, + BackgroundSyncRegistration())); + return; + } + + if (!sw_registration->active_version()->HasWindowClients()) { + base::ThreadTaskRunnerHandle::Get()->PostTask( + FROM_HERE, base::Bind(callback, ERROR_TYPE_NOT_ALLOWED, + BackgroundSyncRegistration())); + return; + } + BackgroundSyncRegistration* existing_registration = LookupRegistration(sw_registration_id, RegistrationKey(options)); if (existing_registration && @@ -351,19 +371,6 @@ void BackgroundSyncManager::RegisterImpl( &sw_to_registrations_map_[sw_registration_id]; new_registration.set_id(registrations->next_id++); - ServiceWorkerRegistration* sw_registration = - service_worker_context_->GetLiveRegistration(sw_registration_id); - if (!sw_registration || !sw_registration->active_version()) { - BackgroundSyncMetrics::CountRegister( - options.periodicity, registration_could_fire, - BackgroundSyncMetrics::REGISTRATION_IS_NOT_DUPLICATE, - ERROR_TYPE_NO_SERVICE_WORKER); - base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::Bind(callback, ERROR_TYPE_NO_SERVICE_WORKER, - BackgroundSyncRegistration())); - return; - } - AddRegistrationToMap(sw_registration_id, sw_registration->pattern().GetOrigin(), new_registration); diff --git a/content/browser/background_sync/background_sync_manager.h b/content/browser/background_sync/background_sync_manager.h index 07b7c43..49f2c92 100644 --- a/content/browser/background_sync/background_sync_manager.h +++ b/content/browser/background_sync/background_sync_manager.h @@ -49,7 +49,8 @@ class CONTENT_EXPORT BackgroundSyncManager ERROR_TYPE_STORAGE, ERROR_TYPE_NOT_FOUND, ERROR_TYPE_NO_SERVICE_WORKER, - ERROR_TYPE_MAX = ERROR_TYPE_NO_SERVICE_WORKER + ERROR_TYPE_NOT_ALLOWED, + ERROR_TYPE_MAX = ERROR_TYPE_NOT_ALLOWED }; using StatusCallback = base::Callback<void(ErrorType)>; diff --git a/content/browser/background_sync/background_sync_manager_unittest.cc b/content/browser/background_sync/background_sync_manager_unittest.cc index 6728ae4..362238b 100644 --- a/content/browser/background_sync/background_sync_manager_unittest.cc +++ b/content/browser/background_sync/background_sync_manager_unittest.cc @@ -256,6 +256,18 @@ class BackgroundSyncManagerTest : public testing::Test { EXPECT_TRUE(called_1); EXPECT_TRUE(called_2); + // Register window clients for the service workers + host_1_.reset(new ServiceWorkerProviderHost( + 34 /* dummy render proces id */, MSG_ROUTING_NONE /* render_frame_id */, + 1 /* dummy provider id */, SERVICE_WORKER_PROVIDER_FOR_WINDOW, + helper_->context()->AsWeakPtr(), nullptr)); + host_1_->SetDocumentUrl(GURL(kPattern1)); + host_2_.reset(new ServiceWorkerProviderHost( + 34 /* dummy render proces id */, MSG_ROUTING_NONE /* render_frame_id */, + 1 /* dummy provider id */, SERVICE_WORKER_PROVIDER_FOR_WINDOW, + helper_->context()->AsWeakPtr(), nullptr)); + host_2_->SetDocumentUrl(GURL(kPattern2)); + // Hang onto the registrations as they need to be "live" when // calling BackgroundSyncManager::Register. helper_->context_wrapper()->FindRegistrationForId( @@ -268,6 +280,14 @@ class BackgroundSyncManagerTest : public testing::Test { base::RunLoop().RunUntilIdle(); EXPECT_TRUE(sw_registration_1_); EXPECT_TRUE(sw_registration_2_); + + sw_registration_1_->active_version()->AddControllee(host_1_.get()); + sw_registration_2_->active_version()->AddControllee(host_2_.get()); + } + + void RemoveWindowClients() { + sw_registration_1_->active_version()->RemoveControllee(host_1_.get()); + sw_registration_2_->active_version()->RemoveControllee(host_2_.get()); } void SetNetwork(net::NetworkChangeNotifier::ConnectionType connection_type) { @@ -482,6 +502,8 @@ class BackgroundSyncManagerTest : public testing::Test { scoped_ptr<BackgroundSyncManager> background_sync_manager_; TestBackgroundSyncManager* test_background_sync_manager_; + scoped_ptr<ServiceWorkerProviderHost> host_1_; + scoped_ptr<ServiceWorkerProviderHost> host_2_; int64 sw_registration_id_1_; int64 sw_registration_id_2_; scoped_refptr<ServiceWorkerRegistration> sw_registration_1_; @@ -1238,4 +1260,22 @@ TEST_F(BackgroundSyncManagerTest, KillManagerMidSync) { EXPECT_EQ(2, sync_events_called_); } +TEST_F(BackgroundSyncManagerTest, RegisterFailsWithoutWindow) { + RemoveWindowClients(); + EXPECT_FALSE(Register(sync_options_1_)); +} + +TEST_F(BackgroundSyncManagerTest, RegisterExistingFailsWithoutWindow) { + EXPECT_TRUE(Register(sync_options_1_)); + RemoveWindowClients(); + EXPECT_FALSE(Register(sync_options_1_)); +} + +TEST_F(BackgroundSyncManagerTest, UnregisterSucceedsWithoutWindow) { + EXPECT_TRUE(Register(sync_options_1_)); + RemoveWindowClients(); + EXPECT_TRUE(Unregister(callback_registration_)); + EXPECT_FALSE(GetRegistration(sync_options_1_)); +} + } // namespace content diff --git a/content/browser/background_sync/background_sync_service_impl.cc b/content/browser/background_sync/background_sync_service_impl.cc index 0975c148..6c5c5af 100644 --- a/content/browser/background_sync/background_sync_service_impl.cc +++ b/content/browser/background_sync/background_sync_service_impl.cc @@ -59,9 +59,10 @@ COMPILE_ASSERT_MATCHING_ENUM(BACKGROUND_SYNC_ERROR_NOT_FOUND, COMPILE_ASSERT_MATCHING_ENUM( BACKGROUND_SYNC_ERROR_NO_SERVICE_WORKER, BackgroundSyncManager::ERROR_TYPE_NO_SERVICE_WORKER); -COMPILE_ASSERT_MATCHING_ENUM( - BACKGROUND_SYNC_ERROR_MAX, - BackgroundSyncManager::ERROR_TYPE_NO_SERVICE_WORKER); +COMPILE_ASSERT_MATCHING_ENUM(BACKGROUND_SYNC_ERROR_NOT_ALLOWED, + BackgroundSyncManager::ERROR_TYPE_NOT_ALLOWED); +COMPILE_ASSERT_MATCHING_ENUM(BACKGROUND_SYNC_ERROR_MAX, + BackgroundSyncManager::ERROR_TYPE_NOT_ALLOWED); COMPILE_ASSERT_MATCHING_ENUM(BACKGROUND_SYNC_NETWORK_STATE_ANY, SyncNetworkState::NETWORK_STATE_ANY); diff --git a/content/browser/background_sync/background_sync_service_impl_unittest.cc b/content/browser/background_sync/background_sync_service_impl_unittest.cc index dbea4ba..39a8166 100644 --- a/content/browser/background_sync/background_sync_service_impl_unittest.cc +++ b/content/browser/background_sync/background_sync_service_impl_unittest.cc @@ -142,11 +142,25 @@ class BackgroundSyncServiceImplTest : public testing::Test { &sw_registration_id_)); base::RunLoop().RunUntilIdle(); EXPECT_TRUE(called); + + // Register window client for the service worker + provider_host_.reset(new ServiceWorkerProviderHost( + 34 /* dummy render proces id */, MSG_ROUTING_NONE /* render_frame_id */, + 1 /* dummy provider id */, SERVICE_WORKER_PROVIDER_FOR_WINDOW, + embedded_worker_helper_->context()->AsWeakPtr(), nullptr)); + provider_host_->SetDocumentUrl(GURL(kServiceWorkerPattern)); + embedded_worker_helper_->context_wrapper()->FindRegistrationForId( sw_registration_id_, GURL(kServiceWorkerPattern).GetOrigin(), base::Bind(FindServiceWorkerRegistrationCallback, &sw_registration_)); base::RunLoop().RunUntilIdle(); EXPECT_TRUE(sw_registration_); + + sw_registration_->active_version()->AddControllee(provider_host_.get()); + } + + void RemoveWindowClient() { + sw_registration_->active_version()->RemoveControllee(provider_host_.get()); } void CreateBackgroundSyncServiceImpl() { @@ -199,6 +213,7 @@ class BackgroundSyncServiceImplTest : public testing::Test { scoped_ptr<EmbeddedWorkerTestHelper> embedded_worker_helper_; scoped_ptr<base::PowerMonitor> power_monitor_; scoped_refptr<BackgroundSyncContextImpl> background_sync_context_; + scoped_ptr<ServiceWorkerProviderHost> provider_host_; int64 sw_registration_id_; scoped_refptr<ServiceWorkerRegistration> sw_registration_; BackgroundSyncServicePtr service_ptr_; @@ -220,6 +235,18 @@ TEST_F(BackgroundSyncServiceImplTest, Register) { EXPECT_EQ("", reg->tag); } +TEST_F(BackgroundSyncServiceImplTest, RegisterWithoutWindow) { + bool called = false; + BackgroundSyncError error; + SyncRegistrationPtr reg; + RemoveWindowClient(); + RegisterOneShot( + default_sync_registration_.Clone(), + base::Bind(&ErrorAndRegistrationCallback, &called, &error, ®)); + EXPECT_TRUE(called); + EXPECT_EQ(BackgroundSyncError::BACKGROUND_SYNC_ERROR_NOT_ALLOWED, error); +} + TEST_F(BackgroundSyncServiceImplTest, Unregister) { bool unregister_called = false; BackgroundSyncError unregister_error; diff --git a/content/browser/service_worker/service_worker_version.cc b/content/browser/service_worker/service_worker_version.cc index d2889ee..9d232d5 100644 --- a/content/browser/service_worker/service_worker_version.cc +++ b/content/browser/service_worker/service_worker_version.cc @@ -977,6 +977,10 @@ void ServiceWorkerVersion::RemoveControllee( FOR_EACH_OBSERVER(Listener, listeners_, OnNoControllees(this)); } +bool ServiceWorkerVersion::HasWindowClients() { + return !GetWindowClientsInternal(false /* include_uncontrolled */).empty(); +} + void ServiceWorkerVersion::AddStreamingURLRequestJob( const ServiceWorkerURLRequestJob* request_job) { DCHECK(streaming_url_request_jobs_.find(request_job) == @@ -1745,17 +1749,8 @@ void ServiceWorkerVersion::GetWindowClients( const ServiceWorkerClientQueryOptions& options) { DCHECK(options.client_type == blink::WebServiceWorkerClientTypeWindow || options.client_type == blink::WebServiceWorkerClientTypeAll); - std::vector<base::Tuple<int, int, std::string>> clients_info; - if (!options.include_uncontrolled) { - for (auto& controllee : controllee_map_) - AddWindowClient(controllee.second, &clients_info); - } else { - for (auto it = - context_->GetClientProviderHostIterator(script_url_.GetOrigin()); - !it->IsAtEnd(); it->Advance()) { - AddWindowClient(it->GetProviderHost(), &clients_info); - } - } + const std::vector<base::Tuple<int, int, std::string>>& clients_info = + GetWindowClientsInternal(options.include_uncontrolled); if (clients_info.empty()) { DidGetWindowClients(request_id, options, @@ -1770,6 +1765,22 @@ void ServiceWorkerVersion::GetWindowClients( weak_factory_.GetWeakPtr(), request_id, options))); } +const std::vector<base::Tuple<int, int, std::string>> +ServiceWorkerVersion::GetWindowClientsInternal(bool include_uncontrolled) { + std::vector<base::Tuple<int, int, std::string>> clients_info; + if (!include_uncontrolled) { + for (auto& controllee : controllee_map_) + AddWindowClient(controllee.second, &clients_info); + } else { + for (auto it = + context_->GetClientProviderHostIterator(script_url_.GetOrigin()); + !it->IsAtEnd(); it->Advance()) { + AddWindowClient(it->GetProviderHost(), &clients_info); + } + } + return clients_info; +} + void ServiceWorkerVersion::DidGetWindowClients( int request_id, const ServiceWorkerClientQueryOptions& options, diff --git a/content/browser/service_worker/service_worker_version.h b/content/browser/service_worker/service_worker_version.h index 3f29230..934103a 100644 --- a/content/browser/service_worker/service_worker_version.h +++ b/content/browser/service_worker/service_worker_version.h @@ -274,6 +274,10 @@ class CONTENT_EXPORT ServiceWorkerVersion // Returns if it has controllee. bool HasControllee() const { return !controllee_map_.empty(); } + // Returns whether the service worker has active window clients under its + // control. + bool HasWindowClients(); + // Adds and removes |request_job| as a dependent job not to stop the // ServiceWorker while |request_job| is reading the stream of the fetch event // response from the ServiceWorker. @@ -473,6 +477,8 @@ class CONTENT_EXPORT ServiceWorkerVersion void GetWindowClients(int request_id, const ServiceWorkerClientQueryOptions& options); + const std::vector<base::Tuple<int, int, std::string>> + GetWindowClientsInternal(bool include_uncontolled); void DidGetWindowClients(int request_id, const ServiceWorkerClientQueryOptions& options, scoped_ptr<ServiceWorkerClients> clients); diff --git a/content/child/background_sync/background_sync_provider.cc b/content/child/background_sync/background_sync_provider.cc index 2bee9bb..bbe67d5 100644 --- a/content/child/background_sync/background_sync_provider.cc +++ b/content/child/background_sync/background_sync_provider.cc @@ -159,6 +159,11 @@ void BackgroundSyncProvider::RegisterCallback( new blink::WebSyncError(blink::WebSyncError::ErrorTypeUnknown, "Background Sync is disabled.")); break; + case BACKGROUND_SYNC_ERROR_NOT_ALLOWED: + callbacks->onError(new blink::WebSyncError( + blink::WebSyncError::ErrorTypeNoPermission, + "Cannot register a sync event without a window client.")); + break; case BACKGROUND_SYNC_ERROR_NO_SERVICE_WORKER: callbacks->onError( new blink::WebSyncError(blink::WebSyncError::ErrorTypeUnknown, @@ -183,6 +188,11 @@ void BackgroundSyncProvider::UnregisterCallback( new blink::WebSyncError(blink::WebSyncError::ErrorTypeUnknown, "Background Sync is disabled.")); break; + case BACKGROUND_SYNC_ERROR_NOT_ALLOWED: + // This error should never be returned from + // BackgroundSyncManager::Unregister + NOTREACHED(); + break; case BACKGROUND_SYNC_ERROR_NO_SERVICE_WORKER: callbacks->onError( new blink::WebSyncError(blink::WebSyncError::ErrorTypeUnknown, @@ -212,6 +222,11 @@ void BackgroundSyncProvider::GetRegistrationCallback( new blink::WebSyncError(blink::WebSyncError::ErrorTypeUnknown, "Background Sync is disabled.")); break; + case BACKGROUND_SYNC_ERROR_NOT_ALLOWED: + // This error should never be returned from + // BackgroundSyncManager::GetRegistration + NOTREACHED(); + break; case BACKGROUND_SYNC_ERROR_NO_SERVICE_WORKER: callbacks->onError( new blink::WebSyncError(blink::WebSyncError::ErrorTypeUnknown, @@ -237,7 +252,8 @@ void BackgroundSyncProvider::GetRegistrationsCallback( callbacks->onSuccess(results); break; case BACKGROUND_SYNC_ERROR_NOT_FOUND: - // This error should never be returned from + case BACKGROUND_SYNC_ERROR_NOT_ALLOWED: + // These errors should never be returned from // BackgroundSyncManager::GetRegistrations NOTREACHED(); break; @@ -277,7 +293,8 @@ void BackgroundSyncProvider::GetPermissionStatusCallback( } break; case BACKGROUND_SYNC_ERROR_NOT_FOUND: - // This error should never be returned from + case BACKGROUND_SYNC_ERROR_NOT_ALLOWED: + // These errors should never be returned from // BackgroundSyncManager::GetPermissionStatus NOTREACHED(); break; diff --git a/content/common/background_sync_service.mojom b/content/common/background_sync_service.mojom index 69a6001..c829960 100644 --- a/content/common/background_sync_service.mojom +++ b/content/common/background_sync_service.mojom @@ -13,7 +13,8 @@ enum BackgroundSyncError { STORAGE, NOT_FOUND, NO_SERVICE_WORKER, - MAX=NO_SERVICE_WORKER + NOT_ALLOWED, + MAX=NOT_ALLOWED }; interface BackgroundSyncService { |