Many content_unittests are failing with ServiceWorkerServicification enabled |
|||||||||
Issue descriptionFor example: $ out/Debug/content_unittests --enable-features=ServiceWorkerServicification --gtest_filter='PresentationServiceImplTest.ListenForScreenAvailability' ... Received signal 11 SEGV_MAPERR 000000000000 #0 0x7f0c3f6f494d base::debug::StackTrace::StackTrace() #1 0x7f0c3f3fc8ea base::debug::StackTrace::StackTrace() #2 0x7f0c3f6f439a base::debug::(anonymous namespace)::StackDumpSignalHandler() #3 0x7f0c25c7b0c0 <unknown> #4 0x000003c74fa7 content::MockRenderProcessHost::CreateURLLoaderFactory() #5 0x7f0c39fee96f content::RenderFrameHostImpl::CreateNetworkServiceDefaultFactoryInternal() #6 0x7f0c3a00dda5 content::RenderFrameHostImpl::CommitNavigation() #7 0x7f0c39fcd86c content::NavigationRequest::CommitNavigation() #8 0x7f0c39fc803e content::NavigationRequest::BeginNavigation() #9 0x7f0c39fdbf02 content::NavigatorImpl::Navigate() #10 0x7f0c39f846b2 content::NavigationControllerImpl::NavigateWithoutEntry() #11 0x7f0c39f83550 content::NavigationControllerImpl::LoadURLWithParams() #12 0x7f0c39f831e4 content::NavigationControllerImpl::LoadURL() #13 0x000003c8fa35 content::NavigationSimulator::SimulateBrowserInitiatedStart() #14 0x000003c8f58a content::NavigationSimulator::Start() #15 0x000003c91214 content::NavigationSimulator::ReadyToCommit() #16 0x000003c91834 content::NavigationSimulator::Commit() #17 0x000003d1810f content::TestWebContents::NavigateAndCommit() #18 0x000003c9f764 content::RenderViewHostTestHarness::NavigateAndCommit() Looks like https://crrev.com/c/1272419 caused these failures. horo@: Would you mind taking a look? Feel free to assign back to me if you don't have enough bandwidth for this.
,
Oct 15
,
Oct 15
We didn't have one. We're going to enable SWS on trunk.
,
Oct 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6d7c3244763fb63c459a8b4d7a54eb2e094aa270 commit 6d7c3244763fb63c459a8b4d7a54eb2e094aa270 Author: Tsuyoshi Horo <horo@chromium.org> Date: Tue Oct 16 01:46:21 2018 Use NotImplementedNetworkURLLoaderFactory in MockRenderProcessHostFactory After crrev.com/c/1272419, all unittests which trigers navigation requires MockRenderProcessHost::url_loader_factory_ when ServiceWorkerServicification is enabled. This is causing many crashes in content_unittests. To fix this, this CL introduces NotImplementedNetworkURLLoaderFactory and use it as default of MockRenderProcessHost's url_loader_factory_. Bug: 895187 Change-Id: I8119845cf73d0a97297efc473db87b84021cf71d Reviewed-on: https://chromium-review.googlesource.com/c/1280072 Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Reviewed-by: Kenichi Ishibashi <bashi@chromium.org> Commit-Queue: Tsuyoshi Horo <horo@chromium.org> Cr-Commit-Position: refs/heads/master@{#599816} [modify] https://crrev.com/6d7c3244763fb63c459a8b4d7a54eb2e094aa270/content/browser/shared_worker/shared_worker_host_unittest.cc [modify] https://crrev.com/6d7c3244763fb63c459a8b4d7a54eb2e094aa270/content/browser/shared_worker/shared_worker_service_impl_unittest.cc [modify] https://crrev.com/6d7c3244763fb63c459a8b4d7a54eb2e094aa270/content/public/test/mock_render_process_host.cc [modify] https://crrev.com/6d7c3244763fb63c459a8b4d7a54eb2e094aa270/content/public/test/mock_render_process_host.h [modify] https://crrev.com/6d7c3244763fb63c459a8b4d7a54eb2e094aa270/content/test/BUILD.gn [add] https://crrev.com/6d7c3244763fb63c459a8b4d7a54eb2e094aa270/content/test/not_implemented_network_url_loader_factory.cc [add] https://crrev.com/6d7c3244763fb63c459a8b4d7a54eb2e094aa270/content/test/not_implemented_network_url_loader_factory.h
,
Oct 16
,
Oct 16
Thanks for the fix horo-san! Unfortunately we still have failures. I'll take a look. ServiceWorkerJobTest.RegisterSameScriptMultipleTimesWhileUninstalling ServiceWorkerJobTest.RemoveControlleeDuringInstall ServiceWorkerJobTest.Update_ScriptUrlChanged ServiceWorkerJobTest.Update_NewVersion ServiceWorkerVersionTest.DispatchEventToStoppedWorker ServiceWorkerRequestHandlerTest.InitializeForNavigation_ExternalFileScheme ServiceWorkerJobTest.RegisterMultipleTimesWhileUninstalling ServiceWorkerJobTest.RegisterWhileUninstalling ServiceWorkerJobTest.RemoveControlleeDuringInstall_RejectActivate
,
Oct 16
falken@: Do you think the following errors can be solved an approach similar to https://crrev.com/c/1164737 ? [ RUN ] ServiceWorkerJobTest.RegisterSameScriptMultipleTimesWhileUninstalling ../../content/browser/service_worker/service_worker_job_unittest.cc:1565: Failure Expected equality of these values: nullptr Which is: NULL registration->waiting_version() Which is: 0x37b2194b2e00 Stack trace: #0 0x0000019025ec testing::internal::UnitTestImpl::CurrentOsStackTraceExceptTop() #1 0x000001901fb9 testing::internal::AssertHelper::operator=() #2 0x0000011567d2 content::ServiceWorkerJobTest_RegisterSameScriptMultipleTimesWhileUninstalling_Test::TestBody() ../../content/browser/service_worker/service_worker_job_unittest.cc:1566: Failure Expected equality of these values: new_version Which is: 0x37b2194b2e00 registration->active_version() Which is: 0x37b2194b3500 Stack trace: #0 0x0000019025ec testing::internal::UnitTestImpl::CurrentOsStackTraceExceptTop() #1 0x000001901fb9 testing::internal::AssertHelper::operator=() #2 0x00000115694a content::ServiceWorkerJobTest_RegisterSameScriptMultipleTimesWhileUninstalling_Test::TestBody() ../../content/browser/service_worker/service_worker_job_unittest.cc:1567: Failure Expected equality of these values: ServiceWorkerVersion::ACTIVATING Which is: 3 new_version->status() Which is: 2
,
Oct 16
Sounds possible. This might have changed with shimazu@'s renderer idle changes? RemoveControllee(); RunUntilIdle() may have different behavior with service worker servicification off vs on.
,
Oct 16
Looking failures in ServiceWorkerJobTest.Update_NewVersion. When S13nSW is off, OnVersionAttributesChanged() is called three times: - SetInstallingVersion - SetWaitingVersion - SetActiveVersion But when S13nSW is on, the last one (SetActiveVersion) is missing. After calling 'first_version->StartUpdate()' both S13nSW and non-S13nSW reach ServiceWorkerRegistration::ActivateWaitingVersionWhenReady(), but IsReadyToActive() returns different value: - S13nSW is on: false - S13nSW is off: true I'm not sure this results in the failure. shimazu@: do you have any idea about the cause?
,
Oct 17
Thanks for letting me know and sorry about that!
I used a wrong flag ('--enable-feature' instead of '--enable-feature*s*') at some point, so I probably missed those failures when landing the CL...
After my change, activation only happens after (1) service worker stops or (2) the renderer's idle timer reports
it's idle when S13nSW is on. Some unit tests assume that activation happens immediately after all controllee are removed, so we need to change the expectations. I'll work on that.
,
Oct 17
Looks like ServiceWorkerRequestHandlerTest.InitializeForNavigation_ExternalFileScheme is still failing after shimazu@'s patch is applied.
,
Oct 17
Oh, that's interesting. ServiceWorkerRequestHandlerTest.InitializeForNavigation_ExternalFileScheme is passing in my local env.
,
Oct 17
I think it's failing on cros bot. Probably have to do a cros build.
,
Oct 17
This looks like a real bug with NetworkService (but not with ServiceWorkerServicifiation). Filed issue 896157 .
,
Oct 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/10bdfaa4154a1faafea4d48e09bf564e939fc531 commit 10bdfaa4154a1faafea4d48e09bf564e939fc531 Author: Makoto Shimazu <shimazu@chromium.org> Date: Wed Oct 17 08:02:43 2018 S13nServiceWorker: Make unittests around activation pass Before S13nServiceWorker, SWVersion::RemoveControllee() immendiately and synchronously triggered activation. However, after S13nServiceWorker, activation happens only after the renderer reports the worker's idle and it successfully finishes. Currently some unit tests assume that RemoveControllee() triggers activation, so this CL makes sure all tasks for activation run by calling RequestTermination() and task runner's methods like RunUntilIdle() and RunPendingTasks(). Bug: 895187 Change-Id: I9cc73d97299a781a366e7f22e2ded90e17a8d367 Reviewed-on: https://chromium-review.googlesource.com/c/1286234 Commit-Queue: Makoto Shimazu <shimazu@chromium.org> Reviewed-by: Matt Falkenhagen <falken@chromium.org> Cr-Commit-Position: refs/heads/master@{#600313} [modify] https://crrev.com/10bdfaa4154a1faafea4d48e09bf564e939fc531/content/browser/service_worker/embedded_worker_test_helper.cc [modify] https://crrev.com/10bdfaa4154a1faafea4d48e09bf564e939fc531/content/browser/service_worker/embedded_worker_test_helper.h [modify] https://crrev.com/10bdfaa4154a1faafea4d48e09bf564e939fc531/content/browser/service_worker/service_worker_job_unittest.cc [modify] https://crrev.com/10bdfaa4154a1faafea4d48e09bf564e939fc531/content/browser/service_worker/service_worker_version_unittest.cc
,
Oct 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c2997950ca4bf61f062e4efef8909feadfe9d911 commit c2997950ca4bf61f062e4efef8909feadfe9d911 Author: Matt Falkenhagen <falken@chromium.org> Date: Wed Oct 17 10:08:40 2018 S13nServiceWorker: Make the provider host for externalfile scheme navigations. This fixes ServiceWorkerRequestHandlerTest.InitializeForNavigation_ExternalFileScheme when ServiceWorkerServicification or NetworkService is enabled. It's the same fix as the non-servicification path, done in r528590. Bug: 895187 , 896157 Change-Id: I8949c0bf002b329a01b1755913d16634dd80e7cd Reviewed-on: https://chromium-review.googlesource.com/c/1286021 Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Commit-Queue: Matt Falkenhagen <falken@chromium.org> Cr-Commit-Position: refs/heads/master@{#600336} [modify] https://crrev.com/c2997950ca4bf61f062e4efef8909feadfe9d911/content/browser/service_worker/service_worker_request_handler.cc
,
Oct 18
,
Oct 24
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4467858024735cdf0091ef6d12b835fdb305c00f commit 4467858024735cdf0091ef6d12b835fdb305c00f Author: Matt Falkenhagen <falken@chromium.org> Date: Wed Oct 24 15:34:23 2018 M71: S13nServiceWorker: Make the provider host for externalfile scheme navigations. This fixes ServiceWorkerRequestHandlerTest.InitializeForNavigation_ExternalFileScheme when ServiceWorkerServicification or NetworkService is enabled. It's the same fix as the non-servicification path, done in r528590. Bug: 895187 , 896157 Change-Id: I8949c0bf002b329a01b1755913d16634dd80e7cd Reviewed-on: https://chromium-review.googlesource.com/c/1286021 Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Commit-Queue: Matt Falkenhagen <falken@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#600336}(cherry picked from commit c2997950ca4bf61f062e4efef8909feadfe9d911) Reviewed-on: https://chromium-review.googlesource.com/c/1297147 Reviewed-by: Matt Falkenhagen <falken@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#291} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [modify] https://crrev.com/4467858024735cdf0091ef6d12b835fdb305c00f/content/browser/service_worker/service_worker_request_handler.cc
,
Oct 24
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4467858024735cdf0091ef6d12b835fdb305c00f Commit: 4467858024735cdf0091ef6d12b835fdb305c00f Author: falken@chromium.org Commiter: falken@chromium.org Date: 2018-10-24 15:34:23 +0000 UTC M71: S13nServiceWorker: Make the provider host for externalfile scheme navigations. This fixes ServiceWorkerRequestHandlerTest.InitializeForNavigation_ExternalFileScheme when ServiceWorkerServicification or NetworkService is enabled. It's the same fix as the non-servicification path, done in r528590. Bug: 895187 , 896157 Change-Id: I8949c0bf002b329a01b1755913d16634dd80e7cd Reviewed-on: https://chromium-review.googlesource.com/c/1286021 Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Commit-Queue: Matt Falkenhagen <falken@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#600336}(cherry picked from commit c2997950ca4bf61f062e4efef8909feadfe9d911) Reviewed-on: https://chromium-review.googlesource.com/c/1297147 Reviewed-by: Matt Falkenhagen <falken@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#291} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by horo@chromium.org
, Oct 15