New issue
Advanced search Search tips

Issue 765064 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

S13nServiceWorker: Enable ServiceWorkerVersionBrowserTests on Mojo FYI bots

Project Member Reported by nhiroki@chromium.org, Sep 14 2017

Issue description

These tests were disabled by [1] because my patch[2] introduced a stricter check for ServiceWorkerVersion::status_ and failed them. We should enable them again or remove them if they're no longer necessary because some of them check the same thing w/ LayoutTests

[1] https://chromium-review.googlesource.com/c/chromium/src/+/664845
[2] https://chromium-review.googlesource.com/c/662957/
 
Labels: -OS-iOS -OS-Fuchsia
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/81f3a6b9554d7f31aefb68c747a72b407a9ca922

commit 81f3a6b9554d7f31aefb68c747a72b407a9ca922
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Tue Sep 26 03:49:20 2017

S13nServiceWorker: Re-enable ServiceWorkerVersionBrowserTests on Mojo FYI bots

These tests were disabled by [1] because [2] introduced a stricter check
for ServiceWorkerVersion::status_ (status_ == NEW) and failed them. We found
this check is wrong because ServiceWorkerScriptURLLoader is also used for
importScripts() that can be called during the Install event (status_ ==
INSTALLING).

This CL makes ServiceWorkerScriptURLLoaderFactory determines whether to use
ServiceWorkerScriptURLLoader more precisely, and changes the check in the ctor
of ServiceWorkerScriptURLLoader based on that.

This CL also corrects the timings to update the service worker status in the
browser tests to avoid the check failure.

[1] https://chromium-review.googlesource.com/c/664845/
[2] https://chromium-review.googlesource.com/c/662957/

Bug:  748415 ,  765064 ,  767916 
Change-Id: Id7786510170129514058e6f5c5d4bd1860b4be1b
Reviewed-on: https://chromium-review.googlesource.com/674143
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504277}
[modify] https://crrev.com/81f3a6b9554d7f31aefb68c747a72b407a9ca922/content/browser/service_worker/service_worker_browsertest.cc
[modify] https://crrev.com/81f3a6b9554d7f31aefb68c747a72b407a9ca922/content/browser/service_worker/service_worker_script_url_loader.cc
[modify] https://crrev.com/81f3a6b9554d7f31aefb68c747a72b407a9ca922/content/browser/service_worker/service_worker_script_url_loader_factory.cc
[modify] https://crrev.com/81f3a6b9554d7f31aefb68c747a72b407a9ca922/testing/buildbot/filters/mojo.fyi.network_content_browsertests.filter

Labels: M-63
Status: Fixed (was: Assigned)

Sign in to add a comment