Remove the stream concept from loading code |
|||||||||||||||||||
Issue descriptionPart of network service related changes. https://cs.chromium.org/chromium/src/content/browser/loader/resource_dispatcher_host_impl.cc?q=resource_dispatcher_host+package:%5Echromium$&l=527 We are not tackling the blocking call to the ResourceDispatcherHostDelegate::ShouldInterceptAsStream() function in this bug. That function looks at registered extensions/plugins for the mime type and returns true or false accordingly. Longer term we could register a cache of mime types which the delegate is interested in with RDHI which would be updated as required. Assuming that the delegate/caller is interested in handling the mime type via a stream, possible approaches include the following: 1. Implement a ProxyStreamResourceHandler class which implements the ResourceHandler interface and calls out to a delegate class (ProxyStreamResourceDelegate?). Alternatively we could extend the ResourceDispatcherHostDelegate interface to add support for the Stream callbacks like OnWillRead/OnReadCompleted/OnResponseCompleted, etc. 2. Change StreamResourceHandler to hold a ProxyStreamWriter instance which in turn calls out to a delegate (ProxyStreamWriterDelegate or an augmented ResourceDispatcherHostDelegate interface) described in portions of point 1 above.
,
Mar 25 2017
While the current code path can't be removed completely until MojoAsyncResourceHandler ships, I think we can get things working like that when MojoAsyncResourceHandler is enabled, so I don't think we need to block progress here on shipping that.
,
Mar 27 2017
I don't think we want renderers to be involved in this as we'd also have to mirror this mapping across processes. It's not clear to me that using streams for this is something we'll stick on long terms, I think we should take a closer look at mojo's data pipe. This seems similar to other cases where parts of content provide data (i.e. appcache, SW). Matt: re creating new pipes, transferring existing mojo pipes is pretty light-weight. We don't need to create new ones for the case where the network process delegates a request to a different component.
,
Mar 27 2017
Note that this isn't delegating a request to a different component, but redirecting a response to a different component (Which doesn't seem like something the network process should be involved in - I'm also a bit horrified by the ResourceHandler swapping logic, and we've run into a number of longstanding bugs in the loader/ code to do that, so I'd really like to see that logic just go away)
,
Mar 27 2017
,
Mar 28 2017
Conceptually, I think it makes sense to keep the network service as close as possible to "Turns a URL into a mojo pipe with the data from the URL." I don't like the idea of the network process as being aware or in control of where the destination of that pipe is. John: I'm not clear why having a mapping for streams across all renderers is a problem (isn't that mapping pretty constant after it's setup?), but if it is, why not then just have the renderer ship the mojo pipe to some central location for dispatch?
,
Mar 28 2017
Randy: I think there is a question of trust - can we trust a Mojo pipe shipped from the renderer process? It could attach an arbitrary bogus URL and data. I'm not entirely sure where we're shipping the pipe to, though, so unclear if that's an issue.
,
Mar 28 2017
I tend to equivalent the server and the renderer in terms of security domains--if the server can create an arbitrary stream, I don't worry if the renderer can substitute one. Arbitrary bogus URL may be a problem, though I'd like to understand the use case better before giving up on the "Network service provides data in response to a URL" high level model.
,
Mar 28 2017
Note that lying about origin can be a big deal, in some contexts. Of course, if the request can go through the renderer's ServiceWorker, that allows lying about origin, anyways.
,
May 24 2017
,
Jul 17 2017
Updating this bug after various discussions. It seems that once we have data pipe for loading (for network service and old code path, which Camille is working on in bug 705744 ), we can get rid of the streams concept. We should be able to pass data pipe to pdf plugin instead of a stream. Then we can simplify a bunch of loading code by removing this concept.
,
Oct 18 2017
,
Nov 7 2017
Apologies, applied the wrong component in bulk.
,
Nov 13 2017
,
Nov 13 2017
,
Dec 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bce8efc63e90550bdb572fb35532cda52b7a371d commit bce8efc63e90550bdb572fb35532cda52b7a371d Author: Reilly Grant <reillyg@chromium.org> Date: Mon Dec 11 22:33:45 2017 Disable PDFExtensionTest.Metrics test with NetworkService This new tests fails with the Network Service enabled just as other PDFExtensionTests do. Lumping this in with issue 705114. Bug: 705114 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo Change-Id: I6da8f21354e7f809e8b86a3f57b3f6723e57e386 Tbr: jam@chromium.org No-Try: true No-Tree-Checks: true No-Presubmit: true Reviewed-on: https://chromium-review.googlesource.com/820810 Reviewed-by: Reilly Grant <reillyg@chromium.org> Commit-Queue: Reilly Grant <reillyg@chromium.org> Cr-Commit-Position: refs/heads/master@{#523236} [modify] https://crrev.com/bce8efc63e90550bdb572fb35532cda52b7a371d/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
,
Feb 9 2018
,
Feb 15 2018
Sorry I won't be able to continue working on this. Please see section 1 "PDF loading with the network service" in this document for a description of status and possible solutions for future work: https://docs.google.com/document/d/1-q04PweVJEDBeSX1nIRPlF9277Vcc0jqtTPI_pPlonw/edit?usp=sharing (@chromium.org account required to view and comment.)
,
Feb 15 2018
Thank you Yuzhu for getting this mostly working! per the tuesday meeting, since there were no volunteers I can take a look at this.
,
Feb 16 2018
,
Feb 28 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2b1f06caabcb58b556249d833c1daf91afbb96a0 commit 2b1f06caabcb58b556249d833c1daf91afbb96a0 Author: John Abd-El-Malek <jam@chromium.org> Date: Wed Feb 28 06:17:38 2018 Simplify ChromeResourceDispatcherHostDelegate::ShouldInterceptResourceAsStream to remove unneeded code path. When writing the equivalient network service code path (cl 910315), Yuzhu found we didn't need the path that's taken when a plugin path is provided. The code also always calls StreamsPrivateAPI when this method returns true. I've tested with the PDF plugin and it works with these changes. The docs extension doesn't seem to work in Chromium builds with and without this change. Bug: 705114 Change-Id: I37434b506326e4ad6a3a07e5e3c8653be3489f73 Reviewed-on: https://chromium-review.googlesource.com/939857 Commit-Queue: John Abd-El-Malek <jam@chromium.org> Reviewed-by: Raymes Khoury <raymes@chromium.org> Cr-Commit-Position: refs/heads/master@{#539730} [modify] https://crrev.com/2b1f06caabcb58b556249d833c1daf91afbb96a0/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc [modify] https://crrev.com/2b1f06caabcb58b556249d833c1daf91afbb96a0/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.h [modify] https://crrev.com/2b1f06caabcb58b556249d833c1daf91afbb96a0/chrome/browser/profiles/off_the_record_profile_io_data.cc [modify] https://crrev.com/2b1f06caabcb58b556249d833c1daf91afbb96a0/chrome/browser/profiles/profile_impl_io_data.cc [modify] https://crrev.com/2b1f06caabcb58b556249d833c1daf91afbb96a0/chrome/browser/profiles/profile_io_data.cc [modify] https://crrev.com/2b1f06caabcb58b556249d833c1daf91afbb96a0/chrome/browser/profiles/profile_io_data.h [modify] https://crrev.com/2b1f06caabcb58b556249d833c1daf91afbb96a0/content/browser/loader/mime_sniffing_resource_handler.cc [modify] https://crrev.com/2b1f06caabcb58b556249d833c1daf91afbb96a0/content/browser/loader/mime_sniffing_resource_handler_unittest.cc [modify] https://crrev.com/2b1f06caabcb58b556249d833c1daf91afbb96a0/content/browser/loader/mojo_async_resource_handler_unittest.cc [modify] https://crrev.com/2b1f06caabcb58b556249d833c1daf91afbb96a0/content/browser/loader/resource_dispatcher_host_impl.cc [modify] https://crrev.com/2b1f06caabcb58b556249d833c1daf91afbb96a0/content/browser/loader/resource_dispatcher_host_impl.h [modify] https://crrev.com/2b1f06caabcb58b556249d833c1daf91afbb96a0/content/public/browser/resource_dispatcher_host_delegate.cc [modify] https://crrev.com/2b1f06caabcb58b556249d833c1daf91afbb96a0/content/public/browser/resource_dispatcher_host_delegate.h
,
Mar 1 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bae8a7430f4e2636d46c0fe1086a2cc819432ad0 commit bae8a7430f4e2636d46c0fe1086a2cc819432ad0 Author: Chris Mumford <cmumford@chromium.org> Date: Thu Mar 01 23:02:23 2018 Loading chrome scheme non-network resources. Bug: 705114 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo Change-Id: I3f092caca160715854324acf58590c18e65a7281 Reviewed-on: https://chromium-review.googlesource.com/922940 Commit-Queue: Chris Mumford <cmumford@chromium.org> Reviewed-by: John Abd-El-Malek <jam@chromium.org> Cr-Commit-Position: refs/heads/master@{#540327} [modify] https://crrev.com/bae8a7430f4e2636d46c0fe1086a2cc819432ad0/chrome/browser/chrome_content_browser_client.cc [modify] https://crrev.com/bae8a7430f4e2636d46c0fe1086a2cc819432ad0/chrome/browser/extensions/chrome_extension_web_contents_observer.cc [modify] https://crrev.com/bae8a7430f4e2636d46c0fe1086a2cc819432ad0/content/browser/BUILD.gn [modify] https://crrev.com/bae8a7430f4e2636d46c0fe1086a2cc819432ad0/content/browser/bad_message.h [modify] https://crrev.com/bae8a7430f4e2636d46c0fe1086a2cc819432ad0/content/browser/frame_host/render_frame_host_impl.cc [modify] https://crrev.com/bae8a7430f4e2636d46c0fe1086a2cc819432ad0/content/browser/loader/navigation_url_loader_network_service.cc [modify] https://crrev.com/bae8a7430f4e2636d46c0fe1086a2cc819432ad0/content/browser/webui/web_ui_url_loader_factory.cc [rename] https://crrev.com/bae8a7430f4e2636d46c0fe1086a2cc819432ad0/content/browser/webui/web_ui_url_loader_factory_internal.h [modify] https://crrev.com/bae8a7430f4e2636d46c0fe1086a2cc819432ad0/content/public/browser/BUILD.gn [add] https://crrev.com/bae8a7430f4e2636d46c0fe1086a2cc819432ad0/content/public/browser/web_ui_url_loader_factory.h [modify] https://crrev.com/bae8a7430f4e2636d46c0fe1086a2cc819432ad0/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter [modify] https://crrev.com/bae8a7430f4e2636d46c0fe1086a2cc819432ad0/tools/metrics/histograms/enums.xml
,
Mar 2 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d commit ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d Author: John Abd-El-Malek <jam@chromium.org> Date: Fri Mar 02 18:47:39 2018 Support PDF frame request loading with network service. The general idea is that we remove the "stream" concept from the code. Instead we pass the data pipe for a plugin resource to the renderer that is serving the pepper plugin. When the browser sees a response for a mime type that's handled by an extension, it will intercept the response using URLLoaderThrottle::InterceptResponse(). The response's URLResponseHead struct is saved, along with the URLLoader interface pointer. They are then sent to the render frame at commit time in the TransferrableURLLoader structure along with a dummy URL to reference them. When the renderer sees a request for that URL, ChildURLLoaderFactoryBundle will use the given URLLoader and response info instead of a new URLLoader. StreamsPrivateAPI is extended to accept a TransferrableURLLoader instead a stream when the network service is used. ChildURLLoaderFactoryBundle is also taught to receive and hold on to this structure. This is based on Yuzhu's https://chromium-review.googlesource.com/c/chromium/src/+/910315 More details of how this works: https://docs.google.com/document/d/1FUuS7fmpTmpaBiT4wgXBPpeFUP9nRBpRouSF5qsdv5I Bug: 705114 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo Change-Id: I7b0c1b78121332a2a8b9a73b2a4de353a75a7b32 Reviewed-on: https://chromium-review.googlesource.com/920142 Commit-Queue: John Abd-El-Malek <jam@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org> Reviewed-by: Ken Rockot <rockot@chromium.org> Cr-Commit-Position: refs/heads/master@{#540567} [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/chrome/browser/BUILD.gn [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/chrome/browser/chrome_content_browser_client.cc [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/chrome/browser/extensions/api/streams_private/streams_private_api.cc [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/chrome/browser/extensions/api/streams_private/streams_private_api.h [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc [add] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/chrome/browser/plugins/plugin_response_interceptor_url_loader_throttle.cc [add] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/chrome/browser/plugins/plugin_response_interceptor_url_loader_throttle.h [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/chrome/browser/plugins/plugin_utils.cc [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/chrome/browser/plugins/plugin_utils.h [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/content/browser/frame_host/navigation_handle_impl.cc [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/content/browser/frame_host/navigation_handle_impl.h [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/content/browser/frame_host/navigation_request.cc [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/content/browser/frame_host/navigation_request.h [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/content/browser/frame_host/render_frame_host_impl.cc [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/content/browser/frame_host/render_frame_host_impl.h [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/content/browser/loader/navigation_url_loader_network_service.cc [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/content/common/frame.mojom [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/content/common/throttling_url_loader.cc [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/content/common/throttling_url_loader.h [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/content/public/browser/navigation_handle.h [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/content/public/common/BUILD.gn [add] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/content/public/common/transferrable_url_loader.mojom [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/content/public/common/url_loader_throttle.cc [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/content/public/common/url_loader_throttle.h [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/content/renderer/loader/child_url_loader_factory_bundle.cc [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/content/renderer/loader/child_url_loader_factory_bundle.h [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/content/renderer/loader/tracked_child_url_loader_factory_bundle.cc [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/content/renderer/render_frame_impl.cc [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/content/renderer/render_frame_impl.h [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/content/test/test_render_frame.cc [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/content/test/test_render_frame_host.cc [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/extensions/browser/api/mime_handler_private/mime_handler_private.cc [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/extensions/browser/api/mime_handler_private/mime_handler_private_unittest.cc [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.cc [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.h [modify] https://crrev.com/ecc6f5febb9c6e1f66ef38781f0a23239adf0c0d/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
,
Mar 5 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e6ee6e943937c6110d11ebb0760e264d6df8cbcd commit e6ee6e943937c6110d11ebb0760e264d6df8cbcd Author: John Abd-El-Malek <jam@chromium.org> Date: Mon Mar 05 20:31:09 2018 Fix loading of PDFs in embedded or object tags with the network service. Since the resource load is initiated by the renderer and goes directly to the network process directly, it has to be intercepted in the renderer. The renderer then sends the intercepted URLLoader to the browser where it then continues with the same path as the frame case. More details of how this works: https://docs.google.com/document/d/1FUuS7fmpTmpaBiT4wgXBPpeFUP9nRBpRouSF5qsdv5I Bug: 705114 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo Change-Id: I1d19c9631584dbf56c653d589ba33d43f2ccb375 Reviewed-on: https://chromium-review.googlesource.com/948264 Commit-Queue: John Abd-El-Malek <jam@chromium.org> Reviewed-by: Ken Rockot <rockot@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org> Cr-Commit-Position: refs/heads/master@{#540918} [modify] https://crrev.com/e6ee6e943937c6110d11ebb0760e264d6df8cbcd/chrome/renderer/chrome_content_renderer_client.cc [modify] https://crrev.com/e6ee6e943937c6110d11ebb0760e264d6df8cbcd/chrome/renderer/chrome_content_renderer_client.h [modify] https://crrev.com/e6ee6e943937c6110d11ebb0760e264d6df8cbcd/chrome/renderer/extensions/chrome_extensions_renderer_client.cc [modify] https://crrev.com/e6ee6e943937c6110d11ebb0760e264d6df8cbcd/chrome/renderer/extensions/chrome_extensions_renderer_client.h [modify] https://crrev.com/e6ee6e943937c6110d11ebb0760e264d6df8cbcd/chrome/renderer/url_loader_throttle_provider_impl.cc [modify] https://crrev.com/e6ee6e943937c6110d11ebb0760e264d6df8cbcd/chromecast/renderer/cast_content_renderer_client.cc [modify] https://crrev.com/e6ee6e943937c6110d11ebb0760e264d6df8cbcd/chromecast/renderer/cast_content_renderer_client.h [modify] https://crrev.com/e6ee6e943937c6110d11ebb0760e264d6df8cbcd/content/public/common/BUILD.gn [modify] https://crrev.com/e6ee6e943937c6110d11ebb0760e264d6df8cbcd/content/public/renderer/content_renderer_client.cc [modify] https://crrev.com/e6ee6e943937c6110d11ebb0760e264d6df8cbcd/content/public/renderer/content_renderer_client.h [modify] https://crrev.com/e6ee6e943937c6110d11ebb0760e264d6df8cbcd/content/renderer/render_frame_impl.cc [modify] https://crrev.com/e6ee6e943937c6110d11ebb0760e264d6df8cbcd/extensions/browser/guest_view/extensions_guest_view_message_filter.cc [modify] https://crrev.com/e6ee6e943937c6110d11ebb0760e264d6df8cbcd/extensions/browser/guest_view/extensions_guest_view_message_filter.h [modify] https://crrev.com/e6ee6e943937c6110d11ebb0760e264d6df8cbcd/extensions/common/BUILD.gn [add] https://crrev.com/e6ee6e943937c6110d11ebb0760e264d6df8cbcd/extensions/common/mojo/guest_view.mojom [modify] https://crrev.com/e6ee6e943937c6110d11ebb0760e264d6df8cbcd/extensions/renderer/DEPS [modify] https://crrev.com/e6ee6e943937c6110d11ebb0760e264d6df8cbcd/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container.cc [modify] https://crrev.com/e6ee6e943937c6110d11ebb0760e264d6df8cbcd/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container.h [modify] https://crrev.com/e6ee6e943937c6110d11ebb0760e264d6df8cbcd/extensions/shell/renderer/shell_content_renderer_client.cc [modify] https://crrev.com/e6ee6e943937c6110d11ebb0760e264d6df8cbcd/extensions/shell/renderer/shell_content_renderer_client.h [modify] https://crrev.com/e6ee6e943937c6110d11ebb0760e264d6df8cbcd/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
,
Mar 6 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/75340bc994d89b80de3170c706b48b4eb636c4c0 commit 75340bc994d89b80de3170c706b48b4eb636c4c0 Author: John Abd-El-Malek <jam@chromium.org> Date: Tue Mar 06 14:54:01 2018 Fix MimeHandlerViewTests/MimeHandlerViewTest*Data tests with network service. The test expectation was expecting a 200 respons code for a data url. This was worked around for PlzNavigate in https://chromium-review.googlesource.com/c/chromium/src/+/744290/4..5. I'm fixing the tests instead so that it works with the network service, and we don't have to add fake headers in production code. Bug: 705114 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo Change-Id: Ib1ece1ac1ff95edf5bd9f1a216e241895d4dc4b4 Reviewed-on: https://chromium-review.googlesource.com/949883 Commit-Queue: John Abd-El-Malek <jam@chromium.org> Reviewed-by: Raymes Khoury <raymes@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Reviewed-by: Andrey Kosyakov <caseq@chromium.org> Cr-Commit-Position: refs/heads/master@{#541102} [modify] https://crrev.com/75340bc994d89b80de3170c706b48b4eb636c4c0/chrome/test/data/extensions/api_test/mime_handler_view/index.js [modify] https://crrev.com/75340bc994d89b80de3170c706b48b4eb636c4c0/content/browser/streams/stream.cc [modify] https://crrev.com/75340bc994d89b80de3170c706b48b4eb636c4c0/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
,
Mar 7 2018
,
Apr 16 2018
Issue 764474 has been merged into this issue.
,
Apr 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c590f236f50c0b2664bda5d882a10b406af2671c commit c590f236f50c0b2664bda5d882a10b406af2671c Author: John Abd-El-Malek <jam@chromium.org> Date: Tue Apr 17 00:15:50 2018 Don't run streams tests with network service. PDF and QuickOffice plugins use the mimeHandler API only. Once QuickOffice that uses mimeHandler is updated to all users (depends on 67 reaching stable) we can delete the streams API code. Bug: 705114 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo Change-Id: Ia402364e4ee88e9f3eb06b3e413253ffb5e71413 Reviewed-on: https://chromium-review.googlesource.com/1013403 Reviewed-by: Sam McNally <sammc@chromium.org> Commit-Queue: John Abd-El-Malek <jam@chromium.org> Cr-Commit-Position: refs/heads/master@{#551178} [modify] https://crrev.com/c590f236f50c0b2664bda5d882a10b406af2671c/chrome/browser/extensions/api/streams_private/streams_private_apitest.cc [modify] https://crrev.com/c590f236f50c0b2664bda5d882a10b406af2671c/chrome/browser/prerender/prerender_browsertest.cc [modify] https://crrev.com/c590f236f50c0b2664bda5d882a10b406af2671c/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
,
Apr 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c590f236f50c0b2664bda5d882a10b406af2671c commit c590f236f50c0b2664bda5d882a10b406af2671c Author: John Abd-El-Malek <jam@chromium.org> Date: Tue Apr 17 00:15:50 2018 Don't run streams tests with network service. PDF and QuickOffice plugins use the mimeHandler API only. Once QuickOffice that uses mimeHandler is updated to all users (depends on 67 reaching stable) we can delete the streams API code. Bug: 705114 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo Change-Id: Ia402364e4ee88e9f3eb06b3e413253ffb5e71413 Reviewed-on: https://chromium-review.googlesource.com/1013403 Reviewed-by: Sam McNally <sammc@chromium.org> Commit-Queue: John Abd-El-Malek <jam@chromium.org> Cr-Commit-Position: refs/heads/master@{#551178} [modify] https://crrev.com/c590f236f50c0b2664bda5d882a10b406af2671c/chrome/browser/extensions/api/streams_private/streams_private_apitest.cc [modify] https://crrev.com/c590f236f50c0b2664bda5d882a10b406af2671c/chrome/browser/prerender/prerender_browsertest.cc [modify] https://crrev.com/c590f236f50c0b2664bda5d882a10b406af2671c/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
,
May 7 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/094121feed942459ec1eddd8fb7d6201b09c457a commit 094121feed942459ec1eddd8fb7d6201b09c457a Author: John Abd-El-Malek <jam@chromium.org> Date: Mon May 07 21:56:01 2018 Fix MimeHandlerViewTests/MimeHandlerViewTest.SingleRequest with network service. Don't use a net::URLRequestInterceptor to count the number of times a resource was loaded, as that doesn't work when net/ runs in a separate process. Instead count the number of times the test server saw the request. Bug: 705114 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo Change-Id: Ie042e56ee192774c51bb8d1b568a569ca90b4e42 Reviewed-on: https://chromium-review.googlesource.com/1047788 Reviewed-by: Doug Turner <dougt@chromium.org> Commit-Queue: John Abd-El-Malek <jam@chromium.org> Cr-Commit-Position: refs/heads/master@{#556580} [modify] https://crrev.com/094121feed942459ec1eddd8fb7d6201b09c457a/extensions/browser/guest_view/mime_handler_view/mime_handler_view_browsertest.cc [modify] https://crrev.com/094121feed942459ec1eddd8fb7d6201b09c457a/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
,
May 22 2018
,
May 25 2018
,
Jun 7 2018
I'm going to remove the Proj-Servicification-Canary label, since this works already with the network service. Once the updated quickoffice plugin is shipped to all users (tracked in bug 819761 ), I'll delete the old streams code. This won't block network service canary though.
,
Jun 7 2018
,
Jun 18 2018
Looking into this a bit deeper from the blob URL side of things: I was under the impression that this bug was about "removing the stream concept from loading code", but looking at the implementation of the mimeHandler API it still seems to rely heavily on all of the streams stuff in the loading code, at least if the network service isn't enabled? So is this actually about removing the stream concept like the bug summary suggests, or is this merely about making things work with network service? If the latter, how hard would it be to use the "network service" code path of this even in the absence of the network service?
,
Jun 19 2018
Hrm...This still seems to depend on ProfileIOData, which we shouldn't even be creating when the network service is enabled. See https://cs.chromium.org/chromium/src/chrome/browser/plugins/plugin_response_interceptor_url_loader_throttle.cc?type=cs&q=PluginUtils::GetExtensionIdForMimeType&g=0&l=32 / https://cs.chromium.org/chromium/src/chrome/browser/plugins/plugin_utils.cc?type=cs&g=0&l=153
,
Jul 24
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0bfebe75d4476a76ed96777633cca2d2317aa355 commit 0bfebe75d4476a76ed96777633cca2d2317aa355 Author: John Abd-El-Malek <jam@chromium.org> Date: Tue Jul 24 19:32:33 2018 Fix progressive loading of PDFs with network service. The problem was different timing of loading messages. Normally, DocumentLoaderImpl::DidRead is called very early which calls into DocumentLoaderImpl::SaveBuffer. The latter sees that the request was in |pending_requests_| and then calls OnPendingRequestComplete which loads the information about the document (e.g. number of pages, linearized PDF info). With the network service, OutOfProcessInstance::DidChangeView was being called before the DidRead calls which was causing |pending_requests_| to be reset. The fix is to only reset pending_requests_ in PDFiumEngine::CalculateVisiblePages if we have already loaded the PDF document structure. This was the intended effect of that code; the extra check wasn't needed before the network service because we were always reaching that function after the document info was loaded. Bug: 705114 Change-Id: I4e668ebfdda0c17ede3c6daa951dea0a1b38eb05 Reviewed-on: https://chromium-review.googlesource.com/1147932 Reviewed-by: Lei Zhang <thestig@chromium.org> Commit-Queue: John Abd-El-Malek <jam@chromium.org> Cr-Commit-Position: refs/heads/master@{#577637} [modify] https://crrev.com/0bfebe75d4476a76ed96777633cca2d2317aa355/pdf/pdfium/pdfium_engine.cc
,
Jul 25
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b5706a3ad7605d299a1e2dbd777f72d9add7d009 commit b5706a3ad7605d299a1e2dbd777f72d9add7d009 Author: John Abd-El-Malek <jam@chromium.org> Date: Wed Jul 25 00:13:40 2018 Fix crash when mimeHandlerPrivate.abortStream is called with the network service enabled. This call isn't needed with the network service, since we pass the TransferrableURLLoader Mojo struct instead of stream URLs. When the PDF closes the main document load, the renderer will reset its Mojo pipe which will tell the network service to close the connection. Bug: 705114, 856161 Cq-Include-Trybots: luci.chromium.try:linux_mojo Change-Id: I1c87753f7820f6b1ce80bf06ace5aab899d1b903 Reviewed-on: https://chromium-review.googlesource.com/1148590 Reviewed-by: Sam McNally <sammc@chromium.org> Commit-Queue: John Abd-El-Malek <jam@chromium.org> Cr-Commit-Position: refs/heads/master@{#577739} [modify] https://crrev.com/b5706a3ad7605d299a1e2dbd777f72d9add7d009/chrome/browser/pdf/pdf_extension_test.cc [modify] https://crrev.com/b5706a3ad7605d299a1e2dbd777f72d9add7d009/extensions/browser/guest_view/mime_handler_view/mime_handler_view_browsertest.cc [modify] https://crrev.com/b5706a3ad7605d299a1e2dbd777f72d9add7d009/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.cc [modify] https://crrev.com/b5706a3ad7605d299a1e2dbd777f72d9add7d009/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
,
Oct 3
This is now blocked on network service launching on win/mac/linux/chromeos, as the old MimeTypesHandler path on these platforms still uses streams. see https://chromium-review.googlesource.com/c/chromium/src/+/1252322#message-f250f3d63a7c8546ebf2d33107da65fc85c2007e
,
Oct 3
|
|||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||
Comment 1 by mmenke@chromium.org
, Mar 25 2017