Annotates functions returning retained objects.

Annotates functions returning retained objects.

Annotates all C functions and C++ methods returning retained Cocoa
objects with NS_RETURNS_RETAINED annotation. This annotation is no-op
without ARC, and with ARC it ensures correct memory management.

More about this annotation:
http://clang-analyzer.llvm.org/annotations.html#attr_ns_returns_retained

More information about the process behind this cl on the bug.

BUG=636375
Committed: https://crrev.com/1f03171454f9041019b56399a5b6752e559b5429
Cr-Commit-Position: refs/heads/master@{#413113}

[stkhapugin, 2016-08-16 15:29:47]

The CQ bit was checked by stkhapugin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-16 15:30:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[stkhapugin, 2016-08-16 16:19:01]

Description was changed from

==========
format


fixes


missed_bit


ns-returns-retained

BUG=
==========

to

==========
Annotates all C functions and C++ methods returning retained Cocoa objects with
NS_RETURNS_RETAINED annotation.
This annotation is no-op without ARC, and with ARC it ensures correct memory
management.

More about this annotation:
http://clang-analyzer.llvm.org/annotations.html#attr_ns_returns_retained

More information about the process behind this cl on the bug.

BUG=636375
==========

[stkhapugin, 2016-08-16 16:19:01]

stkhapugin@chromium.org changed reviewers:
+ bzanotti@chromium.org, eugenebut@google.com

[stkhapugin, 2016-08-16 16:19:37]

stkhapugin@chromium.org changed reviewers:
+ eugenebut@chromium.org
- eugenebut@google.com

[stkhapugin, 2016-08-16 16:28:11]

stkhapugin@chromium.org changed reviewers:
+ rohitrao@chromium.org

[stkhapugin, 2016-08-16 16:28:11]

PTAL

[bzanotti, 2016-08-16 16:34:17]

On 2016/08/16 16:28:11, stkhapugin wrote:
> PTAL

LGTM for components/signin/ios and ios/chrome/browser/signin code.

[Eugene But (OOO till 7-30), 2016-08-16 16:45:48]

Please format CL description to have summary line and limit the width to 72
characters.

https://codereview.chromium.org/2239773002/diff/20001/ios/public/provider/chr...
File ios/public/provider/chrome/browser/chrome_browser_provider.h (right):

https://codereview.chromium.org/2239773002/diff/20001/ios/public/provider/chr...
ios/public/provider/chrome/browser/chrome_browser_provider.h:9: #import
<Foundation/NSObjCRuntime.h>
Please import <Foundation/Foundation.h> instead:

go/objc-style?showone=Use_Umbrella_Frameworks&cl=head#Use_Umbrella_Frameworks

https://codereview.chromium.org/2239773002/diff/20001/ios/public/provider/chr...
ios/public/provider/chrome/browser/chrome_browser_provider.h:80:
InfoBarViewDelegate* delegate) NS_RETURNS_RETAINED;
C++ Style Guide recommends to avoid returning unowned pointers:
https://engdoc.corp.google.com/eng/doc/devguide/cpp/styleguide.shtml?cl=head#...

Should this return autoreleased object instead? Same question for all methods
which return retained object.

https://codereview.chromium.org/2239773002/diff/20001/ios/web/net/clients/crw...
File ios/web/net/clients/crw_js_injection_network_client_unittest.mm (right):

https://codereview.chromium.org/2239773002/diff/20001/ios/web/net/clients/crw...
ios/web/net/clients/crw_js_injection_network_client_unittest.mm:134: BOOL
include_content_length) NS_RETURNS_RETAINED {
Is it necessary to annotate in both interface and implementation?

[stkhapugin, 2016-08-16 16:51:49]

https://codereview.chromium.org/2239773002/diff/20001/ios/public/provider/chr...
File ios/public/provider/chrome/browser/chrome_browser_provider.h (right):

https://codereview.chromium.org/2239773002/diff/20001/ios/public/provider/chr...
ios/public/provider/chrome/browser/chrome_browser_provider.h:80:
InfoBarViewDelegate* delegate) NS_RETURNS_RETAINED;
On 2016/08/16 16:45:48, Eugene But wrote:
> C++ Style Guide recommends to avoid returning unowned pointers:
>
https://engdoc.corp.google.com/eng/doc/devguide/cpp/styleguide.shtml?cl=head#...
> 
> Should this return autoreleased object instead? Same question for all methods
> which return retained object.

I would argue that all of these {C++ methods|C functions} should be returning
autoreleased objects instead. However, this annotation is a quick fix, it can be
easily searched for later, and if everyone agrees that moving forward we want
all functions to return autoreleased objects, we can fix it later. 

Rohit, what do you think?

https://codereview.chromium.org/2239773002/diff/20001/ios/web/net/clients/crw...
File ios/web/net/clients/crw_js_injection_network_client_unittest.mm (right):

https://codereview.chromium.org/2239773002/diff/20001/ios/web/net/clients/crw...
ios/web/net/clients/crw_js_injection_network_client_unittest.mm:134: BOOL
include_content_length) NS_RETURNS_RETAINED {
On 2016/08/16 16:45:48, Eugene But wrote:
> Is it necessary to annotate in both interface and implementation?

Actually this annotation only makes a difference in headers (as it only changes
behavior across arc/mrr boundaries), but we don't really have any guideline for
this, so I marked the ones in implementation files as well.

[Eugene But (OOO till 7-30), 2016-08-16 17:02:31]

https://codereview.chromium.org/2239773002/diff/20001/ios/web/net/clients/crw...
File ios/web/net/clients/crw_js_injection_network_client_unittest.mm (right):

https://codereview.chromium.org/2239773002/diff/20001/ios/web/net/clients/crw...
ios/web/net/clients/crw_js_injection_network_client_unittest.mm:134: BOOL
include_content_length) NS_RETURNS_RETAINED {
On 2016/08/16 16:51:49, stkhapugin wrote:
> On 2016/08/16 16:45:48, Eugene But wrote:
> > Is it necessary to annotate in both interface and implementation?
> 
> Actually this annotation only makes a difference in headers (as it only
changes
> behavior across arc/mrr boundaries), but we don't really have any guideline
for
> this, so I marked the ones in implementation files as well. 
If |NS_RETURNS_RETAINED| in implementation does not add any value (and seems
like it does not) I think we should not clutter our code with unnecessary stuff.
Do you see any good reason for adding these annotations (except that Style Guide
is silent on this)?

[stkhapugin, 2016-08-18 12:35:31]

Description was changed from

==========
Annotates all C functions and C++ methods returning retained Cocoa objects with
NS_RETURNS_RETAINED annotation.
This annotation is no-op without ARC, and with ARC it ensures correct memory
management.

More about this annotation:
http://clang-analyzer.llvm.org/annotations.html#attr_ns_returns_retained

More information about the process behind this cl on the bug.

BUG=636375
==========

to

==========
Annotates all C functions and C++ methods returning retained Cocoa objects with 
NS_RETURNS_RETAINED annotation.
This annotation is no-op without ARC, and with ARC it ensures correct memory
management.

More about this annotation:
http://clang-analyzer.llvm.org/annotations.html#attr_ns_returns_retained

More information about the process behind this cl on the bug.

BUG=636375
==========

[stkhapugin, 2016-08-18 12:35:52]

Description was changed from

==========
Annotates all C functions and C++ methods returning retained Cocoa objects with 
NS_RETURNS_RETAINED annotation.
This annotation is no-op without ARC, and with ARC it ensures correct memory
management.

More about this annotation:
http://clang-analyzer.llvm.org/annotations.html#attr_ns_returns_retained

More information about the process behind this cl on the bug.

BUG=636375
==========

to

==========
Annotates all C functions and C++ methods returning retained Cocoa objects with 
NS_RETURNS_RETAINED annotation.
This annotation is no-op without ARC, and with ARC it ensures correct
memory management.

More about this annotation:
http://clang-analyzer.llvm.org/annotations.html#attr_ns_returns_retained

More information about the process behind this cl on the bug.

BUG=636375
==========

[stkhapugin, 2016-08-18 12:54:20]

Thanks, PTAL.

Going forward, I think all these functions should be refactored to return
autoreleased objects or scoped_nsobjects instead. I updated the bug accordingly.

https://codereview.chromium.org/2239773002/diff/20001/ios/public/provider/chr...
File ios/public/provider/chrome/browser/chrome_browser_provider.h (right):

https://codereview.chromium.org/2239773002/diff/20001/ios/public/provider/chr...
ios/public/provider/chrome/browser/chrome_browser_provider.h:9: #import
<Foundation/NSObjCRuntime.h>
On 2016/08/16 16:45:48, Eugene But wrote:
> Please import <Foundation/Foundation.h> instead:
> 
> go/objc-style?showone=Use_Umbrella_Frameworks&cl=head#Use_Umbrella_Frameworks

Done.

[rohitrao (ping after 24h), 2016-08-18 13:01:35]

If the annotation is not necessary in the implementation files, I agree that we
should leave it out.  Are we sure that it's not necessary?

[Eugene But (OOO till 7-30), 2016-08-18 14:17:25]

lgtm please update CL description to have one line summary and limit width to 72
symbols if possible.

[stkhapugin, 2016-08-18 15:36:01]

Rohit: yes, I just double-checked. The annotation in the header is sufficient.

[stkhapugin, 2016-08-18 15:38:20]

Description was changed from

==========
Annotates all C functions and C++ methods returning retained Cocoa objects with 
NS_RETURNS_RETAINED annotation.
This annotation is no-op without ARC, and with ARC it ensures correct
memory management.

More about this annotation:
http://clang-analyzer.llvm.org/annotations.html#attr_ns_returns_retained

More information about the process behind this cl on the bug.

BUG=636375
==========

to

==========
Annotates functions returning retained objects.

Annotates all C functions and C++ methods returning retained Cocoa
objects with NS_RETURNS_RETAINED annotation. This annotation is no-op
without ARC, and with ARC it ensures correct memory management.
  
More about this annotation:
http://clang-analyzer.llvm.org/annotations.html#attr_ns_returns_retained
  
More information about the process behind this cl on the bug.
  
BUG=636375
==========

[stkhapugin, 2016-08-18 15:38:50]

Eugene: I updated the description to wrap at 72 characters and copied the title.
Thanks for pointing this out!

[rohitrao (ping after 24h), 2016-08-19 11:03:07]

lgtm

[stkhapugin, 2016-08-19 11:32:42]

The CQ bit was checked by stkhapugin@chromium.org

[stkhapugin, 2016-08-19 11:32:43]

The patchset sent to the CQ was uploaded after l-g-t-m from
bzanotti@chromium.org
Link to the patchset: https://codereview.chromium.org/2239773002/#ps40001
(title: "comments")

[commit-bot: I haz the power, 2016-08-19 11:33:00]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-19 12:08:42]

Description was changed from

==========
Annotates functions returning retained objects.

Annotates all C functions and C++ methods returning retained Cocoa
objects with NS_RETURNS_RETAINED annotation. This annotation is no-op
without ARC, and with ARC it ensures correct memory management.
  
More about this annotation:
http://clang-analyzer.llvm.org/annotations.html#attr_ns_returns_retained
  
More information about the process behind this cl on the bug.
  
BUG=636375
==========

to

==========
Annotates functions returning retained objects.

Annotates all C functions and C++ methods returning retained Cocoa
objects with NS_RETURNS_RETAINED annotation. This annotation is no-op
without ARC, and with ARC it ensures correct memory management.
  
More about this annotation:
http://clang-analyzer.llvm.org/annotations.html#attr_ns_returns_retained
  
More information about the process behind this cl on the bug.
  
BUG=636375
==========

[commit-bot: I haz the power, 2016-08-19 12:08:43]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-19 12:12:20]

Description was changed from

==========
Annotates functions returning retained objects.

Annotates all C functions and C++ methods returning retained Cocoa
objects with NS_RETURNS_RETAINED annotation. This annotation is no-op
without ARC, and with ARC it ensures correct memory management.
  
More about this annotation:
http://clang-analyzer.llvm.org/annotations.html#attr_ns_returns_retained
  
More information about the process behind this cl on the bug.
  
BUG=636375
==========

to

==========
Annotates functions returning retained objects.

Annotates all C functions and C++ methods returning retained Cocoa
objects with NS_RETURNS_RETAINED annotation. This annotation is no-op
without ARC, and with ARC it ensures correct memory management.

More about this annotation:
http://clang-analyzer.llvm.org/annotations.html#attr_ns_returns_retained

More information about the process behind this cl on the bug.

BUG=636375

Committed: https://crrev.com/1f03171454f9041019b56399a5b6752e559b5429
Cr-Commit-Position: refs/heads/master@{#413113}
==========

[commit-bot: I haz the power, 2016-08-19 12:12:21]

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