Allow copying scoped_nsobject of different type.

Allow copying scoped_nsobject of different type.

This code was leading to a crash:
scoped_nsobject<MyClass> a([[MyClass alloc] init]);
scoped_nsobject<NSObject> b(a);

Because |a| is converted to raw pointer, and |b| is constructed with the
raw pointer, without calling retain. This creates a subtle crash later on
when the scoped_nsobject are destroyed.

This code however, was not crashing:
scoped_nsobject<MyClass> b(a);
Because in that case the copy constructor is used.

This was somewhat inconsistent and bug prone, especially if the type of
|a| is changed. In that case the type of |b| must be changed at the same
time, or it causes a crash.

This CL introduces a new constructor for the scoped_nsobject variants
that allows passing a scoped_nsobject of a different type.
It is consistent with scoped_refptr which also has this constructor.

Committed: https://crrev.com/941226ba0e49b1e2d359ea084ecb00c672dd6054
Cr-Commit-Position: refs/heads/master@{#311487}

[droger, 2015-01-14 13:21:24]

droger@chromium.org changed reviewers:
+ sdefresne@chromium.org

[droger, 2015-01-14 13:21:25]

FYI I'm running the CL on the downstream bots here:
https://chromereviews.googleplex.com/138877014/

[droger, 2015-01-14 14:15:05]

droger@chromium.org changed reviewers:
+ mark@chromium.org

[droger, 2015-01-14 14:15:05]

+mark as OWNER

[sdefresne, 2015-01-14 14:26:21]

lgtm

[Mark Mentovai, 2015-01-14 14:33:02]

Does this allow you to do an end run around the typing system without explicitly
casting? For example, if ClassOne and ClassTwo are unrelated (except for both
being NSObject subclasses),

scoped_nsobject<ClassOne> one([ClassOne alloc] init]);
scoped_nsobject<ClassTwo> two(one);

should not work and should preferably be a compile-time error.

[droger, 2015-01-14 15:12:46]

On 2015/01/14 14:33:02, Mark Mentovai wrote:
> Does this allow you to do an end run around the typing system without
explicitly
> casting? For example, if ClassOne and ClassTwo are unrelated (except for both
> being NSObject subclasses),
> 
> scoped_nsobject<ClassOne> one([ClassOne alloc] init]);
> scoped_nsobject<ClassTwo> two(one);
> 
> should not work and should preferably be a compile-time error.

I expect this to generate a compile error yes.

I did the test:

scoped_nsobject<NSData> a;
scoped_nsobject<NSString> b(a);

Compile output:

../../base/mac/scoped_nsobject.h:45:9: error: cannot initialize a member
subobject of type 'NSString *' with an rvalue of type 'NSData *'
      : object_([that.get() retain]) {
        ^       ~~~~~~~~~~~~~~~~~~~
../../base/mac/scoped_nsobject.h:129:9: note: in instantiation of function
template specialization 'base::scoped_nsprotocol<NSString
*>::scoped_nsprotocol<NSData *>' requested here
      : scoped_nsprotocol<NST*>(that) {
        ^
../../ios_internal/chrome/app/main_controller.mm:958:35: note: in instantiation
of function template specialization
'base::scoped_nsobject<NSString>::scoped_nsobject<NSData>' requested here
  base::scoped_nsobject<NSString> b(a);
                                  ^
/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator8.1.sdk/usr/include/objc/NSObject.h:36:1:
note: instance method 'retain' is assumed to return an instance of its receiver
type ('NSData *')
- (instancetype)retain OBJC_ARC_UNAVAILABLE;
^
1 error generated.

[sdefresne, 2015-01-14 15:16:19]

On 2015/01/14 at 14:33:02, mark wrote:
> Does this allow you to do an end run around the typing system without
explicitly casting? For example, if ClassOne and ClassTwo are unrelated (except
for both being NSObject subclasses),
> 
> scoped_nsobject<ClassOne> one([ClassOne alloc] init]);
> scoped_nsobject<ClassTwo> two(one);
> 
> should not work and should preferably be a compile-time error.

No, this does not work around the type system, since the compilation will fail
if there is no conversion between the two types, as this will basically boils
down to "two.object_ = [one.object_ retain]" where "two" is of type "ClassTwo*"
and "one" of type "ClassOne*", which only compile if there is a valid conversion
from "ClassOne" to "ClassTwo".

BTW, scoped_ptr has the same conversion too:
https://code.google.com/p/chromium/codesearch#chromium/src/base/memory/scoped...

[Mark Mentovai, 2015-01-14 15:21:13]

Perfect, LGTM.

[droger, 2015-01-14 15:22:16]

Thanks for the reviews.

[droger, 2015-01-14 15:22:20]

The CQ bit was checked by droger@chromium.org

[commit-bot: I haz the power, 2015-01-14 15:23:16]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/848033006/1

[commit-bot: I haz the power, 2015-01-14 16:18:18]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-01-14 16:19:34]

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