New issue
Advanced search Search tips

Issue 894819 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Task


Sign in to add a comment

Deprecate WebDataConsumerHandle

Project Member Reported by yhirano@chromium.org, Oct 12

Issue description

We should use mojo datapipe instead.
 
Blockedon: 884807 894815
Blocking: 853518
Status: Started (was: Assigned)
Thanks, I was just about to file this bug.  This will allow pass-through service worker fetch handlers to avoid copying the body data to a new mojo::DataPipe.

I have two WIP CLs.  This one passes the mojo::DataPipe down through the loader stack instead of the WebDataConsumerHandle:

https://chromium-review.googlesource.com/c/chromium/src/+/1251841

Its passing CQ, but I need to do some cleanup and rejigger how devtools data is provided.

I'll then end up removing WebDataConsumerHandle in this CL:

https://chromium-review.googlesource.com/c/chromium/src/+/1257963
Blocking: 891637
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 6

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

commit 06b3921273304ae12c0a71dea285464fce8cb419
Author: Ben Kelly <wanderview@chromium.org>
Date: Tue Nov 06 15:46:04 2018

Remove WebDataConsumerHandle usage from DataPipeAndDataBytesConsumer.

As a step towards removing WebDataConsumerHandle stop using the type in
DataPipeAndDataBytesConsumer.  Instead this code can instead use
DataPipeBytesConsumer to read the mojo::DataPipe.

Bug: 894819
Change-Id: Ie9cad46dfa9d67998d23bf0efd6300870428f3cb
Reviewed-on: https://chromium-review.googlesource.com/c/1313389
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605689}
[modify] https://crrev.com/06b3921273304ae12c0a71dea285464fce8cb419/third_party/blink/renderer/core/fetch/data_pipe_bytes_consumer.cc
[modify] https://crrev.com/06b3921273304ae12c0a71dea285464fce8cb419/third_party/blink/renderer/core/fetch/form_data_bytes_consumer.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Nov 7

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

commit 021d9e70103633e54c8304e1091721929c3b3e1d
Author: Dmitry Titov <dimich@chromium.org>
Date: Wed Nov 07 01:09:43 2018

Revert "Remove WebDataConsumerHandle usage from DataPipeAndDataBytesConsumer."

This reverts commit 06b3921273304ae12c0a71dea285464fce8cb419.

Reason for revert: to fix the test external/wpt/workers/semantics/structured-clone/shared.html

first started to fail in build: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20Tests%20%28dbg%29%281%29/75530

Original change's description:
> Remove WebDataConsumerHandle usage from DataPipeAndDataBytesConsumer.
> 
> As a step towards removing WebDataConsumerHandle stop using the type in
> DataPipeAndDataBytesConsumer.  Instead this code can instead use
> DataPipeBytesConsumer to read the mojo::DataPipe.
> 
> Bug: 894819
> Change-Id: Ie9cad46dfa9d67998d23bf0efd6300870428f3cb
> Reviewed-on: https://chromium-review.googlesource.com/c/1313389
> Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
> Commit-Queue: Ben Kelly <wanderview@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#605689}

TBR=yhirano@chromium.org,wanderview@chromium.org

Change-Id: I6a792df67c062020085362929f402c433d3fa60e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 894819
Reviewed-on: https://chromium-review.googlesource.com/c/1321856
Reviewed-by: Dmitry Titov <dimich@chromium.org>
Commit-Queue: Dmitry Titov <dimich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605909}
[modify] https://crrev.com/021d9e70103633e54c8304e1091721929c3b3e1d/third_party/blink/renderer/core/fetch/data_pipe_bytes_consumer.cc
[modify] https://crrev.com/021d9e70103633e54c8304e1091721929c3b3e1d/third_party/blink/renderer/core/fetch/form_data_bytes_consumer.cc

Thanks for the revert!

The test is failing due to a CHECK:

09:00:18.458 6416 worker/7 external/wpt/workers/semantics/structured-clone/shared.html crashed, (stderr lines):
09:00:18.458 6416   [1:1:1106/090017.965248:FATAL:body_stream_buffer.cc(526)] Check failed: loader_. 
09:00:18.458 6416   #0 0x7f03782db3ad base::debug::StackTrace::StackTrace()
09:00:18.458 6416   #1 0x7f0377fd0e1a base::debug::StackTrace::StackTrace()
09:00:18.458 6416   #2 0x7f0378042beb logging::LogMessage::~LogMessage()
09:00:18.458 6416   #3 0x7f03625deea1 blink::BodyStreamBuffer::EndLoading()
09:00:18.458 6416   #4 0x7f03625e0b6c blink::BodyStreamBuffer::LoaderClient::DidFetchDataLoadedArrayBuffer()
09:00:18.458 6416   #5 0x7f03625f29bd blink::(anonymous namespace)::FetchDataLoaderAsArrayBuffer::OnStateChange()
09:00:18.458 6416   #6 0x7f0362619750 blink::(anonymous namespace)::DataPipeAndDataBytesConsumer::DataPipeGetterCallback()
09:00:18.458 6416   #7 0x7f0362619eb4 _ZN4base8internal13FunctorTraitsIMN5blink12_GLOBAL__N_128DataPipeAndDataBytesConsumerEFvimEvE6InvokeIS6_NS2_14WeakPersistentIS4_EEJimEEEvT_OT0_DpOT1_
09:00:18.459 6416   #8 0x7f0362619db8 _ZN4base8internal12InvokeHelperILb1EvE8MakeItSoIMN5blink12_GLOBAL__N_128DataPipeAndDataBytesConsumerEFvimENS4_14WeakPersistentIS6_EEJimEEEvOT_OT0_DpOT1_
09:00:18.459 6416   #9 0x7f0362619d20 _ZN4base8internal7InvokerINS0_9BindStateIMN5blink12_GLOBAL__N_128DataPipeAndDataBytesConsumerEFvimEJNS3_14WeakPersistentIS5_EEEEEFvimEE7RunImplIS7_NSt3__15tupleIJS9_EEEJLm0EEEEvOT_OT0_NSE_16integer_sequenceImJXspT1_EEEEOiOm
09:00:18.459 6416   #10 0x7f0362619c48 _ZN4base8internal7InvokerINS0_9BindStateIMN5blink12_GLOBAL__N_128DataPipeAndDataBytesConsumerEFvimEJNS3_14WeakPersistentIS5_EEEEEFvimEE7RunOnceEPNS0_13BindStateBaseEim
09:00:18.459 6416   #11 0x7f036261d1c2 _ZNO4base12OnceCallbackIFvimEE3RunEim
09:00:18.459 6416   #12 0x7f036261d142 WTF::ThreadCheckingCallbackWrapper<>::RunInternal()
09:00:18.459 6416   #13 0x7f036261cea7 WTF::ThreadCheckingCallbackWrapper<>::Run()
09:00:18.459 6416   #14 0x7f036261d0ed _ZN4base8internal13FunctorTraitsIMN3WTF29ThreadCheckingCallbackWrapperINS_12OnceCallbackIFvimEEES5_EEFvimEvE6InvokeIS9_NSt3__110unique_ptrIS7_NSC_14default_deleteIS7_EEEEJimEEEvT_OT0_DpOT1_
09:00:18.459 6416   #15 0x7f036261d02a _ZN4base8internal12InvokeHelperILb0EvE8MakeItSoIMN3WTF29ThreadCheckingCallbackWrapperINS_12OnceCallbackIFvimEEES7_EEFvimEJNSt3__110unique_ptrIS9_NSC_14default_deleteIS9_EEEEimEEEvOT_DpOT0_
09:00:18.459 6416   #16 0x7f036261cfa0 _ZN4base8internal7InvokerINS0_9BindStateIMN3WTF29ThreadCheckingCallbackWrapperINS_12OnceCallbackIFvimEEES6_EEFvimEJNSt3__110unique_ptrIS8_NSB_14default_deleteIS8_EEEEEEES6_E7RunImplISA_NSB_5tupleIJSF_EEEJLm0EEEEvOT_OT0_NSB_16integer_sequenceImJXspT1_EEEEOiOm
09:00:18.459 6416   #17 0x7f036261cf28 _ZN4base8internal7InvokerINS0_9BindStateIMN3WTF29ThreadCheckingCallbackWrapperINS_12OnceCallbackIFvimEEES6_EEFvimEJNSt3__110unique_ptrIS8_NSB_14default_deleteIS8_EEEEEEES6_E7RunOnceEPNS0_13BindStateBaseEim
09:00:18.459 6416   #18 0x7f035f59be42 _ZNO4base12OnceCallbackIFvimEE3RunEim
09:00:18.459 6416   #19 0x7f035f887838 network::mojom::blink::DataPipeGetter_Read_ForwardToCallback::Accept()
09:00:18.459 6416   #20 0x7f0378465e52 mojo::InterfaceEndpointClient::HandleValidatedMessage()
09:00:18.459 6416   #21 0x7f03784649e1 mojo::InterfaceEndpointClient::HandleIncomingMessageThunk::Accept()
09:00:18.459 6416   #22 0x7f0378462e82 mojo::FilterChain::Accept()
09:00:18.459 6416   #23 0x7f0378468bf5 mojo::InterfaceEndpointClient::HandleIncomingMessage()
09:00:18.459 6416   #24 0x7f03784756bb mojo::internal::MultiplexRouter::ProcessIncomingMessage()
09:00:18.459 6416   #25 0x7f0378474c05 mojo::internal::MultiplexRouter::Accept()
09:00:18.459 6416   #26 0x7f0378462e82 mojo::FilterChain::Accept()
09:00:18.459 6416   #27 0x7f037845429b mojo::Connector::ReadSingleMessage()
09:00:18.459 6416   #28 0x7f03784554e1 mojo::Connector::ReadAllAvailableMessages()
09:00:18.459 6416   #29 0x7f0378455205 mojo::Connector::OnHandleReadyInternal()
09:00:18.459 6416   #30 0x7f03784550eb mojo::Connector::OnWatcherHandleReady()
09:00:18.459 6416   #31 0x7f037845925f _ZN4base8internal13FunctorTraitsIMN4mojo9ConnectorEFvjEvE6InvokeIS5_PS3_JjEEEvT_OT0_DpOT1_
09:00:18.459 6416   #32 0x7f03784591bf _ZN4base8internal12InvokeHelperILb0EvE8MakeItSoIRKMN4mojo9ConnectorEFvjEJPS5_jEEEvOT_DpOT0_
09:00:18.459 6416   #33 0x7f0378459155 _ZN4base8internal7InvokerINS0_9BindStateIMN4mojo9ConnectorEFvjEJNS0_17UnretainedWrapperIS4_EEEEEFvjEE7RunImplIRKS6_RKNSt3__15tupleIJS8_EEEJLm0EEEEvOT_OT0_NSF_16integer_sequenceImJXspT1_EEEEOj
09:00:18.459 6416   #34 0x7f037845907b _ZN4base8internal7InvokerINS0_9BindStateIMN4mojo9ConnectorEFvjEJNS0_17UnretainedWrapperIS4_EEEEEFvjEE3RunEPNS0_13BindStateBaseEj
09:00:18.459 6416   #35 0x7f037844f16e _ZNKR4base17RepeatingCallbackIFvjEE3RunEj
09:00:18.459 6416   #36 0x7f037845807f mojo::SimpleWatcher::DiscardReadyState()
09:00:18.459 6416   #37 0x7f03784582ef _ZN4base8internal13FunctorTraitsIPFvRKNS_17RepeatingCallbackIFvjEEEjRKN4mojo18HandleSignalsStateEEvE6InvokeIRKSC_JS6_jSA_EEEvOT_DpOT0_
09:00:18.459 6416   #38 0x7f037845828d _ZN4base8internal12InvokeHelperILb0EvE8MakeItSoIRKPFvRKNS_17RepeatingCallbackIFvjEEEjRKN4mojo18HandleSignalsStateEEJS8_jSC_EEEvOT_DpOT0_
09:00:18.459 6416   #39 0x7f0378458230 _ZN4base8internal7InvokerINS0_9BindStateIPFvRKNS_17RepeatingCallbackIFvjEEEjRKN4mojo18HandleSignalsStateEEJS5_EEEFvjSB_EE7RunImplIRKSD_RKNSt3__15tupleIJS5_EEEJLm0EEEEvOT_OT0_NSK_16integer_sequenceImJXspT1_EEEEOjSB_
09:00:18.459 6416   #40 0x7f0378458156 _ZN4base8internal7InvokerINS0_9BindStateIPFvRKNS_17RepeatingCallbackIFvjEEEjRKN4mojo18HandleSignalsStateEEJS5_EEEFvjSB_EE3RunEPNS0_13BindStateBaseEjSB_
09:00:18.460 6416   #41 0x7f0378514bfe _ZNKR4base17RepeatingCallbackIFvjRKN4mojo18HandleSignalsStateEEE3RunEjS4_
09:00:18.460 6416   #42 0x7f0378514559 mojo::SimpleWatcher::OnHandleReady()
09:00:18.460 6416   #43 0x7f03785153f3 _ZN4base8internal13FunctorTraitsIMN4mojo13SimpleWatcherEFvijRKNS2_18HandleSignalsStateEEvE6InvokeIS8_RKNS_7WeakPtrIS3_EEJRKiRKjS6_EEEvT_OT0_DpOT1_
09:00:18.460 6416   #44 0x7f0378515335 _ZN4base8internal12InvokeHelperILb1EvE8MakeItSoIRKMN4mojo13SimpleWatcherEFvijRKNS4_18HandleSignalsStateEERKNS_7WeakPtrIS5_EEJRKiRKjS8_EEEvOT_OT0_DpOT1_
09:00:18.460 6416   #45 0x7f0378515292 _ZN4base8internal7InvokerINS0_9BindStateIMN4mojo13SimpleWatcherEFvijRKNS3_18HandleSignalsStateEEJNS_7WeakPtrIS4_EEijS5_EEEFvvEE7RunImplIRKS9_RKNSt3__15tupleIJSB_ijS5_EEEJLm0ELm1ELm2ELm3EEEEvOT_OT0_NSI_16integer_sequenceImJXspT1_EEEE
09:00:18.460 6416   #46 0x7f03785150dc _ZN4base8internal7InvokerINS0_9BindStateIMN4mojo13SimpleWatcherEFvijRKNS3_18HandleSignalsStateEEJNS_7WeakPtrIS4_EEijS5_EEEFvvEE3RunEPNS0_13BindStateBaseE
09:00:18.460 6416   #47 0x7f0377f8042e _ZNO4base12OnceCallbackIFvvEE3RunEv
09:00:18.460 6416   #48 0x7f0377fd2462 base::debug::TaskAnnotator::RunTask()
09:00:18.460 6416   #49 0x7f03781b4ad3 base::sequence_manager::internal::ThreadControllerImpl::DoWork()
09:00:18.460 6416   #50 0x7f03781b79b1 _ZN4base8internal13FunctorTraitsIMNS_16sequence_manager8internal20ThreadControllerImplEFvNS4_8WorkTypeEEvE6InvokeIS7_RKNS_7WeakPtrIS4_EEJRKS5_EEEvT_OT0_DpOT1_
09:00:18.460 6416   #51 0x7f03781b7915 _ZN4base8internal12InvokeHelperILb1EvE8MakeItSoIRKMNS_16sequence_manager8internal20ThreadControllerImplEFvNS6_8WorkTypeEERKNS_7WeakPtrIS6_EEJRKS7_EEEvOT_OT0_DpOT1_
09:00:18.460 6416   #52 0x7f03781b788d _ZN4base8internal7InvokerINS0_9BindStateIMNS_16sequence_manager8internal20ThreadControllerImplEFvNS5_8WorkTypeEEJNS_7WeakPtrIS5_EES6_EEEFvvEE7RunImplIRKS8_RKNSt3__15tupleIJSA_S6_EEEJLm0ELm1EEEEvOT_OT0_NSH_16integer_sequenceImJXspT1_EEEE
09:00:18.460 6416   #53 0x7f03781b778c _ZN4base8internal7InvokerINS0_9BindStateIMNS_16sequence_manager8internal20ThreadControllerImplEFvNS5_8WorkTypeEEJNS_7WeakPtrIS5_EES6_EEEFvvEE3RunEPNS0_13BindStateBaseE
09:00:18.460 6416   #54 0x7f0377f8042e _ZNO4base12OnceCallbackIFvvEE3RunEv
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 9

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

commit 47fb8c4b3ec7c91acada51fced318b986ed4714b
Author: Ben Kelly <wanderview@chromium.org>
Date: Fri Nov 09 02:29:54 2018

Remove WebDataConsumerHandle usage from DataPipeAndDataBytesConsumer.

As a step towards removing WebDataConsumerHandle stop using the type in
DataPipeAndDataBytesConsumer.  Instead this code can instead use
DataPipeBytesConsumer to read the mojo::DataPipe.

Bug: 894819
Change-Id: I8b8cac639441a22b4498bc064f1b7bfd929b6acf
Reviewed-on: https://chromium-review.googlesource.com/c/1323789
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606712}
[modify] https://crrev.com/47fb8c4b3ec7c91acada51fced318b986ed4714b/third_party/blink/renderer/core/fetch/data_pipe_bytes_consumer.cc
[modify] https://crrev.com/47fb8c4b3ec7c91acada51fced318b986ed4714b/third_party/blink/renderer/core/fetch/form_data_bytes_consumer.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Nov 12

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

commit cecd4c6ce375e159210e7ede579bad832f5716ec
Author: Yutaka Hirano <yhirano@chromium.org>
Date: Mon Nov 12 10:27:29 2018

Introduce RequestPeer::GetTaskRunner

This is preliminary work for reading data pipes in RequestPeer
implementations.

Bug: 894819
Change-Id: I93ce399fa83796ab6fd095ea7dbf4666fb1458d4
Reviewed-on: https://chromium-review.googlesource.com/c/1312184
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607184}
[modify] https://crrev.com/cecd4c6ce375e159210e7ede579bad832f5716ec/chrome/renderer/extensions/extension_localization_peer.cc
[modify] https://crrev.com/cecd4c6ce375e159210e7ede579bad832f5716ec/chrome/renderer/extensions/extension_localization_peer.h
[modify] https://crrev.com/cecd4c6ce375e159210e7ede579bad832f5716ec/chrome/renderer/extensions/extension_localization_peer_unittest.cc
[modify] https://crrev.com/cecd4c6ce375e159210e7ede579bad832f5716ec/chrome/renderer/security_filter_peer.cc
[modify] https://crrev.com/cecd4c6ce375e159210e7ede579bad832f5716ec/chrome/renderer/security_filter_peer.h
[modify] https://crrev.com/cecd4c6ce375e159210e7ede579bad832f5716ec/content/public/renderer/request_peer.h
[modify] https://crrev.com/cecd4c6ce375e159210e7ede579bad832f5716ec/content/renderer/loader/resource_dispatcher_unittest.cc
[modify] https://crrev.com/cecd4c6ce375e159210e7ede579bad832f5716ec/content/renderer/loader/sync_load_context.cc
[modify] https://crrev.com/cecd4c6ce375e159210e7ede579bad832f5716ec/content/renderer/loader/sync_load_context.h
[modify] https://crrev.com/cecd4c6ce375e159210e7ede579bad832f5716ec/content/renderer/loader/test_request_peer.cc
[modify] https://crrev.com/cecd4c6ce375e159210e7ede579bad832f5716ec/content/renderer/loader/test_request_peer.h
[modify] https://crrev.com/cecd4c6ce375e159210e7ede579bad832f5716ec/content/renderer/loader/url_response_body_consumer_unittest.cc
[modify] https://crrev.com/cecd4c6ce375e159210e7ede579bad832f5716ec/content/renderer/loader/web_url_loader_impl.cc

Blockedon: 907793
Cc: wanderview@chromium.org
Owner: ----
Status: Available (was: Started)
I still would like to see this happen, but I've had some other priority work come up and I'm not actively working on this bug right now.
Cc: shimazu@chromium.org
Owner: yhirano@chromium.org
Status: Assigned (was: Available)
Blocking: 910487
Blockedon: 910489
Blocking: -910487
Blockedon: 910487
Blockedon: 911027
Blockedon: 911036
Blockedon: 920081
Blockedon: 920084
Blockedon: 922349

Comment 20 by shimazu@chromium.org, Yesterday (30 hours ago)

Blockedon: 923779

Sign in to add a comment