New issue
Advanced search Search tips

Issue 827051 link

Starred by 1 user
Status: Fixed
Owner:
bashi@chromium.org
Closed: Mar 2018
Cc:
kinuko@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

S13nServiceWorker: Hitting NOTREACHED() in ServiceWorkerSubresourceLoaderFactory::Clone()

Project Member Reported by bashi@chromium.org, Mar 29 2018 
What steps will reproduce the problem?
(1) $ third_party/WebKit/Tools/Scripts/run-blink-wptserve
(2) $ out/Release/content_shell --enable-features=NetworkService http://localhost:8001/service-workers/service-worker/fetch-request-xhr-sync.https.html
(3) Wait for several seconds (~10 sec)

What is the expected result?
No renderer crash

What happens instead?
Hit NOTREACHED()

Please use labels and text to provide additional information.
Example stack trace:

[1:1:0329/155354.492677:FATAL:service_worker_subresource_loader.cc(575)] Check failed: false.
#0 0x7fb1e3fc041c base::debug::StackTrace::StackTrace()       
#1 0x7fb1e3fea72b logging::LogMessage::~LogMessage()       
#2 0x7fb1e35ff129 content::ServiceWorkerSubresourceLoaderFactory::Clone()
#3 0x7fb1e28a6fa3 network::mojom::URLLoaderFactoryStubDispatch::Accept()
#4 0x7fb1e44852f2 mojo::InterfaceEndpointClient::HandleValidatedMessage()                                                                         #5 0x7fb1e4484be6 mojo::FilterChain::Accept()                                                            
#6 0x7fb1e44867a2 mojo::InterfaceEndpointClient::HandleIncomingMessage()
#7 0x7fb1e448d5fd mojo::internal::MultiplexRouter::ProcessIncomingMessage()  
#8 0x7fb1e448c9c0 mojo::internal::MultiplexRouter::Accept()                                                                                       #9 0x7fb1e4484be6 mojo::FilterChain::Accept()            
#10 0x7fb1e447f43b mojo::Connector::ReadSingleMessage() 
#11 0x7fb1e447fff4 mojo::Connector::ReadAllAvailableMessages() 
#12 0x7fb1e447fe56 mojo::Connector::OnHandleReadyInternal()
#13 0x7fb1e4480824 mojo::SimpleWatcher::DiscardReadyState()
...

For graphics-related bugs, please copy/paste the contents of the about:gpu
page at the end of this report.

Comment 1 by bashi@chromium.org, Mar 29 2018

Cc: kinuko@chromium.org
I'm not sure ServiceWorkerSubresourceLoaderFactor::Clone() shouldn't be called. It seems that the method isn't implemented from the beginning.
https://chromium-review.googlesource.com/607948

If we should implement this method, doing something similar to BlobURLLoaderFactory probably makes sense? BlobURLLoaderFactory uses a binding set to maintain connected pipes.
https://cs.chromium.org/chromium/src/storage/browser/blob/blob_url_loader_factory.cc?l=40&rcl=00018286b4aa58ffafc419ca73ab8965923f34ec

kinuko@: Do you have any idea? Who should I ask this kind of network service related questions in general?

Comment 2 by falken@chromium.org, Mar 29 2018 

I think this is a known limitation, the Clone() method is apparently provided for sync requests and this is testing sync XHR:
https://cs.chromium.org/chromium/src/content/browser/service_worker/service_worker_script_loader_factory.cc?l=84&rcl=8b9210eedc763b7e3d49bb0adf8b7ee9671cf3d3

I suspect the NOTREACHED should be treated as NOTIMPLEMENTED

Agree we probably need to do what the other factories do.

(and kinuko@ is the correct person to ask :)

Comment 3 by kinuko@chromium.org, Mar 29 2018 

Yep, I think we should implement it. When we initially implemented the factory we didn't need it but things have changed a lot since then...

And yes, doing similar to what BlobURLLoaderFactory's doing should work. As you'll see we'll need to change the part where we do simple MakeStrongBinding thing in sw_provider_context but worker_fetch_context_impl accordingly.
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 30 2018 

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

commit c2d27c03b7e6431eccc04d0de79a0e5c5899625f
Author: Kenichi Ishibashi <bashi@chromium.org>
Date: Fri Mar 30 06:06:23 2018

S13nSW: Implement ServiceWorkerSubresourceLoaderFactory::Clone()

To support Clone(), an instance of ServiceWorkerSubresourceLoaderFactory
needs to maintain more than one bindings. Use BindingSet to handle
multiple bindings. This is similar approach to BlobURLLoaderFactory.

Bug:  827051 
Change-Id: I17f46accd3a66160d19d01aea1cb026a3fb999be
Reviewed-on: https://chromium-review.googlesource.com/987218
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547115}
[modify] https://crrev.com/c2d27c03b7e6431eccc04d0de79a0e5c5899625f/content/renderer/service_worker/service_worker_provider_context.cc
[modify] https://crrev.com/c2d27c03b7e6431eccc04d0de79a0e5c5899625f/content/renderer/service_worker/service_worker_subresource_loader.cc
[modify] https://crrev.com/c2d27c03b7e6431eccc04d0de79a0e5c5899625f/content/renderer/service_worker/service_worker_subresource_loader.h
[modify] https://crrev.com/c2d27c03b7e6431eccc04d0de79a0e5c5899625f/content/renderer/service_worker/service_worker_subresource_loader_unittest.cc
[modify] https://crrev.com/c2d27c03b7e6431eccc04d0de79a0e5c5899625f/content/renderer/service_worker/worker_fetch_context_impl.cc

Comment 5 by bashi@chromium.org, Mar 30 2018

Status: Fixed (was: Available)

Comment 6 by falken@chromium.org, Jul 3

Labels: M-67
►

Sign in to add a comment