Remove implicit conversions from scoped_refptr to T* in chrome/browser/sync/

Remove implicit conversions from scoped_refptr to T* in chrome/browser/sync/

This patch was generated by running the rewrite_scoped_refptr clang tool
on a Linux build.

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

[dcheng, 2014-08-26 18:40:57]

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

[dcheng, 2014-08-26 18:40:57]

Unfortunately, we can't make scoped_refptr testable until the conversion
operator has been removed. While it would be possible to disable the conversion
operator and make it testable in the same patch, the automated tool tries to
skip rewriting code where manual cleanups would be useful. This includes things
like:

// What if ReturnsScopedRefPtr() creates a new object internally?
if (ReturnsScopedRefPtr())

Foo* Bar() {
  const scoped_refptr<Foo>& f = ReturnsScopedRefPtr();
  return f;  // Does the return type need to be rewritten? Or is .get() safe?
}

Doing this in one patch would make catching some of these instances (in
particular, a boolean test on a temporary scoped_refptr) more difficult. As a
result, the work is being split into several steps:
Step 1: Rewrite all implicit conversions to be explicit (or rewrite the receiver
type to scoped_refptr<T>, if the tool believes the conversion to be unsafe).
Step 2: Once a platform compiles cleanly without the conversion operator, #ifdef
the conversion operator out.
Step 3: Make scoped_refptr testable on that platform.
Step 4: Remove unnecessary calls to .get().

https://codereview.chromium.org/504273003/diff/1/chrome/browser/sync/profile_...
File chrome/browser/sync/profile_sync_components_factory_impl.cc (right):

https://codereview.chromium.org/504273003/diff/1/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_components_factory_impl.cc:534:
scoped_refptr<password_manager::PasswordStore> password_store =
Strictly speaking, this seems like an unnecessary conversion. The tool made this
rewrite though, because in general, it's not safe to convert a temporary
scoped_refptr to a raw pointer. Please let me know if you'd like me to revert
the variable type change here.

[Nicolas Zea, 2014-08-26 23:31:37]

LG with one comment.

https://codereview.chromium.org/504273003/diff/1/chrome/browser/sync/profile_...
File chrome/browser/sync/profile_sync_components_factory_impl.cc (right):

https://codereview.chromium.org/504273003/diff/1/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_components_factory_impl.cc:534:
scoped_refptr<password_manager::PasswordStore> password_store =
On 2014/08/26 18:40:57, dcheng (OOO) wrote:
> Strictly speaking, this seems like an unnecessary conversion. The tool made
this
> rewrite though, because in general, it's not safe to convert a temporary
> scoped_refptr to a raw pointer. Please let me know if you'd like me to revert
> the variable type change here.

This seems fine, although I would have expected the tool to conver the
password_store turnary three lines below into a password_store.get() (or
possible a !!password_store.get()).

[dcheng, 2014-08-26 23:34:54]

PTAL.

https://codereview.chromium.org/504273003/diff/1/chrome/browser/sync/profile_...
File chrome/browser/sync/profile_sync_components_factory_impl.cc (right):

https://codereview.chromium.org/504273003/diff/1/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_components_factory_impl.cc:534:
scoped_refptr<password_manager::PasswordStore> password_store =
On 2014/08/26 23:31:37, Nicolas Zea wrote:
> On 2014/08/26 18:40:57, dcheng (OOO) wrote:
> > Strictly speaking, this seems like an unnecessary conversion. The tool made
> this
> > rewrite though, because in general, it's not safe to convert a temporary
> > scoped_refptr to a raw pointer. Please let me know if you'd like me to
revert
> > the variable type change here.
> 
> This seems fine, although I would have expected the tool to conver the
> password_store turnary three lines below into a password_store.get() (or
> possible a !!password_store.get()).

Oops. Fixed. The tool is single-pass, I usually scan CLs for instances where
types were rewritten and then run the tool again--but I missed this one. Thanks
for the catch.

[Nicolas Zea, 2014-08-26 23:36:04]

lgtm

[dcheng, 2014-08-26 23:37:46]

The CQ bit was checked by dcheng@chromium.org

[commit-bot: I haz the power, 2014-08-26 23:39:43]

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

[commit-bot: I haz the power, 2014-08-27 01:33:38]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  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-27 02:16:30]

Committed patchset #2 (20001) as 4cd56adc8c7d0404be760405dc58133232a5e497

[commit-bot: I haz the power, 2014-09-10 02:49:11]

Patchset 2 (id:??) landed as
https://crrev.com/fda68628ff5b7158c4d2cf20853e6221e253dd06
Cr-Commit-Position: refs/heads/master@{#292081}