net: Allow URLFetcher users to inject custom URLFetcherResponseWriter

net: Allow URLFetcher users to inject custom URLFetcherResponseWriter

Add a new method URLFetcher::SaveResponseWithWriter()
Add NET_EXPORT to URLFetcherResponseWriter and its subclasses so that user code can inherit them to implement custom writers.

BUG=126753
TEST=net_unittests

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

[hashimoto, 2013-10-11 09:54:55]

Please review.

After this patch, I'd like to implement a subclass of URLFetcherResponseWriter
for Google Drive code and remove OnURLFetchDownloadData() from
URLFetcherDelegate.

[wtc, 2013-10-14 19:57:23]

Patch set 1 LGTM. It would be nice to ask kinaba to also take a look. Thanks!

https://codereview.chromium.org/26230003/diff/3001/net/url_request/url_fetcher.h
File net/url_request/url_fetcher.h (right):

https://codereview.chromium.org/26230003/diff/3001/net/url_request/url_fetche...
net/url_request/url_fetcher.h:247: scoped_ptr<URLFetcherResponseWriter>
response_writer) = 0;

Why don't you remove the SaveResponseToFileAtPath and
SaveResponseToTemporaryFile methods? Are you planning to remove them later?

https://codereview.chromium.org/26230003/diff/3001/net/url_request/url_fetche...
File net/url_request/url_fetcher_response_writer.h (right):

https://codereview.chromium.org/26230003/diff/3001/net/url_request/url_fetche...
net/url_request/url_fetcher_response_writer.h:80: class NET_EXPORT
URLFetcherFileWriter : public URLFetcherResponseWriter {

Nit: I understand why we need to export the base class URLFetcherResponseWriter.
Do we also need to export the derived classes URLFetcherStringWriter and
URLFetcherFileWriter?

[hashimoto, 2013-10-15 01:00:35]

kinaba@,
Could you take a look on this change?

https://codereview.chromium.org/26230003/diff/3001/net/url_request/url_fetcher.h
File net/url_request/url_fetcher.h (right):

https://codereview.chromium.org/26230003/diff/3001/net/url_request/url_fetche...
net/url_request/url_fetcher.h:247: scoped_ptr<URLFetcherResponseWriter>
response_writer) = 0;
On 2013/10/14 19:57:23, wtc wrote:
> 
> Why don't you remove the SaveResponseToFileAtPath and
> SaveResponseToTemporaryFile methods? Are you planning to remove them later?

I thought it might be OK to leave SaveResponseToFileAtPath,
SaveResponseToTemporaryFile and GetResponseAsFilePath unremoved because these
methods are quite simple now.
But if we are to remove these methods, we can do it later, after converting all
user code (Google Drive, imageburner, component_updater and extension_download)
to use SaveResponseWithWriter.

https://codereview.chromium.org/26230003/diff/3001/net/url_request/url_fetche...
File net/url_request/url_fetcher_response_writer.h (right):

https://codereview.chromium.org/26230003/diff/3001/net/url_request/url_fetche...
net/url_request/url_fetcher_response_writer.h:80: class NET_EXPORT
URLFetcherFileWriter : public URLFetcherResponseWriter {
On 2013/10/14 19:57:23, wtc wrote:
> 
> Nit: I understand why we need to export the base class
URLFetcherResponseWriter.
> Do we also need to export the derived classes URLFetcherStringWriter and
> URLFetcherFileWriter?

By exporting these derived classes, URLFetcher user code can implement custom
URLFetcherResponseWriter easily, without implementing the whole class from
scratch. (e.g. Our Google Drive code will use a flavored URLFetcherFileWriter
with OnURLFetchDownloadData() equivalent functionality.)

[kinaba, 2013-10-15 01:16:23]

lgtm

[commit-bot: I haz the power, 2013-10-15 09:09:19]

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

[commit-bot: I haz the power, 2013-10-15 09:47:06]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win7_aura.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-10-16 04:12:24]

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

[commit-bot: I haz the power, 2013-10-16 04:49:26]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win7_aura.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-10-16 04:52:47]

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

[commit-bot: I haz the power, 2013-10-16 05:00:48]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_clan...

[commit-bot: I haz the power, 2013-10-16 05:25:14]

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

[commit-bot: I haz the power, 2013-10-16 10:28:13]

Change committed as 228885