FileAPI/sync file system cleanups for scoped_refptr operator T* cleanup.

FileAPI/sync file system cleanups for scoped_refptr operator T* cleanup.

BUG=110610
Committed: https://crrev.com/803cfebdca2a5a924a13053cd7a85733e7110eb2
Cr-Commit-Position: refs/heads/master@{#292400}

[dcheng, 2014-08-28 09:24:45]

dcheng@chromium.org changed reviewers:
+ nhiroki@chromium.org

[dcheng, 2014-08-28 09:24:45]

These are some manual fixups for cases that the tool skips.

There are many call sites which pass a ref counted object as a raw pointer, but
then store it into a scoped_refptr. I have changed these to pass the argument as
a const scoped_refptr<T>&, to help signal that the function will take a
reference to it.

[nhiroki, 2014-08-28 09:36:57]

lgtm with nits

https://codereview.chromium.org/515093002/diff/1/chrome/browser/sync_file_sys...
File
chrome/browser/sync_file_system/drive_backend/drive_backend_sync_unittest.cc
(right):

https://codereview.chromium.org/515093002/diff/1/chrome/browser/sync_file_sys...
chrome/browser/sync_file_system/drive_backend/drive_backend_sync_unittest.cc:34:
#include "net/url_request/url_request_context_getter.h"
nit: is this necessary?

https://codereview.chromium.org/515093002/diff/1/chrome/browser/sync_file_sys...
File chrome/browser/sync_file_system/drive_backend/sync_engine_unittest.cc
(right):

https://codereview.chromium.org/515093002/diff/1/chrome/browser/sync_file_sys...
chrome/browser/sync_file_system/drive_backend/sync_engine_unittest.cc:20:
#include "net/url_request/url_request_context_getter.h"
ditto.

[dcheng, 2014-08-28 09:40:06]

https://codereview.chromium.org/515093002/diff/1/chrome/browser/sync_file_sys...
File
chrome/browser/sync_file_system/drive_backend/drive_backend_sync_unittest.cc
(right):

https://codereview.chromium.org/515093002/diff/1/chrome/browser/sync_file_sys...
chrome/browser/sync_file_system/drive_backend/drive_backend_sync_unittest.cc:34:
#include "net/url_request/url_request_context_getter.h"
On 2014/08/28 09:36:57, nhiroki wrote:
> nit: is this necessary?

This is required, because this test instantiates a SyncEngine but passes in NULL
for the URLRequestContextGetter. Since this argument is now a const
scoped_refptr<T>&, it constructs a temporary
scoped_refptr<URLRequestContextGetter> to pass to the constructor.
Unfortunately, this needs the full definition of URLRequestContextGetter.

Another option is to move the includes to the header for SyncEngine. I'm not
sure how you would feel about that though.

https://codereview.chromium.org/515093002/diff/1/chrome/browser/sync_file_sys...
File chrome/browser/sync_file_system/drive_backend/sync_engine_unittest.cc
(right):

https://codereview.chromium.org/515093002/diff/1/chrome/browser/sync_file_sys...
chrome/browser/sync_file_system/drive_backend/sync_engine_unittest.cc:20:
#include "net/url_request/url_request_context_getter.h"
On 2014/08/28 09:36:57, nhiroki wrote:
> ditto.

Similar reason for this one as well.

[nhiroki, 2014-08-28 09:51:57]

https://codereview.chromium.org/515093002/diff/1/chrome/browser/sync_file_sys...
File
chrome/browser/sync_file_system/drive_backend/drive_backend_sync_unittest.cc
(right):

https://codereview.chromium.org/515093002/diff/1/chrome/browser/sync_file_sys...
chrome/browser/sync_file_system/drive_backend/drive_backend_sync_unittest.cc:34:
#include "net/url_request/url_request_context_getter.h"
On 2014/08/28 09:40:05, dcheng (OOO) wrote:
> On 2014/08/28 09:36:57, nhiroki wrote:
> > nit: is this necessary?
> 
> This is required, because this test instantiates a SyncEngine but passes in
NULL
> for the URLRequestContextGetter. Since this argument is now a const
> scoped_refptr<T>&, it constructs a temporary
> scoped_refptr<URLRequestContextGetter> to pass to the constructor.
> Unfortunately, this needs the full definition of URLRequestContextGetter.
> 
> Another option is to move the includes to the header for SyncEngine. I'm not
> sure how you would feel about that though.

That makes sense! Thanks for your explanation. It's ok to commit this patchset.

https://codereview.chromium.org/515093002/diff/1/chrome/browser/sync_file_sys...
File chrome/browser/sync_file_system/drive_backend/sync_engine_unittest.cc
(right):

https://codereview.chromium.org/515093002/diff/1/chrome/browser/sync_file_sys...
chrome/browser/sync_file_system/drive_backend/sync_engine_unittest.cc:20:
#include "net/url_request/url_request_context_getter.h"
On 2014/08/28 09:40:05, dcheng (OOO) wrote:
> On 2014/08/28 09:36:57, nhiroki wrote:
> > ditto.
> 
> Similar reason for this one as well.

Acknowledged.

[dcheng, 2014-08-28 09:52:56]

The CQ bit was checked by dcheng@chromium.org

[commit-bot: I haz the power, 2014-08-28 09:53:29]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dcheng@chromium.org/515093002/1

[dcheng, 2014-08-28 10:18:25]

dcheng@chromium.org changed reviewers:
+ danakj@chromium.org, sky@chromium.org

[dcheng, 2014-08-28 10:18:25]

+danakj for OWNERS review of webkit, +sky for OWNERS review of
content/browser/quota changes.

[commit-bot: I haz the power, 2014-08-28 11:00:53]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[commit-bot: I haz the power, 2014-08-28 11:06:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-28 11:06:03]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[sky, 2014-08-28 16:27:20]

LGTM

[danakj, 2014-08-28 16:41:12]

webkit/ LGTM

[dcheng, 2014-08-28 16:43:02]

The CQ bit was checked by dcheng@chromium.org

[commit-bot: I haz the power, 2014-08-28 16:44:17]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dcheng@chromium.org/515093002/1

[commit-bot: I haz the power, 2014-08-28 16:50:24]

Committed patchset #1 (id:1) as 052e4ce203245909dadecf729237d3b9642b7853

[commit-bot: I haz the power, 2014-09-10 03:00:09]

Patchset 1 (id:??) landed as
https://crrev.com/803cfebdca2a5a924a13053cd7a85733e7110eb2
Cr-Commit-Position: refs/heads/master@{#292400}