as expected simply adding the checks from SimpleURLLoader to some of the other URLLoaderClient implementations causes tons and tons of test failures: https://chromium-review.googlesource.com/c/chromium/src/+/984908

My feeling is we should be consistent here - either always call it, or never call it when the body is empty.  Allowing both behaviors increases the risk of consumers not handling the case they aren't tested against.  Testing all consumers against both behaviors also seems extra work for no practical gain.

I'm fine with either behavior.

Agreed that consistency would be ideal. And consistently always calling it on success does seem like it would make a bunch of code simpler (at least a bunch of code I've been writing recently, where otherwise I constantly have to special-case the success-with-empty-body case).

But I don't have a strong opinion either, the main concern either way I'd have is that it is hard to enforce that all URLLoaders actually are consistent. Even if we add whatever checks we deem correct to SimpleURLLoader/TestURLLoaderClient/URLLoaderClientImpl that won't help unless every URLLoader implementation actually has a test that tests an empty bodies response.

I think reaching a decision on behavior is more important than updating code, whatever the decision - that way, if something breaks, at least we know which end of the pipe is misbehaving.  Not that we should fix consumers/subclasses, just that having an API going forward seems the more urgent task.

Yeah, agreed that figuring out the contract for URLLoader is the most important thing here. My weak vote would be for requiring OnResponseBodyStarted for successful loads, but I don't think my opinion should count for too much, since this part of the code isn't exactly my area of expertise.

jam: can you make a decision here regarding contract for URLLoader implementations/users?

Sorry for the delay, I missed this.

Adding a few more folks to see what they think so we can reach consensus.

I have two questions:
1) is there a behavior difference in what network service's URLLoader does vs non-nS's MojoAsyncResourceHandler? if the former should copy the latter to remove differences between the 2 code paths.
2) If the above are the same, what did net::URLFetcher expect? I suspect we want SimpleURLLoader to behave the same, if they have differences, to ease the transition.

MojoAsyncResourceHandler won't call OnStartLoadingResponseBody until the first (non-zero) OnReadCompleted (so it won't call it at all for an empty response body). 

Network service's URLLoader has logic in NotifyCompleted to make sure it calls OnStartLoadingResponseBody before calling OnComplete if it hadn't already called it (so it will always call it even for an empty response body).

net::URLFetcher exposed an entirely different API from URLLoader, and didn't provide pipes, but rather just pushed data, so there's no real analog there.  SimpleURLLoader masks the fact that there are two different pipes, it just provides a callback to completion, so a change here would require a SimpleURLLoader, but have no effect on any of its consumers.

It looks to me like MojoAsyncResourceHandler only passes along its data pipe on the first non-zero byte read, I believe (Which is the opposed of what URLLoader does).  I'm not sure if we should use that decision to bias our choice one way or another - we could just as easily modify MojoAsyncResourceHandler to match URLLoader, as the other way around, I believe.

Thanks for the info.

Personally I don't care which one we standardize on.

I have a preference to match what MojoAsyncResourceHandler does, but as Matt points out we can also change MojoAsyncResourceHandler to do what we think is best.

I think I slightly prefer not calling StartLoadingResponseBody if there's no data, but suspect Network Service's URLLoader contract might make things slightly simpler.

I guess we'll need to change a few URLLoader implementations in addition to MojoAsyncResourceHandler if we take that approach, but maybe that can happen.

I prefer MojoAsyncResourceHandler's way, too.

It seems like the majority opinion is to go with MojoAsyncResourceHandler's API, so let's go with that, then.  We should update URLLoader and SimpleULRLoader (In the same CL) to change the API.

Consumers will still have to work with receiving pipes but no data on them in (rather uncommon) error cases, and will have to handle pipes with no data followed by a net::OK, though this will be an error case now, as this will only happen if the net::OK completion code indicates a non-zero amount of data was received.  This could happen if there's some sort of error on the data pipe after success.  Hopefully everything will use SimpleURLLoader, and not have to worry about all that.

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

commit 407e4de01e534567b5c9b7e453cbdf7b79f467af
Author: Makoto Shimazu <shimazu@chromium.org>
Date: Mon Jun 11 10:22:29 2018

NetS13nServiceWorker: don't assume OnStartLoadingResponseBody() is called

In the current implementation, ServiceWorkerNewScriptLoader assume that
OnScriptLoadingResponseBody() is called before OnComplete(). However,
MojoAsyncResourceHandler doesn't call it when response body is empty. This CL is
to adopt that based on discussion at  crbug.com/826868 .

Bug:  848606 ,  826868 
Change-Id: I46646fc2d48c51aa89e02d63c057ef42e74ebb7d
Reviewed-on: https://chromium-review.googlesource.com/1088463
Commit-Queue: Makoto Shimazu <shimazu@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565953}
[modify] https://crrev.com/407e4de01e534567b5c9b7e453cbdf7b79f467af/content/browser/service_worker/service_worker_new_script_loader.cc
[modify] https://crrev.com/407e4de01e534567b5c9b7e453cbdf7b79f467af/content/browser/service_worker/service_worker_new_script_loader.h
[modify] https://crrev.com/407e4de01e534567b5c9b7e453cbdf7b79f467af/content/browser/service_worker/service_worker_new_script_loader_unittest.cc
[modify] https://crrev.com/407e4de01e534567b5c9b7e453cbdf7b79f467af/third_party/WebKit/LayoutTests/TestExpectations

+arthursonzogni@

Arthur's trying to make a change to prepare and send the body data pipe (along with response header and URLLoader{,Client} pairs) to the renderer frame on Navigation Commit. This could potentially bring some perf benefits, while it will also change MojoAsyncResourceHandler's behavior to always call StartLoadingResponseBody() regardless of whether the body is empty or not, i.e: it makes the behavior of URLLoader and MojoAsyncResourceHandler same, but in the opposite way that is agreed on #14.

Given the potential perf benefits I started to feel okay with that direction, but wanted to check how others feel-- any thoughts?

Either way is fine with me.

I'm going to remove the canary-blocking label on this unless we know there's a bug.

We know there is a bug though. SimpleURLLoader considers a load an error if it didn't receive a body stream before OnComplete is called, while various URLLoader implementations don't send a body stream for empty bodies. So using SimpleURLLoader to load empty resources from certain schemes will result in buggy behavior.

Yes my understanding is that this could block Canary.  All we have to do is just to converge on, and I think if people are happy with the change #18 (maybe the last person I should talk to is yhirano@ once he's back) we should be just going over other existing URLLoaders to make sure they behave same as network::URLLoader.

(Restoring the lable, but feel free to revert if anyone feels otherwise)

@mek: can you specifically say which URLLoader implementation currently has this bug that is possible to see by users? To be clear I'm not asking this in a snarky way, but we have we have many URLLoader implementations and it's not clear to me that they all have to be consistent to avoid user visible bugs. Most of them are not used by SimpleURLLoader.

SimpleURLLoader is currently only used with the network service directly, I believe, so I don't think there are currently any bugs one could encounter here.  Using the SimpleURLLoader with different URLLoaderFactories, on the other hand, one could well run into issues.

#23 isn't true. It's at least also used with things like WebUIURLLoaderFactory. But that one fortunately already behaves the same as network::URLLoader, so isn't a problem.

I haven't exhaustively scanned all several hundred usages of SimpleURLLoader though, so no idea if there are other ones hiding somewhere that could be problematic.

But mostly I don't understand jam's resistance to just making a decision and fixing this (at least that's how #22 reads to me). Trying to figure out if not fixing this for canary is safe seems like it is way way more work than just picking one side, and making sure all URLLoader implementations are correct (there are a lot less URLLoader implementations than SimpleURLLoader users after all). I realize there's plenty of other (potentially higher priority) issues than this one, but it doesn't have to take months to at least unify SimpleURLLoader, URLLoaderCliemtImpl and TestURLLoaderClient (and maybe even DCHECK for misbehaving factories rather than the current silent failure in SimpleURLLoader? But I guess that has the problem that the network process can take down other processes. It could at least BadMessage the URLLoaderClient pipe though, as that seems the most appropriate response to API contract violating mojom API calls?).

@mek I'm definitely not opposed to fixing this, and either approach sounds good to me as I've mentioned above.

All I'm saying is that we're using canary-blocking as things that we know will cause bugs. I think we have different perspective on this; where you're saying things default to canary blocking unless we're sure that it can't have bugs.

There are pros and cons to each position; I take this one mostly because 1) canary is a lower bar than stable (e.g. I would mark this one as stable blocking, but not canary blocking) and 2) I think the benefits of going to canary earlier, even with possibilities of bugs, outweigh the risks.

The problem with going to canary with known brokenness is that it makes Canary less stable - and we don't want to scare people away from using Canary, given how important it is to keep Chrome healthy.

I agree with comment 26. We're going in circles re "known brokenness" :) I still don't know of an example where this difference is currently breaking chrome with network service.

Fwiw, I do agree that any breakage as a result of this is likely to be minor enough to not have to block releasing to Canary. But at the same time I think getting the fundamentals like this right is also important enough that priority wise this should be resolved soon enough that it would end up being resolved before all the Canary blockers are solved anyway. So wether this bug itself is Canary blocking is rather irrelevant then.

Ok it sounds like we're on the same page (I agree this is important to fix)

It looks to me like NavigationURLLoaderImpl is broken if a net::OK response is sent without passing a body pipe - it seems to do nothing in that case.  I think that case will end up with the spinner going forever?  See URLLoaderRequestController::OnComplete / NavigationURLLoaderImpl::OnComplete.

#30: after a response is received the loading (loader ptr and client) is basically detached and transferred to the renderer process, so the code path is called only when loading fails before OnResponseReceived. Or are you seeing problems in the case? MojoAsyncResourceHandler can call OnComplete without passing a body stream pipe and NavigationURLLoaderImpl uses it when Network Service is disabled, so it should be working fine I suppose.

I was wrong - the case where we just throw away a result and hang eternally is if we get net::OK without receiving any headers - seems like we should DCHECK if that happens, if the code doesn't handle it (We are dealing with less trusted processes, though this wouldn't be a security issue - we just ignore the net::OK complete result, rather than do anything about it).

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

commit fecf153880556829aa643a33d7e015184ad8b1af
Author: Arthur Sonzogni <arthursonzogni@chromium.org>
Date: Thu Nov 08 08:44:36 2018

In tests, always send response's body data pipe, even if empty.

After sending:
  URLLoaderClient::OnReceiveResponse(response_head),
not all URLLoader are sending:
  URLLoaderClient::OnStartLoadingResponseBody(response_body).

It even happens to send URLLoaderClient::OnComplete(net::OK) after that,
which may confuse the URLLoaderClient.

Most URLLoader not sending a response's body datapipe are the one
with an empty response. For consistency, always send a response's
body datapipe, even if it doesn't contains data.

This CL updates every tests not aligned with this.

Once every URLLoader and tests are consistent. DCHECK enforcing this
will be added.

This CL is a prerequisite for:
https://chromium-review.googlesource.com/c/chromium/src/+/1172290

Bug:  826868 , 831155
Change-Id: Id3434ea442c93b47bd200d15322cc21cd0c0b89f
Reviewed-on: https://chromium-review.googlesource.com/c/1323092
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606386}
[modify] https://crrev.com/fecf153880556829aa643a33d7e015184ad8b1af/chrome/browser/devtools/devtools_sanity_browsertest.cc
[modify] https://crrev.com/fecf153880556829aa643a33d7e015184ad8b1af/chrome/browser/ui/views/location_bar/location_bar_view_browsertest.cc
[modify] https://crrev.com/fecf153880556829aa643a33d7e015184ad8b1af/content/renderer/loader/resource_dispatcher_unittest.cc

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

commit 2dfa163da62c53bcfbe1b87240ac1368cae66c67
Author: Arthur Sonzogni <arthursonzogni@chromium.org>
Date: Thu Nov 08 16:04:06 2018

AboutURLLoader: send an empty response's body.

This CL is a prerequisite for:
https://chromium-review.googlesource.com/c/chromium/src/+/1172290

After sending:
URLLoaderClient::OnReceiveResponse(response_head),
not all URLLoader are sending:
URLLoaderClient::OnStartLoadingResponseBody(response_body).

It even happens to send URLLoaderClient::OnComplete(net::OK) after that,
which may confuse the URLLoaderClient.

This CL updates the AboutURLLoader class. The goal is to align every
URLLoaders, they will always send a data pipe after OnReceiveResponse().

Bug:  826868 , 831155
Change-Id: Iab3be1ed84011793a23eac304621d0ec0fc7f47e
Reviewed-on: https://chromium-review.googlesource.com/c/1323070
Reviewed-by: Camille Lamy <clamy@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606485}
[modify] https://crrev.com/2dfa163da62c53bcfbe1b87240ac1368cae66c67/content/browser/loader/navigation_url_loader_impl.cc

Assigning this bug to myself.

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

commit c25c2041ed579bcea8215c1c40063275ac86144f
Author: Arthur Sonzogni <arthursonzogni@chromium.org>
Date: Mon Nov 12 16:54:32 2018

Simplify BlobURLLoader and always send OnStartLoadingResponseBody.

This is a sort of prerequisite for:
https://chromium-review.googlesource.com/c/chromium/src/+/1172290

The main goal is after sending:
  URLLoaderClient::OnReceiveResponse(response_header)
to ALWAYS send:
  URLLoaderClient::OnStartLoadingResponseBody(response_body)

Some URLLoader, when the response's body body is empty, don't even try to send
the response_body data pipe. In the URLLoaderClient, not having to handle the
case with no data pipe would be a nice simplification.

What this CL does:
1) In BlobUrlLoader, create the data pipe at ::Start() time.
2) Pass the producer handle to the MojoBlobCreate at construction time.
3) Pass the consumer handle to OnStartLoadingResponseBody(), immediately
   after OnReceiveResponse().

Some code simplification was possible. There is no more need to
implement PassDataPipe() in BlobURLLoader, ReaderDelegate and
DataPipeReaderDelegate. There is no more need to give them the pipe at
construction time neither.

Bug:  826868 , 831155
Change-Id: I827cb39bdd782624ff362874e98be90cab0d47f3
Reviewed-on: https://chromium-review.googlesource.com/c/1329741
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607266}
[modify] https://crrev.com/c25c2041ed579bcea8215c1c40063275ac86144f/storage/browser/blob/blob_impl.cc
[modify] https://crrev.com/c25c2041ed579bcea8215c1c40063275ac86144f/storage/browser/blob/blob_url_loader.cc
[modify] https://crrev.com/c25c2041ed579bcea8215c1c40063275ac86144f/storage/browser/blob/blob_url_loader.h
[modify] https://crrev.com/c25c2041ed579bcea8215c1c40063275ac86144f/storage/browser/blob/mojo_blob_reader.cc
[modify] https://crrev.com/c25c2041ed579bcea8215c1c40063275ac86144f/storage/browser/blob/mojo_blob_reader.h

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

commit b6d920e2f3a2457057853f2e8791916ffffe55bf
Author: Arthur Sonzogni <arthursonzogni@chromium.org>
Date: Mon Nov 12 17:05:23 2018

Service worker: match OnStartLoadingResponseBody with OnReceiveResponse.

Some URLLoaders aren't always sending a response's body datapipe to their
client after sending URLLoaderClient::OnReceiveResponse(response_head).
It even happens to send URLLoaderClient::OnComplete(net::OK) after that.

Most of the time, they do not send a datapipe, because the response's
body is empty.

The goal is to align URLLoaders so that they will always send a data
pipe by using OnStartLoadingResponseBody(response_body).

This CL is about ServiceWorker's URLLoaders.

This CL is a prerequisite for:
https://chromium-review.googlesource.com/c/chromium/src/+/1172290

Bug:  826868 , 831155
Change-Id: Ib28fc2067240d611ec149099b5365f70610a9369
Reviewed-on: https://chromium-review.googlesource.com/c/1323109
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607273}
[modify] https://crrev.com/b6d920e2f3a2457057853f2e8791916ffffe55bf/content/browser/service_worker/service_worker_navigation_loader.cc
[modify] https://crrev.com/b6d920e2f3a2457057853f2e8791916ffffe55bf/content/browser/service_worker/service_worker_navigation_loader.h
[modify] https://crrev.com/b6d920e2f3a2457057853f2e8791916ffffe55bf/content/renderer/service_worker/service_worker_subresource_loader.cc
[modify] https://crrev.com/b6d920e2f3a2457057853f2e8791916ffffe55bf/content/renderer/service_worker/service_worker_subresource_loader.h

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

commit 9aca9fc8da7772338917bc310af2410997963044
Author: Arthur Sonzogni <arthursonzogni@chromium.org>
Date: Thu Nov 29 10:47:02 2018

Update MojoAsyncResourceHandler. Always send response's body datapipe.

Instead of creating the response's body data pipe during the first call to
OnWillRead(), it is now done in OnWillStart(). The data pipe
consumer handle is available in OnResponseStarted().

It allows the MojoAsyncResourceHandler to call:
 * URLLoader::OnReceiveResponse(response_headers)
 * URLLoader::OnStartLoadingResponseBody(response_body)
at the same time.

The goal is to guarantee the response's body is always sent, even if it doesn't
contains any data.

Bug: 831155,  826868 
Change-Id: I85d84d05ad3fd362d96394834e14297d550f5f13
Reviewed-on: https://chromium-review.googlesource.com/c/1352254
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612137}
[modify] https://crrev.com/9aca9fc8da7772338917bc310af2410997963044/content/browser/loader/mojo_async_resource_handler.cc
[modify] https://crrev.com/9aca9fc8da7772338917bc310af2410997963044/content/browser/loader/mojo_async_resource_handler.h
[modify] https://crrev.com/9aca9fc8da7772338917bc310af2410997963044/content/browser/loader/mojo_async_resource_handler_unittest.cc

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

commit b78c35086d661489c42042e94362294cf3e9a15b
Author: Arthur Sonzogni <arthursonzogni@chromium.org>
Date: Thu Nov 29 11:50:48 2018

MimeSniffingURLLoader: Always forward empty response's body.

This is similar to:
 * https://chromium-review.googlesource.com/c/chromium/src/+/1323109
 * https://chromium-review.googlesource.com/c/chromium/src/+/1329163
 * https://chromium-review.googlesource.com/c/chromium/src/+/1323070
 * https://chromium-review.googlesource.com/c/chromium/src/+/1323092

On request success, it makes the MimeSniffingURLLoader to always send a
response's body datapipe, even when it is empty.

It also adds a DCHECK to ensure the |source_url_loader| is also behaving
the same way.

Bug: 831155, 826868 
Change-Id: I468a24c31142cb0a8d646cd09928f7b03680d779
Reviewed-on: https://chromium-review.googlesource.com/c/1350874
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612152}
[modify] https://crrev.com/b78c35086d661489c42042e94362294cf3e9a15b/content/common/mime_sniffing_throttle_unittest.cc
[modify] https://crrev.com/b78c35086d661489c42042e94362294cf3e9a15b/content/common/mime_sniffing_url_loader.cc

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

commit 63f924d605b141a7ab84606f7c954007ddf9fbbf
Author: Arthur Sonzogni <arthursonzogni@chromium.org>
Date: Wed Dec 05 08:45:26 2018

PrefetchURLLoader.

Always send a response's body data pipe on URL load.

Bug: 831155,  826868 
Change-Id: I170cdfbacb8b1d6f56b868717de51aa91ec596b6
Reviewed-on: https://chromium-review.googlesource.com/c/1352774
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613909}
[modify] https://crrev.com/63f924d605b141a7ab84606f7c954007ddf9fbbf/content/browser/loader/prefetch_url_loader.cc

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

commit 0bc656bf1d60b779bda6503f8852a8e0964725d5
Author: Arthur Sonzogni <arthursonzogni@chromium.org>
Date: Wed Dec 05 11:15:39 2018

DCHECK: Successful URL load requires a response's body

DCHECK that when URLLoaderClient::OnComplete(net::OK) is called, a
response's body has been received before.

Bug: 831155, 826868 
Change-Id: I821267f5fdefcc1e8f18be6a96138237207e7b77
Reviewed-on: https://chromium-review.googlesource.com/c/1352259
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613936}
[modify] https://crrev.com/0bc656bf1d60b779bda6503f8852a8e0964725d5/content/renderer/loader/url_loader_client_impl.cc
[modify] https://crrev.com/0bc656bf1d60b779bda6503f8852a8e0964725d5/content/renderer/loader/url_loader_client_impl.h
[modify] https://crrev.com/0bc656bf1d60b779bda6503f8852a8e0964725d5/content/renderer/loader/url_loader_client_impl_unittest.cc

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

commit a7fe6dfa93848704dd2eff05fe40656c6254328d
Author: Jun Cai <juncai@chromium.org>
Date: Wed Dec 05 20:38:50 2018

Revert "DCHECK: Successful URL load requires a response's body"

This reverts commit 0bc656bf1d60b779bda6503f8852a8e0964725d5.

Reason for revert: During network service sheriffing, noticed that the Mojo Linux bot started failing from:
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mojo%20Linux/23165
and two layout tests failed:
http/tests/worklet/import-filesystem-url.html
virtual/threaded/http/tests/worklet/import-filesystem-url.html

It looks like it is related to this CL.

Original change's description:
> DCHECK: Successful URL load requires a response's body
> 
> DCHECK that when URLLoaderClient::OnComplete(net::OK) is called, a
> response's body has been received before.
> 
> Bug: 831155, 826868 
> Change-Id: I821267f5fdefcc1e8f18be6a96138237207e7b77
> Reviewed-on: https://chromium-review.googlesource.com/c/1352259
> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#613936}

TBR=kinuko@chromium.org,arthursonzogni@chromium.org, jam@chromium.org

Change-Id: I61cb591566839e6587390de82e90d8ccf08edfdd
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 831155,  826868 ,  912293 
Reviewed-on: https://chromium-review.googlesource.com/c/1363899
Reviewed-by: Jun Cai <juncai@chromium.org>
Commit-Queue: Jun Cai <juncai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614085}
[modify] https://crrev.com/a7fe6dfa93848704dd2eff05fe40656c6254328d/content/renderer/loader/url_loader_client_impl.cc
[modify] https://crrev.com/a7fe6dfa93848704dd2eff05fe40656c6254328d/content/renderer/loader/url_loader_client_impl.h
[modify] https://crrev.com/a7fe6dfa93848704dd2eff05fe40656c6254328d/content/renderer/loader/url_loader_client_impl_unittest.cc

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

commit 93f4cecd7900f827c4eab321b64072650a92ef6d
Author: Arthur Sonzogni <arthursonzogni@chromium.org>
Date: Fri Dec 07 08:27:16 2018

FileSystemURLLoader: always send response's body.

When a file is empty, the FileSystemURLLoader must send the response's
head, but also the response's body. The latter was missing.

Bug: 831155, 826868 , 912293 
Change-Id: I51805ca596bd7d97e04221f24222e15576a20d34
Reviewed-on: https://chromium-review.googlesource.com/c/1365431
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614641}
[modify] https://crrev.com/93f4cecd7900f827c4eab321b64072650a92ef6d/content/browser/fileapi/file_system_url_loader_factory.cc

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

commit a727d4b76ffcdf8c3dc03e19a08c90d217420c62
Author: Arthur Sonzogni <arthursonzogni@chromium.org>
Date: Mon Dec 10 09:10:18 2018

(reland) DCHECK: Successful URL load requires a response's body

Reland of https://chromium-review.googlesource.com/c/chromium/src/+/1352259

DCHECK that when URLLoaderClient::OnComplete(net::OK) is called, a
response's body has been received before.

Bug: 831155, 826868 , 912293 
Change-Id: I387fc3930cf3ffadbd6a8ddb6b96c30c4dbc1120
Reviewed-on: https://chromium-review.googlesource.com/c/1352259
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#613936}
Reviewed-on: https://chromium-review.googlesource.com/c/1365234
Cr-Commit-Position: refs/heads/master@{#615069}
[modify] https://crrev.com/a727d4b76ffcdf8c3dc03e19a08c90d217420c62/content/renderer/loader/url_loader_client_impl.cc
[modify] https://crrev.com/a727d4b76ffcdf8c3dc03e19a08c90d217420c62/content/renderer/loader/url_loader_client_impl.h
[modify] https://crrev.com/a727d4b76ffcdf8c3dc03e19a08c90d217420c62/content/renderer/loader/url_loader_client_impl_unittest.cc

It should be completed now.