Add SetUploadFilePath method to URLFetcher.

Add SetUploadFilePath method to URLFetcher.

BUG=169551
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=185780

[mattm, 2013-02-28 04:39:18]

The discussion of possible fixes on https://codereview.chromium.org/12224066/
all seemed pretty complicated, so I decided to give the simplest possible
solution a try.  What do you think?

[mmenke, 2013-02-28 16:53:23]

When making those suggestions, I hadn't realized this was all you needed.  I had
assumed the goal was for implementing custom subclasses to provide dynamically
generated data.

https://codereview.chromium.org/12383015/diff/1/net/url_request/url_fetcher_c...
File net/url_request/url_fetcher_core.cc (right):

https://codereview.chromium.org/12383015/diff/1/net/url_request/url_fetcher_c...
net/url_request/url_fetcher_core.cc:157: file_task_runner_ = file_task_runner;
When combined with SaveResponseToFileAtPath, this seems a little unexpected.

Both sets of file operations are isolated enough that using the "wrong" file
thread wouldn't currently cause any issues, but this seems regression prone to
me.

The options I see are to either heavily document this behavior, or to just track
another task runner.  I'd suggest the latter - the overhead is pretty minimal.

While the behavior in this CL is unlikely to break anything, it's a subtle
enough difference that I'd rather stay on the side of paranoia.

https://codereview.chromium.org/12383015/diff/1/net/url_request/url_fetcher_i...
File net/url_request/url_fetcher_impl_unittest.cc (right):

https://codereview.chromium.org/12383015/diff/1/net/url_request/url_fetcher_i...
net/url_request/url_fetcher_impl_unittest.cc:21: #include
"net/base/upload_file_element_reader.h"
Neither of these is needed.

[mattm, 2013-02-28 22:13:05]

On 2013/02/28 16:53:23, mmenke wrote:
> When making those suggestions, I hadn't realized this was all you needed.  I
had
> assumed the goal was for implementing custom subclasses to provide dynamically
> generated data.

Well, I did a similar comment on that codereview, but I guess it wasn't clear or
got lost in the noise.

https://codereview.chromium.org/12383015/diff/1/net/url_request/url_fetcher_c...
File net/url_request/url_fetcher_core.cc (right):

https://codereview.chromium.org/12383015/diff/1/net/url_request/url_fetcher_c...
net/url_request/url_fetcher_core.cc:157: file_task_runner_ = file_task_runner;
On 2013/02/28 16:53:23, mmenke wrote:
> When combined with SaveResponseToFileAtPath, this seems a little unexpected.
> 
> Both sets of file operations are isolated enough that using the "wrong" file
> thread wouldn't currently cause any issues, but this seems regression prone to
> me.
> 
> The options I see are to either heavily document this behavior, or to just
track
> another task runner.  I'd suggest the latter - the overhead is pretty minimal.
> 
> While the behavior in this CL is unlikely to break anything, it's a subtle
> enough difference that I'd rather stay on the side of paranoia.

Done.

https://codereview.chromium.org/12383015/diff/1/net/url_request/url_fetcher_i...
File net/url_request/url_fetcher_impl_unittest.cc (right):

https://codereview.chromium.org/12383015/diff/1/net/url_request/url_fetcher_i...
net/url_request/url_fetcher_impl_unittest.cc:21: #include
"net/base/upload_file_element_reader.h"
On 2013/02/28 16:53:23, mmenke wrote:
> Neither of these is needed.

Done.

[mmenke, 2013-03-01 17:03:15]

On 2013/02/28 22:13:05, mattm wrote:
> On 2013/02/28 16:53:23, mmenke wrote:
> > When making those suggestions, I hadn't realized this was all you needed.  I
> had
> > assumed the goal was for implementing custom subclasses to provide
dynamically
> > generated data.
> 
> Well, I did a similar comment on that codereview, but I guess it wasn't clear
or
> got lost in the noise.

I think I just misunderstood that "file" meant "file on a local disk".

LGTM

[commit-bot: I haz the power, 2013-03-01 19:44:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mattm@chromium.org/12383015/3003

[commit-bot: I haz the power, 2013-03-02 16:23:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mattm@chromium.org/12383015/3003

[commit-bot: I haz the power, 2013-03-02 19:34:56]

Retried try job too often on win_rel for step(s) base_unittests, browser_tests,
cacheinvalidation_unittests, check_deps, chrome_frame_net_tests,
chrome_frame_tests, chrome_frame_unittests, chromedriver2_unittests,
components_unittests, content_browsertests, content_unittests, crypto_unittests,
gpu_unittests, installer_util_unittests, ipc_tests, jingle_unittests,
media_unittests, mini_installer_test, nacl_integration, net_unittests,
ppapi_unittests, printing_unittests, remoting_unittests, sql_unittests,
sync_integration_tests, sync_unit_tests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-03-02 21:45:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mattm@chromium.org/12383015/3003

[commit-bot: I haz the power, 2013-03-02 21:46:17]

Change committed as 185780