Issue metadata
Sign in to add a comment
|
NetworkService: http/tests/workers/shared-worker-shared.html may be flaky |
||||||||||||||||||||
Issue descriptionhttps://ci.chromium.org/buildbot/chromium.fyi/Mojo%20Linux/11999 http/tests/workers/shared-worker-shared.html PASS: Creating SharedWorker with different URLs but the same name PASS: Accessing new instance of shared worker: self.foo: undefined PASS: Setting global variable in shared worker: self.foo = 1234: 1234 -PASS: Accessing simultaneously-loaded instance of shared worker: self.foo: 1234 +FAIL: Accessing simultaneously-loaded instance of shared worker: self.foo: undefined PASS: Accessing new instance of shared worker: self.foo: undefined PASS: Accessing already-loaded instance of shared worker: self.foo: 1234 DONE I suspect this is because I introduced the hop to the IO thread during shared worker startup. Probably when two shared workers for the same URL are created near the same time, they end up creating two shared worker execution contexts since the second one doesn't realize the first one is being created until we hop back to the UI thread. Need to ensure only one is created.
,
Apr 12 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e860f24ff11550576fa362b72252f5f6adb33221 commit e860f24ff11550576fa362b72252f5f6adb33221 Author: Matt Falkenhagen <falken@chromium.org> Date: Thu Apr 12 15:37:15 2018 shared worker: Move DevTools WorkerCreated call to SharedWorkerHost. This makes more sense as SharedWorkerHost's destructor calls WorkerDestroyed to balance it out. It also simplifies the params to Start(). Not strictly needed for the linked bug. Refactoring that will make it easier to fix. Bug: 830288 Change-Id: I3be38ad7cc58c35bef301cd21074987ea9e5c64d Reviewed-on: https://chromium-review.googlesource.com/1004887 Commit-Queue: Matt Falkenhagen <falken@chromium.org> Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org> Cr-Commit-Position: refs/heads/master@{#550213} [modify] https://crrev.com/e860f24ff11550576fa362b72252f5f6adb33221/content/browser/shared_worker/shared_worker_host.cc [modify] https://crrev.com/e860f24ff11550576fa362b72252f5f6adb33221/content/browser/shared_worker/shared_worker_host.h [modify] https://crrev.com/e860f24ff11550576fa362b72252f5f6adb33221/content/browser/shared_worker/shared_worker_service_impl.cc
,
Apr 16 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d23f83cb77dc0d7ffc992bdfd1b5584f808783a8 commit d23f83cb77dc0d7ffc992bdfd1b5584f808783a8 Author: Matt Falkenhagen <falken@chromium.org> Date: Mon Apr 16 06:09:16 2018 shared worker: Extract mock shared worker classes to their own file. These will be useful for planned unit tests for SharedWorkerHost which will be in its own test file. Also: * Add missing EXPECT_TRUE for some Check*() calls, which return a bool. * Remove calls to deprecated RunAllPendingInMessageLoop(). * Change some CHECK to DCHECK and rm unneeded includes. Bug: 830288 Change-Id: I3f7ed117fdde4d859047f6e95b5080119857daec Reviewed-on: https://chromium-review.googlesource.com/1013817 Commit-Queue: Matt Falkenhagen <falken@chromium.org> Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org> Cr-Commit-Position: refs/heads/master@{#550941} [add] https://crrev.com/d23f83cb77dc0d7ffc992bdfd1b5584f808783a8/content/browser/shared_worker/mock_shared_worker.cc [add] https://crrev.com/d23f83cb77dc0d7ffc992bdfd1b5584f808783a8/content/browser/shared_worker/mock_shared_worker.h [modify] https://crrev.com/d23f83cb77dc0d7ffc992bdfd1b5584f808783a8/content/browser/shared_worker/shared_worker_service_impl_unittest.cc [modify] https://crrev.com/d23f83cb77dc0d7ffc992bdfd1b5584f808783a8/content/test/BUILD.gn
,
Apr 16 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/694a3c4a8ade9088baf44a99c30f686c57b9c8d6 commit 694a3c4a8ade9088baf44a99c30f686c57b9c8d6 Author: Matt Falkenhagen <falken@chromium.org> Date: Mon Apr 16 08:37:18 2018 shared worker: Compare URLs instead of strings in unit tests. The strings just get converted to URLs. Simpler to just use URLs directly. More cleanup for the linked bug in preparation for more unit tests. Bug: 830288 Change-Id: Ie467b841ee8c07a2bf22c26d4151c1711943f69f Reviewed-on: https://chromium-review.googlesource.com/1013022 Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org> Commit-Queue: Matt Falkenhagen <falken@chromium.org> Cr-Commit-Position: refs/heads/master@{#550949} [modify] https://crrev.com/694a3c4a8ade9088baf44a99c30f686c57b9c8d6/content/browser/shared_worker/mock_shared_worker.cc [modify] https://crrev.com/694a3c4a8ade9088baf44a99c30f686c57b9c8d6/content/browser/shared_worker/mock_shared_worker.h [modify] https://crrev.com/694a3c4a8ade9088baf44a99c30f686c57b9c8d6/content/browser/shared_worker/shared_worker_service_impl_unittest.cc
,
Apr 16 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c038460eafe97d3bd5e760967e3173ed13640e52 commit c038460eafe97d3bd5e760967e3173ed13640e52 Author: Matt Falkenhagen <falken@chromium.org> Date: Mon Apr 16 10:37:28 2018 shared worker: Simplify telling devtools about start and stop of worker. SharedWorkerHost was calling WorkedDestroyed at three callsites using two flags to determine when to do so. Use a RAII helper class to manage the state instead. One motiviation is fixing 830288 which is going to add more state to SharedWorkerHost (creating a host before calling start), so when to call WorkerDestroyed would get more complicated. Bug: 830288 Change-Id: Ie3f135d925091518b172a4afcd8c475656aa9db4 Reviewed-on: https://chromium-review.googlesource.com/1013360 Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org> Commit-Queue: Matt Falkenhagen <falken@chromium.org> Cr-Commit-Position: refs/heads/master@{#550958} [modify] https://crrev.com/c038460eafe97d3bd5e760967e3173ed13640e52/content/browser/shared_worker/shared_worker_host.cc [modify] https://crrev.com/c038460eafe97d3bd5e760967e3173ed13640e52/content/browser/shared_worker/shared_worker_host.h
,
Apr 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e860f24ff11550576fa362b72252f5f6adb33221 commit e860f24ff11550576fa362b72252f5f6adb33221 Author: Matt Falkenhagen <falken@chromium.org> Date: Thu Apr 12 15:37:15 2018 shared worker: Move DevTools WorkerCreated call to SharedWorkerHost. This makes more sense as SharedWorkerHost's destructor calls WorkerDestroyed to balance it out. It also simplifies the params to Start(). Not strictly needed for the linked bug. Refactoring that will make it easier to fix. Bug: 830288 Change-Id: I3be38ad7cc58c35bef301cd21074987ea9e5c64d Reviewed-on: https://chromium-review.googlesource.com/1004887 Commit-Queue: Matt Falkenhagen <falken@chromium.org> Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org> Cr-Commit-Position: refs/heads/master@{#550213} [modify] https://crrev.com/e860f24ff11550576fa362b72252f5f6adb33221/content/browser/shared_worker/shared_worker_host.cc [modify] https://crrev.com/e860f24ff11550576fa362b72252f5f6adb33221/content/browser/shared_worker/shared_worker_host.h [modify] https://crrev.com/e860f24ff11550576fa362b72252f5f6adb33221/content/browser/shared_worker/shared_worker_service_impl.cc
,
Apr 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d23f83cb77dc0d7ffc992bdfd1b5584f808783a8 commit d23f83cb77dc0d7ffc992bdfd1b5584f808783a8 Author: Matt Falkenhagen <falken@chromium.org> Date: Mon Apr 16 06:09:16 2018 shared worker: Extract mock shared worker classes to their own file. These will be useful for planned unit tests for SharedWorkerHost which will be in its own test file. Also: * Add missing EXPECT_TRUE for some Check*() calls, which return a bool. * Remove calls to deprecated RunAllPendingInMessageLoop(). * Change some CHECK to DCHECK and rm unneeded includes. Bug: 830288 Change-Id: I3f7ed117fdde4d859047f6e95b5080119857daec Reviewed-on: https://chromium-review.googlesource.com/1013817 Commit-Queue: Matt Falkenhagen <falken@chromium.org> Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org> Cr-Commit-Position: refs/heads/master@{#550941} [add] https://crrev.com/d23f83cb77dc0d7ffc992bdfd1b5584f808783a8/content/browser/shared_worker/mock_shared_worker.cc [add] https://crrev.com/d23f83cb77dc0d7ffc992bdfd1b5584f808783a8/content/browser/shared_worker/mock_shared_worker.h [modify] https://crrev.com/d23f83cb77dc0d7ffc992bdfd1b5584f808783a8/content/browser/shared_worker/shared_worker_service_impl_unittest.cc [modify] https://crrev.com/d23f83cb77dc0d7ffc992bdfd1b5584f808783a8/content/test/BUILD.gn
,
Apr 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/694a3c4a8ade9088baf44a99c30f686c57b9c8d6 commit 694a3c4a8ade9088baf44a99c30f686c57b9c8d6 Author: Matt Falkenhagen <falken@chromium.org> Date: Mon Apr 16 08:37:18 2018 shared worker: Compare URLs instead of strings in unit tests. The strings just get converted to URLs. Simpler to just use URLs directly. More cleanup for the linked bug in preparation for more unit tests. Bug: 830288 Change-Id: Ie467b841ee8c07a2bf22c26d4151c1711943f69f Reviewed-on: https://chromium-review.googlesource.com/1013022 Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org> Commit-Queue: Matt Falkenhagen <falken@chromium.org> Cr-Commit-Position: refs/heads/master@{#550949} [modify] https://crrev.com/694a3c4a8ade9088baf44a99c30f686c57b9c8d6/content/browser/shared_worker/mock_shared_worker.cc [modify] https://crrev.com/694a3c4a8ade9088baf44a99c30f686c57b9c8d6/content/browser/shared_worker/mock_shared_worker.h [modify] https://crrev.com/694a3c4a8ade9088baf44a99c30f686c57b9c8d6/content/browser/shared_worker/shared_worker_service_impl_unittest.cc
,
Apr 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c038460eafe97d3bd5e760967e3173ed13640e52 commit c038460eafe97d3bd5e760967e3173ed13640e52 Author: Matt Falkenhagen <falken@chromium.org> Date: Mon Apr 16 10:37:28 2018 shared worker: Simplify telling devtools about start and stop of worker. SharedWorkerHost was calling WorkedDestroyed at three callsites using two flags to determine when to do so. Use a RAII helper class to manage the state instead. One motiviation is fixing 830288 which is going to add more state to SharedWorkerHost (creating a host before calling start), so when to call WorkerDestroyed would get more complicated. Bug: 830288 Change-Id: Ie3f135d925091518b172a4afcd8c475656aa9db4 Reviewed-on: https://chromium-review.googlesource.com/1013360 Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org> Commit-Queue: Matt Falkenhagen <falken@chromium.org> Cr-Commit-Position: refs/heads/master@{#550958} [modify] https://crrev.com/c038460eafe97d3bd5e760967e3173ed13640e52/content/browser/shared_worker/shared_worker_host.cc [modify] https://crrev.com/c038460eafe97d3bd5e760967e3173ed13640e52/content/browser/shared_worker/shared_worker_host.h
,
Apr 18 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/be16ca76bc0ab6b6c5224234db82ac648933dc7b commit be16ca76bc0ab6b6c5224234db82ac648933dc7b Author: Matt Falkenhagen <falken@chromium.org> Date: Wed Apr 18 18:38:40 2018 NetworkService: shared worker: Fix race when two workers start simultaneously. r550158 introduced a regression when NetworkService/S13nServiceWorker is enabled, because it hops to the IO thread during the start worker sequence in order to support service worker interception. When two clients call `new SharedWorker(url)` at the same time, the expectation is that only one worker thread is started, and the clients share the worker. But there was a race where the first request is on the IO thread when the second request starts, and the second request doesn't know the first request is happening, resulting in the creation of two workers. The fix is to create SharedWorkerHost on the UI thread before hopping to the IO thread, so the second request will notice the host and use it instead of creating a new one. This deflakes http/tests/workers/shared-worker-shared.html when run with the NetworkService flag. Change-Id: Ib4d189c454fc3cdab941af00596b799e2f6dc172 Bug: 830288 Reviewed-on: https://chromium-review.googlesource.com/1013440 Commit-Queue: Matt Falkenhagen <falken@chromium.org> Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org> Cr-Commit-Position: refs/heads/master@{#551757} [modify] https://crrev.com/be16ca76bc0ab6b6c5224234db82ac648933dc7b/content/browser/shared_worker/mock_shared_worker.cc [modify] https://crrev.com/be16ca76bc0ab6b6c5224234db82ac648933dc7b/content/browser/shared_worker/mock_shared_worker.h [modify] https://crrev.com/be16ca76bc0ab6b6c5224234db82ac648933dc7b/content/browser/shared_worker/shared_worker_host.cc [modify] https://crrev.com/be16ca76bc0ab6b6c5224234db82ac648933dc7b/content/browser/shared_worker/shared_worker_host.h [add] https://crrev.com/be16ca76bc0ab6b6c5224234db82ac648933dc7b/content/browser/shared_worker/shared_worker_host_unittest.cc [modify] https://crrev.com/be16ca76bc0ab6b6c5224234db82ac648933dc7b/content/browser/shared_worker/shared_worker_service_impl.cc [modify] https://crrev.com/be16ca76bc0ab6b6c5224234db82ac648933dc7b/content/browser/shared_worker/shared_worker_service_impl.h [modify] https://crrev.com/be16ca76bc0ab6b6c5224234db82ac648933dc7b/content/browser/shared_worker/shared_worker_service_impl_unittest.cc [modify] https://crrev.com/be16ca76bc0ab6b6c5224234db82ac648933dc7b/content/test/BUILD.gn
,
Apr 19 2018
|
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by falken@chromium.org
, Apr 10 2018