New issue
Advanced search Search tips

Issue 761117 link

Starred by 1 user
Status: Fixed
Owner:
jam@chromium.org
Closed: May 2018
Cc:
kinuko@chromium.org
mek@chromium.org
dmu...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug
Proj-Servicification

Blocking:
issue 598073



Sign in to add a comment

Handle POSTs of blob & filesystem with network service

Project Member Reported by jam@chromium.org, Aug 31 2017 
We need to handle form posts when one of the fields is a blob.

Since we're trying to make network service not aware of web platform, this means that renderer shouldn't send blob UUIDs through URLLoaderFactory. Instead, most likely it should send a data pipe containing the blob which it got from the blob service.

external/wpt/XMLHttpRequest/formdata-blob.htm [ Failure ]
external/wpt/XMLHttpRequest/formdata.htm [ Failure ]
external/wpt/XMLHttpRequest/overridemimetype-blob.html [ Failure ]
external/wpt/XMLHttpRequest/send-accept.htm [ Failure ]
external/wpt/XMLHttpRequest/send-blob-with-no-mime-type.html [ Failure ]
external/wpt/XMLHttpRequest/send-data-blob.htm [ Failure ]
http/tests/local/blob/send-data-blob.html [ Failure ]
http/tests/local/blob/send-hybrid-blob.html [ Failure ]
http/tests/local/blob/send-sliced-data-blob.html [ Failure ]
 crbug.com/761117  http/tests/xmlhttprequest/post-formdata.html [ Failure ]
 crbug.com/761117  http/tests/xmlhttprequest/workers/post-formdata.html [ Failure ]

Comment 1 by jam@chromium.org, Aug 31 2017

Description: Show this description
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 31 2017 

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

commit 077b0f850e1f737c66c2ab29cc736bd32c5aed6d
Author: John Abd-El-Malek <jam@chromium.org>
Date: Thu Aug 31 21:41:12 2017

Add some annotations to network service layout test expectations.

A bunch of failures are related to old Mojo JS APIs. Another group is because we don't handle blobs in POSTS.

BUG= 761117 , 758675 
TBR=reillyg@chromium.org
NOTRY=true

Change-Id: I131be4b793b3c91b1b735116cf7f86c34819dcb8
Reviewed-on: https://chromium-review.googlesource.com/646514
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499014}
[modify] https://crrev.com/077b0f850e1f737c66c2ab29cc736bd32c5aed6d/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService
Project Member

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

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

commit f4cec5503d203268fbd8857a8861e56847965a08
Author: John Abd-El-Malek <jam@chromium.org>
Date: Sat Sep 02 00:26:58 2017

Remove ParamTraits::GetSize since it's not used anymore.

The buffer now grows after r490340.

BUG= 742369 ,  761117 

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: If735bd46400b96aae9eff1a6cbd4748c73e6668b
Reviewed-on: https://chromium-review.googlesource.com/648249
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: enne <enne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499355}
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/android_webview/common/android_webview_message_generator.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/cc/ipc/cc_param_traits.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/cc/ipc/cc_param_traits.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/chrome/common/OWNERS
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/chrome/common/cast_messages.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/chrome/common/cast_messages.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/chrome/common/common_message_generator.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/chrome/common/common_param_traits.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/chrome/common/importer/profile_import_process_param_traits.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/chrome/common/safe_browsing/BUILD.gn
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/chrome/common/safe_browsing/ipc_protobuf_message_macros.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/chrome/common/safe_browsing/ipc_protobuf_message_unittest.cc
[delete] https://crrev.com/0c0a2d07aabf9576e6b925d6896fd984e01224bc/chrome/common/safe_browsing/protobuf_message_size_macros.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/components/autofill/content/common/autofill_message_generator.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/components/cdm/common/cdm_message_generator.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/components/guest_view/common/guest_view_message_generator.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/components/nacl/common/OWNERS
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/components/nacl/common/nacl_host_messages.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/components/nacl/common/nacl_messages.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/components/nacl/common/nacl_types_param_traits.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/components/network_hints/common/network_hints_message_generator.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/components/printing/common/print_messages.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/components/safe_browsing/common/safebrowsing_message_generator.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/components/spellcheck/common/spellcheck_message_generator.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/components/tracing/common/tracing_messages.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/content/common/content_message_generator.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/content/common/content_param_traits.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/content/common/content_param_traits.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/content/common/input/input_param_traits.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/content/common/input/input_param_traits.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/content/common/media/media_devices_param_traits.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/content/common/media/media_stream_param_traits.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/content/common/resource_messages.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/content/common/resource_messages.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/content/public/common/common_param_traits.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/content/public/common/common_param_traits.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/extensions/common/extension_message_generator.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/extensions/common/extension_messages.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/extensions/common/extension_messages.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/extensions/common/permissions/api_permission.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/extensions/common/permissions/api_permission.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/extensions/common/permissions/manifest_permission.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/extensions/common/permissions/manifest_permission.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/extensions/common/permissions/set_disjunction_permission.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/extensions/common/permissions/settings_override_permission.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/extensions/common/permissions/settings_override_permission.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/gpu/ipc/common/gpu_command_buffer_traits.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/gpu/ipc/common/gpu_command_buffer_traits.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/gpu/ipc/common/gpu_message_generator.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/gpu/ipc/common/gpu_param_traits.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/ipc/README.md
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/ipc/ipc_message_macros.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/ipc/ipc_message_protobuf_utils.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/ipc/ipc_message_protobuf_utils_unittest.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/ipc/ipc_message_utils.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/ipc/ipc_message_utils.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/ipc/ipc_message_utils_unittest.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/ipc/ipc_mojo_param_traits.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/ipc/ipc_mojo_param_traits.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/ipc/mach_port_mac.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/ipc/mach_port_mac.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/ipc/param_traits_macros.h
[delete] https://crrev.com/0c0a2d07aabf9576e6b925d6896fd984e01224bc/ipc/param_traits_size_macros.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/media/base/ipc/media_param_traits.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/media/base/ipc/media_param_traits.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/media/capture/ipc/capture_param_traits.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/media/capture/ipc/capture_param_traits.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/media/gpu/ipc/common/media_message_generator.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/media/gpu/ipc/common/media_param_traits.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/media/gpu/ipc/common/media_param_traits.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/mojo/public/cpp/bindings/lib/native_struct_serialization.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/mojo/public/cpp/bindings/tests/pickled_types_blink.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/mojo/public/cpp/bindings/tests/pickled_types_blink.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/mojo/public/cpp/bindings/tests/pickled_types_chromium.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/mojo/public/cpp/bindings/tests/pickled_types_chromium.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/ppapi/proxy/ppapi_messages.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/ppapi/proxy/ppapi_param_traits.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/ppapi/proxy/ppapi_param_traits.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/remoting/host/OWNERS
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/remoting/host/chromoting_messages.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/remoting/host/chromoting_param_traits.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/remoting/host/chromoting_param_traits.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/ui/gfx/ipc/color/gfx_param_traits.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/ui/gfx/ipc/color/gfx_param_traits.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/ui/gfx/ipc/geometry/gfx_param_traits.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/ui/gfx/ipc/geometry/gfx_param_traits.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/ui/gfx/ipc/gfx_param_traits.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/ui/gfx/ipc/gfx_param_traits.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/ui/gfx/ipc/skia/gfx_skia_param_traits.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/ui/gfx/ipc/skia/gfx_skia_param_traits.h
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/ui/latency/ipc/latency_info_param_traits.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/ui/latency/ipc/latency_info_param_traits.h
[add] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/url/ipc/OWNERS
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/url/ipc/url_param_traits.cc
[modify] https://crrev.com/f4cec5503d203268fbd8857a8861e56847965a08/url/ipc/url_param_traits.h
Project Member

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

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

commit 93e92e6e94948bf7613155d3d0ed1bdeb7c044c6
Author: John Abd-El-Malek <jam@chromium.org>
Date: Sat Sep 02 00:53:58 2017

Turn on the Mojo Blob code path when network service is enabled.

BUG=611935, 761117 

Change-Id: I63243ff26fe81667a47619f104627d4f5b5111c1
Reviewed-on: https://chromium-review.googlesource.com/646504
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Yuzhu Shen <yzshen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499363}
[modify] https://crrev.com/93e92e6e94948bf7613155d3d0ed1bdeb7c044c6/content/browser/blob_storage/blob_dispatcher_host.cc
[modify] https://crrev.com/93e92e6e94948bf7613155d3d0ed1bdeb7c044c6/content/browser/blob_storage/blob_registry_wrapper.cc
[modify] https://crrev.com/93e92e6e94948bf7613155d3d0ed1bdeb7c044c6/content/browser/cache_storage/cache_storage_cache.cc
[modify] https://crrev.com/93e92e6e94948bf7613155d3d0ed1bdeb7c044c6/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/93e92e6e94948bf7613155d3d0ed1bdeb7c044c6/content/browser/service_worker/service_worker_url_request_job.cc
[modify] https://crrev.com/93e92e6e94948bf7613155d3d0ed1bdeb7c044c6/content/browser/storage_partition_impl.cc
[modify] https://crrev.com/93e92e6e94948bf7613155d3d0ed1bdeb7c044c6/content/child/runtime_features.cc
[modify] https://crrev.com/93e92e6e94948bf7613155d3d0ed1bdeb7c044c6/content/public/common/content_features.cc
[modify] https://crrev.com/93e92e6e94948bf7613155d3d0ed1bdeb7c044c6/content/public/common/content_features.h
[modify] https://crrev.com/93e92e6e94948bf7613155d3d0ed1bdeb7c044c6/content/renderer/cache_storage/cache_storage_dispatcher.cc
[modify] https://crrev.com/93e92e6e94948bf7613155d3d0ed1bdeb7c044c6/content/renderer/service_worker/service_worker_context_client.cc
[modify] https://crrev.com/93e92e6e94948bf7613155d3d0ed1bdeb7c044c6/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 19 2017 

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

commit a8faeaa7ac737da8f1ecb3d89b64a7b2d4ceb09f
Author: John Abd-El-Malek <jam@chromium.org>
Date: Thu Oct 19 20:27:30 2017

Support blobs in POST body for the network service.

Since the network service doesn't know about blobs, the renderer sends a data pipe of the blob.

BUG=  761117 

Change-Id: I362a6afff33002c8f3758a19c9f0f0268c1c7f89
Reviewed-on: https://chromium-review.googlesource.com/721241
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510187}
[modify] https://crrev.com/a8faeaa7ac737da8f1ecb3d89b64a7b2d4ceb09f/content/browser/blob_storage/blob_dispatcher_host.cc
[modify] https://crrev.com/a8faeaa7ac737da8f1ecb3d89b64a7b2d4ceb09f/content/browser/loader/upload_data_stream_builder.cc
[modify] https://crrev.com/a8faeaa7ac737da8f1ecb3d89b64a7b2d4ceb09f/content/child/blob_storage/blob_transport_controller.cc
[modify] https://crrev.com/a8faeaa7ac737da8f1ecb3d89b64a7b2d4ceb09f/content/child/web_url_request_util.cc
[modify] https://crrev.com/a8faeaa7ac737da8f1ecb3d89b64a7b2d4ceb09f/content/common/resource_messages.cc
[modify] https://crrev.com/a8faeaa7ac737da8f1ecb3d89b64a7b2d4ceb09f/content/network/url_loader_impl.cc
[modify] https://crrev.com/a8faeaa7ac737da8f1ecb3d89b64a7b2d4ceb09f/content/public/common/BUILD.gn
[modify] https://crrev.com/a8faeaa7ac737da8f1ecb3d89b64a7b2d4ceb09f/content/public/common/DEPS
[modify] https://crrev.com/a8faeaa7ac737da8f1ecb3d89b64a7b2d4ceb09f/content/public/common/resource_request_body.cc
[modify] https://crrev.com/a8faeaa7ac737da8f1ecb3d89b64a7b2d4ceb09f/content/public/common/resource_request_body.h
[modify] https://crrev.com/a8faeaa7ac737da8f1ecb3d89b64a7b2d4ceb09f/storage/browser/blob/blob_data_builder.cc
[modify] https://crrev.com/a8faeaa7ac737da8f1ecb3d89b64a7b2d4ceb09f/storage/browser/blob/blob_reader.cc
[modify] https://crrev.com/a8faeaa7ac737da8f1ecb3d89b64a7b2d4ceb09f/storage/browser/blob/blob_storage_context.cc
[modify] https://crrev.com/a8faeaa7ac737da8f1ecb3d89b64a7b2d4ceb09f/storage/browser/blob/view_blob_internals_job.cc
[modify] https://crrev.com/a8faeaa7ac737da8f1ecb3d89b64a7b2d4ceb09f/storage/common/data_element.cc
[modify] https://crrev.com/a8faeaa7ac737da8f1ecb3d89b64a7b2d4ceb09f/storage/common/data_element.h
[modify] https://crrev.com/a8faeaa7ac737da8f1ecb3d89b64a7b2d4ceb09f/storage/public/interfaces/BUILD.gn
[add] https://crrev.com/a8faeaa7ac737da8f1ecb3d89b64a7b2d4ceb09f/storage/public/interfaces/size_getter.mojom
[modify] https://crrev.com/a8faeaa7ac737da8f1ecb3d89b64a7b2d4ceb09f/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService

Comment 7 by jam@chromium.org, Oct 19 2017

Status: Fixed (was: Started)

Comment 8 by jam@chromium.org, Oct 20 2017

Cc: kinuko@chromium.org
Status: Started (was: Fixed)
Summary: Handle POSTs with network service (was: Handle POSTs with blob data with network service)
I'm going to reopen this so it can be used for other upload scenarios:
-files
-filesystem API

Comment 9 by jam@chromium.org, Oct 20 2017 

Also: ensuring that we do security checks when uploading files.

Comment 10 by dougt@chromium.org, Oct 20 2017

Components: Internals>Network>Service

Comment 11 by jam@chromium.org, Oct 24 2017

Summary: Handle POSTs of blob & filesystem with network service (was: Handle POSTs with network service)

Comment 12 by hugefile...@gmail.com, Oct 25 2017 

Uploading a test file from Google Drive on Chrome OS, with crrev.com/c/736857 locally applied.
testfile.txt
1.4 MB View Download

Comment 13 by hugefile...@gmail.com, Oct 26 2017 

testing again
testfile1.txt
436 KB View Download
Project Member

Comment 14 by bugdroid1@chromium.org, Oct 27 2017 

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

commit 9fafd56ae8be4e62ba23b7f1f54d70f4385cf75c
Author: John Abd-El-Malek <jam@chromium.org>
Date: Fri Oct 27 21:09:17 2017

Remove code to handle reading filesystem URLs directly in uploads.

This isn't reachable anymore. Per hirono@, r183871 made blink::File's constructor for file system
URLs have has_backing_file=false. Both FormData.cpp and XMLHttpRequest.cpp in turn use
FormData::AppendBlob instead of FormData::AppendFileSystemURL.

BUG= 761117 

Change-Id: I07d258a5a5243e9912b160a8473fd4c1110ad8e1
Reviewed-on: https://chromium-review.googlesource.com/736857
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512283}
[modify] https://crrev.com/9fafd56ae8be4e62ba23b7f1f54d70f4385cf75c/content/browser/BUILD.gn
[delete] https://crrev.com/5097f5016f81285095ae16f13d451883f89a8481/content/browser/fileapi/upload_file_system_file_element_reader.cc
[delete] https://crrev.com/5097f5016f81285095ae16f13d451883f89a8481/content/browser/fileapi/upload_file_system_file_element_reader.h
[delete] https://crrev.com/5097f5016f81285095ae16f13d451883f89a8481/content/browser/fileapi/upload_file_system_file_element_reader_unittest.cc
[modify] https://crrev.com/9fafd56ae8be4e62ba23b7f1f54d70f4385cf75c/content/browser/loader/upload_data_stream_builder.cc
[modify] https://crrev.com/9fafd56ae8be4e62ba23b7f1f54d70f4385cf75c/content/network/url_loader_impl.cc
[modify] https://crrev.com/9fafd56ae8be4e62ba23b7f1f54d70f4385cf75c/content/test/BUILD.gn
[modify] https://crrev.com/9fafd56ae8be4e62ba23b7f1f54d70f4385cf75c/third_party/WebKit/Source/core/html/forms/FormData.cpp
[modify] https://crrev.com/9fafd56ae8be4e62ba23b7f1f54d70f4385cf75c/third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp
Project Member

Comment 15 by bugdroid1@chromium.org, Nov 1 2017 

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

commit bd4c113da890e51965bd03f53c4af2957e27d81c
Author: John Abd-El-Malek <jam@chromium.org>
Date: Wed Nov 01 15:47:30 2017

Remove more unused code related to filesystem API.

BUG= 761117 

Change-Id: I50bc0758b8e5afbba18f95b814b8341f0dcbc400
Reviewed-on: https://chromium-review.googlesource.com/742326
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513141}
[modify] https://crrev.com/bd4c113da890e51965bd03f53c4af2957e27d81c/content/renderer/loader/web_url_request_util.cc
[modify] https://crrev.com/bd4c113da890e51965bd03f53c4af2957e27d81c/third_party/WebKit/Source/modules/fetch/FormDataBytesConsumer.cpp
[modify] https://crrev.com/bd4c113da890e51965bd03f53c4af2957e27d81c/third_party/WebKit/Source/modules/fetch/FormDataBytesConsumerTest.cpp
[modify] https://crrev.com/bd4c113da890e51965bd03f53c4af2957e27d81c/third_party/WebKit/Source/platform/exported/WebHTTPBody.cpp
[modify] https://crrev.com/bd4c113da890e51965bd03f53c4af2957e27d81c/third_party/WebKit/Source/platform/network/EncodedFormData.cpp
[modify] https://crrev.com/bd4c113da890e51965bd03f53c4af2957e27d81c/third_party/WebKit/Source/platform/network/EncodedFormData.h
[modify] https://crrev.com/bd4c113da890e51965bd03f53c4af2957e27d81c/third_party/WebKit/Source/platform/network/EncodedFormDataTest.cpp
[modify] https://crrev.com/bd4c113da890e51965bd03f53c4af2957e27d81c/third_party/WebKit/public/platform/WebHTTPBody.h

Comment 16 by laforge@google.com, Nov 7 2017

Components: -Internals>Network>Service Internals>Services>Network
Apologies, applied the wrong component in bulk.
Project Member

Comment 17 by bugdroid1@chromium.org, Nov 29 2017 

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

commit c27a80d56a3850df748b73ea2d46b30c42ae8f4e
Author: Matt Falkenhagen <falken@chromium.org>
Date: Wed Nov 29 06:15:59 2017

NetworkService: Support re-sending the request body after redirects.

Some redirects like 307 require reissuing the request with the original request
body. This was crashing because the data pipe elements were already consumed.
The solution is to introduce DataPipeGetter backed by a Blob which allows
getting a data pipe multiple times.

Bug:  761117 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I7fbc09e626f853e1e078aecace5eca14d2788808
Reviewed-on: https://chromium-review.googlesource.com/784775
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520029}
[modify] https://crrev.com/c27a80d56a3850df748b73ea2d46b30c42ae8f4e/content/common/resource_messages.cc
[modify] https://crrev.com/c27a80d56a3850df748b73ea2d46b30c42ae8f4e/content/network/url_loader.cc
[modify] https://crrev.com/c27a80d56a3850df748b73ea2d46b30c42ae8f4e/content/network/url_loader_unittest.cc
[modify] https://crrev.com/c27a80d56a3850df748b73ea2d46b30c42ae8f4e/content/public/common/DEPS
[modify] https://crrev.com/c27a80d56a3850df748b73ea2d46b30c42ae8f4e/content/public/common/resource_request_body.cc
[modify] https://crrev.com/c27a80d56a3850df748b73ea2d46b30c42ae8f4e/content/public/common/resource_request_body.h
[modify] https://crrev.com/c27a80d56a3850df748b73ea2d46b30c42ae8f4e/content/renderer/loader/web_url_request_util.cc
[modify] https://crrev.com/c27a80d56a3850df748b73ea2d46b30c42ae8f4e/services/network/public/interfaces/BUILD.gn
[add] https://crrev.com/c27a80d56a3850df748b73ea2d46b30c42ae8f4e/services/network/public/interfaces/data_pipe_getter.mojom
[modify] https://crrev.com/c27a80d56a3850df748b73ea2d46b30c42ae8f4e/storage/DEPS
[modify] https://crrev.com/c27a80d56a3850df748b73ea2d46b30c42ae8f4e/storage/common/BUILD.gn
[modify] https://crrev.com/c27a80d56a3850df748b73ea2d46b30c42ae8f4e/storage/common/data_element.cc
[modify] https://crrev.com/c27a80d56a3850df748b73ea2d46b30c42ae8f4e/storage/common/data_element.h
[modify] https://crrev.com/c27a80d56a3850df748b73ea2d46b30c42ae8f4e/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService
[modify] https://crrev.com/c27a80d56a3850df748b73ea2d46b30c42ae8f4e/third_party/WebKit/LayoutTests/external/wpt/XMLHttpRequest/send-redirect-post-upload.htm
[modify] https://crrev.com/c27a80d56a3850df748b73ea2d46b30c42ae8f4e/third_party/WebKit/LayoutTests/external/wpt/fetch/api/redirect/redirect-method.js
[modify] https://crrev.com/c27a80d56a3850df748b73ea2d46b30c42ae8f4e/third_party/WebKit/common/BUILD.gn
[delete] https://crrev.com/838d0c04de665d5a41dc40513f8f4d9239e475e8/third_party/WebKit/common/blob/size_getter.mojom
Project Member

Comment 18 by bugdroid1@chromium.org, Mar 15 2018 

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

commit be8ffb1b1c877bf1ef29b8a0044cc13b81b130c5
Author: Marijn Kruisselbrink <mek@chromium.org>
Date: Thu Mar 15 17:12:22 2018

Don't crash if attempting to serialize TYPE_DATA_PIPE bodies.

Actually somehow serializing these is going to be more difficult, but
this will at least help to not crash for tests that hit this codepath.

On of several changes towards making uploading blobs with network service work.

Tested in a separate CL because multiple unrelated bugfixes are needed to make this
all work and testable (https://chromium-review.googlesource.com/c/chromium/src/+/963008).

(see also some of the discussion in https://groups.google.com/a/chromium.org/d/msg/network-service-dev/Jby2gkdDhi8/3GURbFqRAAAJ
where the problem of serializing uploaded blobs in history was mentioned)

Bug:  821878 ,  777879 ,  761117 
Change-Id: Id60791d26bc2f53c79290305cd25f7a95f247a85
Reviewed-on: https://chromium-review.googlesource.com/963015
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543413}
[modify] https://crrev.com/be8ffb1b1c877bf1ef29b8a0044cc13b81b130c5/content/common/page_state_serialization.cc
Project Member

Comment 19 by bugdroid1@chromium.org, Mar 16 2018 

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

commit 1214a9117eabec916388f6ff0ac95886d0b174c4
Author: Marijn Kruisselbrink <mek@chromium.org>
Date: Fri Mar 16 06:22:50 2018

Change DataElement to hold DataPipeGetterPtrInfo instead of Ptr.

Holding an InterfacePtr makes the struct weirdly sequence bound. Also
get rid of const casts that invalidate a const DataElement and replace
them with cloning the DataPipeGetter. This is needed because the same
ResourceRequestBody is sent to multiple mojo methods, and thus the
param traits should not modify the DataElements.

On of several changes towards making uploading blobs with network service work.

Tested in a separate CL because multiple unrelated bugfixes are needed to make this
all work and testable (https://chromium-review.googlesource.com/c/chromium/src/+/963008).

Bug:  821878 ,  761117 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Ic1d63e6f0f7112cf37c75526c561c524a9c15201
Reviewed-on: https://chromium-review.googlesource.com/963085
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543639}
[modify] https://crrev.com/1214a9117eabec916388f6ff0ac95886d0b174c4/content/common/service_worker/service_worker_loader_helpers.cc
[modify] https://crrev.com/1214a9117eabec916388f6ff0ac95886d0b174c4/content/renderer/loader/web_url_request_util.cc
[modify] https://crrev.com/1214a9117eabec916388f6ff0ac95886d0b174c4/content/renderer/service_worker/service_worker_subresource_loader_unittest.cc
[modify] https://crrev.com/1214a9117eabec916388f6ff0ac95886d0b174c4/services/network/public/cpp/data_element.cc
[modify] https://crrev.com/1214a9117eabec916388f6ff0ac95886d0b174c4/services/network/public/cpp/data_element.h
[modify] https://crrev.com/1214a9117eabec916388f6ff0ac95886d0b174c4/services/network/public/cpp/network_param_ipc_traits.cc
[modify] https://crrev.com/1214a9117eabec916388f6ff0ac95886d0b174c4/services/network/public/cpp/simple_url_loader_unittest.cc
[modify] https://crrev.com/1214a9117eabec916388f6ff0ac95886d0b174c4/services/network/url_loader.cc
Project Member

Comment 20 by bugdroid1@chromium.org, Mar 16 2018 

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

commit d3e3a388e54f0163a0c02838b0028f491142f797
Author: Marijn Kruisselbrink <mek@chromium.org>
Date: Fri Mar 16 20:28:36 2018

[Blobs] Add AsDataPipeGetter method to Blob mojo interface.

This makes it simpler to create a DataPipeGetter out of a Blob, and also
ensures that a blob that is used as the body of a request can still be
read even after the process that created the DataPipeGetter (i.e. the
renderer that initiated the fetch) dies.

Bug:  821878 ,  761117 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I519a6061900f4cb29b786b8445dc14f034959e27
Reviewed-on: https://chromium-review.googlesource.com/963088
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543807}
[modify] https://crrev.com/d3e3a388e54f0163a0c02838b0028f491142f797/content/renderer/loader/web_url_request_util.cc
[modify] https://crrev.com/d3e3a388e54f0163a0c02838b0028f491142f797/storage/browser/blob/blob_impl.cc
[modify] https://crrev.com/d3e3a388e54f0163a0c02838b0028f491142f797/storage/browser/blob/blob_impl.h
[modify] https://crrev.com/d3e3a388e54f0163a0c02838b0028f491142f797/storage/browser/blob/blob_registry_impl_unittest.cc
[modify] https://crrev.com/d3e3a388e54f0163a0c02838b0028f491142f797/third_party/WebKit/Source/platform/blob/testing/FakeBlob.cpp
[modify] https://crrev.com/d3e3a388e54f0163a0c02838b0028f491142f797/third_party/WebKit/Source/platform/blob/testing/FakeBlob.h
[modify] https://crrev.com/d3e3a388e54f0163a0c02838b0028f491142f797/third_party/WebKit/public/mojom/BUILD.gn
[modify] https://crrev.com/d3e3a388e54f0163a0c02838b0028f491142f797/third_party/WebKit/public/mojom/blob/blob.mojom

Comment 21 by jam@chromium.org, May 22 2018

Status: Fixed (was: Started)
This is fixed, other than the security check which I mentioned in comment 9. I've filed  bug 845612  to track that one.

Comment 22 by mek@chromium.org, May 22 2018 

There are other things still missing too, such as the NOTIMPLEMENTED() in https://cs.chromium.org/chromium/src/content/common/page_state_serialization.cc?type=cs&sq=package:chromium&g=0&l=710

That one is particularly tricky since currently history state of a tab is entirely serialized as a string, but with datapipe getters instead of blobs that isn't possible, so back/forward navigations and/or reloads don't correctly keep their blob body if there is one...

Comment 23 by kinuko@chromium.org, May 23 2018 

#22- I actually am not really sure how the legacy code could have handled blobs well in the page state serialization code. We do serialize blob uuids yes but do we retain the blob body somewhere?

Comment 24 by mek@chromium.org, May 23 2018 

Good point. The old code was somewhat broken too, so maybe this is okay? Although there still seem to be a lot of "the tests are passing, so everything must be okay" going on here, rather than actual reasoning/investigating over what functionality is supposed to work/used to work, and making sure all that still works. We seem to be putting a rather large amount of faith in the quality of our tests (and even if the tests used to have 100% code coverage and were perfect, with how drastically different the network service implementation is that says absolute nothing about how much of the various permutations of functionality is still tested with network service enabled... But that's more a general concern I have not specific to this bug).

Comment 25 by jam@chromium.org, May 23 2018 

@Marijn: if you know of any failures, even if not covered by tests, that only occur with network service enabled please reopen this bug (or any other). We (core network service team) depend on tests or teams to tell us when something is broken; we don't have enough knowledge of all the features to do otherwise :)
►

Sign in to add a comment