net: Use SequencedTaskRunner for URLFetcherResponseWriter

net: Use SequencedTaskRunner for URLFetcherResponseWriter

TBR=scottbyer@chromium.org for chrome/service/cloud_print/cloud_print_url_fetcher.h
BUG=316065
TEST=build

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

[hashimoto, 2013-11-07 09:41:49]

mmenke@: please review net/
kinaba@: please review google_apis related parts

[mmenke, 2013-11-07 17:02:36]

https://codereview.chromium.org/63923002/diff/140002/chrome/browser/drive/dri...
File chrome/browser/drive/drive_api_service.cc (right):

https://codereview.chromium.org/63923002/diff/140002/chrome/browser/drive/dri...
chrome/browser/drive/drive_api_service.cc:23: #include
"net/url_request/url_request_context_getter.h"
Don't we need sequenced_task_runner here, to increment the ref count?

https://codereview.chromium.org/63923002/diff/140002/chrome/browser/drive/dri...
File chrome/browser/drive/drive_service_interface.h (right):

https://codereview.chromium.org/63923002/diff/140002/chrome/browser/drive/dri...
chrome/browser/drive/drive_service_interface.h:16: }
Is this related to this CL?

https://codereview.chromium.org/63923002/diff/140002/chrome/browser/drive/gda...
File chrome/browser/drive/gdata_wapi_service.cc (right):

https://codereview.chromium.org/63923002/diff/140002/chrome/browser/drive/gda...
chrome/browser/drive/gdata_wapi_service.cc:22: #include
"net/url_request/url_request_context_getter.h"
Don't we need sequenced_task_runner here, to increment the ref count?

https://codereview.chromium.org/63923002/diff/140002/chrome/browser/google_ap...
File chrome/browser/google_apis/base_requests.cc (right):

https://codereview.chromium.org/63923002/diff/140002/chrome/browser/google_ap...
chrome/browser/google_apis/base_requests.cc:111: new
net::URLFetcherFileWriter(file_task_runner, file_path));
Don't we need sequenced_task_runner here, to increment the ref count?

https://codereview.chromium.org/63923002/diff/140002/chrome/browser/sync_file...
File chrome/browser/sync_file_system/drive_backend_v1/api_util.cc (right):

https://codereview.chromium.org/63923002/diff/140002/chrome/browser/sync_file...
chrome/browser/sync_file_system/drive_backend_v1/api_util.cc:31: #include
"net/base/mime_util.h"
Again, the include.

https://codereview.chromium.org/63923002/diff/140002/net/url_request/url_fetc...
File net/url_request/url_fetcher_core.cc (right):

https://codereview.chromium.org/63923002/diff/140002/net/url_request/url_fetc...
net/url_request/url_fetcher_core.cc:269:
scoped_refptr<base::SequencedTaskRunner> file_task_runner) {
Shouldn't we include sequenced_task_runner.h for this (We have to increment the
ref count).  We do get it from single_thread_task_runner, but don't think we
should be relying on that.

https://codereview.chromium.org/63923002/diff/140002/net/url_request/url_fetc...
File net/url_request/url_fetcher_impl.cc (right):

https://codereview.chromium.org/63923002/diff/140002/net/url_request/url_fetc...
net/url_request/url_fetcher_impl.cc:124:
scoped_refptr<base::SequencedTaskRunner> file_task_runner) {
Again, shouldn't we be including sequenced_task_runner.h?

[hashimoto, 2013-11-08 04:08:16]

https://codereview.chromium.org/63923002/diff/140002/chrome/browser/drive/dri...
File chrome/browser/drive/drive_api_service.cc (right):

https://codereview.chromium.org/63923002/diff/140002/chrome/browser/drive/dri...
chrome/browser/drive/drive_api_service.cc:23: #include
"net/url_request/url_request_context_getter.h"
On 2013/11/07 17:02:36, mmenke wrote:
> Don't we need sequenced_task_runner here, to increment the ref count?

This doesn't result in a compile error because
content/public/browser/browser_thread.h (included above) includes
base/message_loop/message_loop_proxy.h, and it eventually includes
sequenced_task_runner.h.
Though, having an explicit include sounds good.
Done.

https://codereview.chromium.org/63923002/diff/140002/chrome/browser/drive/dri...
File chrome/browser/drive/drive_service_interface.h (right):

https://codereview.chromium.org/63923002/diff/140002/chrome/browser/drive/dri...
chrome/browser/drive/drive_service_interface.h:16: }
On 2013/11/07 17:02:36, mmenke wrote:
> Is this related to this CL?

Sorry for this change being messy.
Because I removed task_runner.h include (which results in including
base/time/time.h) in url_fetcher.h, class Time is no longer declared in
namespace base at this point.

https://codereview.chromium.org/63923002/diff/140002/chrome/browser/drive/gda...
File chrome/browser/drive/gdata_wapi_service.cc (right):

https://codereview.chromium.org/63923002/diff/140002/chrome/browser/drive/gda...
chrome/browser/drive/gdata_wapi_service.cc:22: #include
"net/url_request/url_request_context_getter.h"
On 2013/11/07 17:02:36, mmenke wrote:
> Don't we need sequenced_task_runner here, to increment the ref count?

browser_thread.h includes it.
Anyways, added explicit one.

https://codereview.chromium.org/63923002/diff/140002/chrome/browser/google_ap...
File chrome/browser/google_apis/base_requests.cc (right):

https://codereview.chromium.org/63923002/diff/140002/chrome/browser/google_ap...
chrome/browser/google_apis/base_requests.cc:111: new
net::URLFetcherFileWriter(file_task_runner, file_path));
On 2013/11/07 17:02:36, mmenke wrote:
> Don't we need sequenced_task_runner here, to increment the ref count?

chrome/browser/google_apis/task_util.h includes sequenced_task_runner.h via
message_loop_proxy.h.
Added explicit one.

https://codereview.chromium.org/63923002/diff/140002/chrome/browser/sync_file...
File chrome/browser/sync_file_system/drive_backend_v1/api_util.cc (right):

https://codereview.chromium.org/63923002/diff/140002/chrome/browser/sync_file...
chrome/browser/sync_file_system/drive_backend_v1/api_util.cc:31: #include
"net/base/mime_util.h"
On 2013/11/07 17:02:36, mmenke wrote:
> Again, the include.
Added explicit one.

https://codereview.chromium.org/63923002/diff/140002/net/url_request/url_fetc...
File net/url_request/url_fetcher_core.cc (right):

https://codereview.chromium.org/63923002/diff/140002/net/url_request/url_fetc...
net/url_request/url_fetcher_core.cc:269:
scoped_refptr<base::SequencedTaskRunner> file_task_runner) {
On 2013/11/07 17:02:36, mmenke wrote:
> Shouldn't we include sequenced_task_runner.h for this (We have to increment
the
> ref count).  We do get it from single_thread_task_runner, but don't think we
> should be relying on that.

Done.

https://codereview.chromium.org/63923002/diff/140002/net/url_request/url_fetc...
File net/url_request/url_fetcher_impl.cc (right):

https://codereview.chromium.org/63923002/diff/140002/net/url_request/url_fetc...
net/url_request/url_fetcher_impl.cc:124:
scoped_refptr<base::SequencedTaskRunner> file_task_runner) {
On 2013/11/07 17:02:36, mmenke wrote:
> Again, shouldn't we be including sequenced_task_runner.h?

Done.

[mmenke, 2013-11-08 15:23:49]

LGTM

[kinaba, 2013-11-11 06:31:45]

On 2013/11/08 15:23:49, mmenke wrote:
> LGTM

drive/ google_apis/ LGTM

[hashimoto, 2013-11-11 06:37:23]

tzik@chromium.org: Please review chrome/browser/sync_file_system
satorux@chromium.org: Please review chrome/browser/chromeos/contacts

[satorux1, 2013-11-11 07:43:14]

contacts lgtm

https://codereview.chromium.org/63923002/diff/460001/chrome/browser/chromeos/...
File chrome/browser/chromeos/contacts/gdata_contacts_service.cc (right):

https://codereview.chromium.org/63923002/diff/460001/chrome/browser/chromeos/...
chrome/browser/chromeos/contacts/gdata_contacts_service.cc:859:
BrowserThread::GetBlockingPool()->GetSequenceToken()).get(),
looks verbose, but looks it's necessary...

[tzik, 2013-11-11 08:00:54]

lgtm

[hashimoto, 2013-11-11 08:49:29]

TBRing scottbyer@chromium.org for an addition of base/time/time.h include in
chrome/service/cloud_print/cloud_print_url_fetcher.h
This include is needed because of the deletion of task_runner.h include from
url_fetcher.h

[commit-bot: I haz the power, 2013-11-11 08:50:33]

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

[commit-bot: I haz the power, 2013-11-11 12:26:56]

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_rel&...

[commit-bot: I haz the power, 2013-11-11 14:09:08]

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

[commit-bot: I haz the power, 2013-11-11 19:50:04]

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...

[Scott Byer, 2013-11-11 21:48:45]

LGTM for cloud print

[commit-bot: I haz the power, 2013-11-12 00:35:33]

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

[commit-bot: I haz the power, 2013-11-12 01:09:58]

Change committed as 234357