net: Move file operation code from UploadData to UploadDataStream

net: Move file operation code from UploadData to UploadDataStream

ResetOffset(), GetContentLength() and IsInMemory() are moved from UploadData to UploadDataStream.
This is an intermediate step to make simlar change to UploadElement

BUG=72001
TEST=net_unittests

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=154594

[hashimoto, 2012-08-27 22:09:40]

Will,

Could you review this change?
This change is a preparation for removing file operation code from UploadElement
(http://codereview.chromium.org/10868064/)

http://codereview.chromium.org/10878082/diff/4001/webkit/tools/test_shell/sim...
File webkit/tools/test_shell/simple_resource_loader_bridge.cc (right):

http://codereview.chromium.org/10878082/diff/4001/webkit/tools/test_shell/sim...
webkit/tools/test_shell/simple_resource_loader_bridge.cc:625:
net::UploadDataStream stream(request_->get_upload_mutable());
This filthy code here and in resource_dispatcher_host_impl.cc will get unneeded
once we commit http://codereview.chromium.org/10825073/

[willchan no longer on Chromium, 2012-08-28 23:45:22]

http://codereview.chromium.org/10878082/diff/18002/content/browser/renderer_h...
File content/browser/renderer_host/resource_dispatcher_host_impl.cc (right):

http://codereview.chromium.org/10878082/diff/18002/content/browser/renderer_h...
content/browser/renderer_host/resource_dispatcher_host_impl.cc:969: if
(upload_stream.Init() == net::OK)
I'm concerned about calling Init() here. Doesn't that have side effects? Do we
really want to be doing that all the time? It looks like it resets the offsets
in the elements. I guess that's fine here, but in the webkit
SimpleResourceLoaderBridge where we're just getting upload progress, isn't
resetting offsets wrong?

[hashimoto, 2012-08-28 23:49:56]

http://codereview.chromium.org/10878082/diff/18002/content/browser/renderer_h...
File content/browser/renderer_host/resource_dispatcher_host_impl.cc (right):

http://codereview.chromium.org/10878082/diff/18002/content/browser/renderer_h...
content/browser/renderer_host/resource_dispatcher_host_impl.cc:969: if
(upload_stream.Init() == net::OK)
On 2012/08/28 23:45:22, willchan wrote:
> I'm concerned about calling Init() here. Doesn't that have side effects? Do we
> really want to be doing that all the time? It looks like it resets the offsets
> in the elements. I guess that's fine here, but in the webkit
> SimpleResourceLoaderBridge where we're just getting upload progress, isn't
> resetting offsets wrong?

You're right.  Resetting offsets here is wrong.
We can get rid of this code once GetUploadProgress starts returning the upload
size (http://codereview.chromium.org/10825073/ is still waiting for darin's
review)
Let's submit this change after that.

While waiting for darin's review, could you take a look at other parts?
Thanks,

[willchan no longer on Chromium, 2012-08-28 23:59:36]

http://codereview.chromium.org/10878082/diff/18002/content/browser/renderer_h...
File content/browser/renderer_host/resource_dispatcher_host_impl.cc (right):

http://codereview.chromium.org/10878082/diff/18002/content/browser/renderer_h...
content/browser/renderer_host/resource_dispatcher_host_impl.cc:969: if
(upload_stream.Init() == net::OK)
On 2012/08/28 23:49:56, hashimoto wrote:
> On 2012/08/28 23:45:22, willchan wrote:
> > I'm concerned about calling Init() here. Doesn't that have side effects? Do
we
> > really want to be doing that all the time? It looks like it resets the
offsets
> > in the elements. I guess that's fine here, but in the webkit
> > SimpleResourceLoaderBridge where we're just getting upload progress, isn't
> > resetting offsets wrong?
> 
> You're right.  Resetting offsets here is wrong.
> We can get rid of this code once GetUploadProgress starts returning the upload
> size (http://codereview.chromium.org/10825073/ is still waiting for darin's
> review)
> Let's submit this change after that.
> 
> While waiting for darin's review, could you take a look at other parts?
> Thanks,

I read everything else already. That was my only concern.

[hashimoto, 2012-08-30 23:13:31]

On 2012/08/28 23:59:36, willchan wrote:
>
http://codereview.chromium.org/10878082/diff/18002/content/browser/renderer_h...
> File content/browser/renderer_host/resource_dispatcher_host_impl.cc (right):
> 
>
http://codereview.chromium.org/10878082/diff/18002/content/browser/renderer_h...
> content/browser/renderer_host/resource_dispatcher_host_impl.cc:969: if
> (upload_stream.Init() == net::OK)
> On 2012/08/28 23:49:56, hashimoto wrote:
> > On 2012/08/28 23:45:22, willchan wrote:
> > > I'm concerned about calling Init() here. Doesn't that have side effects?
Do
> we
> > > really want to be doing that all the time? It looks like it resets the
> offsets
> > > in the elements. I guess that's fine here, but in the webkit
> > > SimpleResourceLoaderBridge where we're just getting upload progress, isn't
> > > resetting offsets wrong?
> > 
> > You're right.  Resetting offsets here is wrong.
> > We can get rid of this code once GetUploadProgress starts returning the
upload
> > size (http://codereview.chromium.org/10825073/ is still waiting for darin's
> > review)
> > Let's submit this change after that.
> > 
> > While waiting for darin's review, could you take a look at other parts?
> > Thanks,
> 
> I read everything else already. That was my only concern.

Could you revisit this change?

[willchan no longer on Chromium, 2012-09-01 00:12:19]

LGTM

[commit-bot: I haz the power, 2012-09-01 00:13:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hashimoto@chromium.org/10878082/29002

[commit-bot: I haz the power, 2012-09-01 03:46:07]

Change committed as 154594