Commit 41adee80 authored by Istiaque Ahmed's avatar Istiaque Ahmed Committed by Commit Bot

[Ext SW] Fix installation rejection in FoundRegistrationForStartWorker.

ServiceWorkerContextWrapper::StartWorkerForScope searches for
registration with active or installing worker through
DidFindRegistrationForFindImpl. For the case where there is an
installingworker, DidFindRegistrationForFindImpl can see a
registration.installing_version(), but the version can be gone by
the time it PostTask-s to FoundRegistrationForStartWorker
if the service worker script rejected the 'install' event.

This CL removes the DCHECK that incorrectly assumes either active
or installing version *must* be present
in FoundRegistrationForStartWorker. The DCHECK failure is evident
in the flaky extension test failure:
ServiceWorkerBasedBackgroundTest.WorkerStartFailureClearsPendingTasks.
The CL also enables the mentioned test.

Bug: 1063476
Change-Id: I12dd3b5f2155bc8b8677f9734443832c1a96fe61
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2119208Reviewed-by: default avatarIstiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: default avatarMatt Falkenhagen <falken@chromium.org>
Commit-Queue: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#753555}
parent 8594c79e
......@@ -1946,11 +1946,8 @@ IN_PROC_BROWSER_TEST_F(ServiceWorkerBasedBackgroundTest,
// PendingTasks correctly. Also tests that subsequent tasks are properly
// cleared.
// Regression test for https://crbug.com/1019161.
//
// Flaky (https://crbug.com/1063476): Fails DCHECK(version_ptr) in
// ServiceWorkerContextWrapper.
IN_PROC_BROWSER_TEST_F(ServiceWorkerBasedBackgroundTest,
DISABLED_WorkerStartFailureClearsPendingTasks) {
WorkerStartFailureClearsPendingTasks) {
content::StoragePartition* storage_partition =
content::BrowserContext::GetDefaultStoragePartition(browser()->profile());
content::ServiceWorkerContext* context =
......@@ -1990,14 +1987,14 @@ IN_PROC_BROWSER_TEST_F(ServiceWorkerBasedBackgroundTest,
ASSERT_EQ(kTestExtensionId, extension->id());
LazyContextId context_id(browser()->profile(), extension->id(),
extension->url());
ServiceWorkerStartFailureObserver worker_start_failure_observer(
extension->id());
// Let the worker start so it rejects 'install' event. This causes the worker
// to stop.
observer.WaitForWorkerStart();
observer.WaitForWorkerStop();
ServiceWorkerStartFailureObserver worker_start_failure_observer(
extension->id());
ServiceWorkerTaskQueue* service_worker_task_queue =
ServiceWorkerTaskQueue::Get(browser()->profile());
// Adding a pending task to ServiceWorkerTaskQueue will try to start the
......
......@@ -126,12 +126,17 @@ void FoundRegistrationForStartWorker(
? registration->active_version()
: registration->installing_version();
// Since FindRegistrationForScope returned
// blink::ServiceWorkerStatusCode::kOk, there must be either: -
// an active version, which optionally might have activated from a waiting
// blink::ServiceWorkerStatusCode::kOk, there must have been either:
// - an active version, which optionally might have activated from a waiting
// version (as DidFindRegistrationForFindImpl will activate any waiting
// version).
// - or an installing version.
DCHECK(version_ptr);
// However, if the installation is rejected, the installing version can go
// away by the time we reach here from DidFindRegistrationForFindImpl.
if (!version_ptr) {
callback_runner->PostTask(FROM_HERE, std::move(failure_callback));
return;
}
// Note: There might be a remote possibility that |registration|'s |version|
// might change between here and DidStartWorker, so bind |version| to
......
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