[ObjC ARC] Converts ios/chrome/common:unit_tests to ARC.

[ObjC ARC] Converts ios/chrome/common:unit_tests to ARC.

Automatically generated ARCMigrate commit
Notable issues:
Separated block_unittest.mm into its own target as it checks memory management and does not apply to an ARC environment. Bug opened at https://bugs.chromium.org/p/chromium/issues/detail?id=690941 to create a test to check for correct interaction of C++ and blocks in the ARC world.
BUG=624363
TEST=None

Review-Url: https://codereview.chromium.org/2681353002
Cr-Commit-Position: refs/heads/master@{#450391}
Committed: https://chromium.googlesource.com/chromium/src/+/6a5106048c624b4ae8421a1f88e7cbe4d7e85869

[lody, 2017-02-09 17:17:27]

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

[commit-bot: I haz the power, 2017-02-09 17:18:05]

Dry run: 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, 2017-02-09 18:19:07]

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

[commit-bot: I haz the power, 2017-02-09 18:19:07]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[lody, 2017-02-10 14:32:53]

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

[commit-bot: I haz the power, 2017-02-10 14:33:18]

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

[lody, 2017-02-10 15:30:48]

Description was changed from

==========
[ObjC ARC] Converts ios/chrome/common:unit_tests to ARC.

Automatically generated ARCMigrate commit
Notable issues:None
BUG=624363
TEST=None
==========

to

==========
[ObjC ARC] Converts ios/chrome/common:unit_tests to ARC.

Automatically generated ARCMigrate commit
Notable issues:
Separated block_unittest.mm into its own target as it checks memory management
and does not apply to an ARC environment. Bug opened at
https://bugs.chromium.org/p/chromium/issues/detail?id=690941 to create a test to
check for correct interaction of C++ and blocks in the ARC world.
BUG=624363
TEST=None
==========

[lody, 2017-02-10 15:31:20]

lod@chromium.org changed reviewers:
+ sdefresne@chromium.org, stkhapugin@chromium.org

[lody, 2017-02-10 15:31:21]

Please review. See the note in the CL description.

[sdefresne, 2017-02-10 15:36:36]

https://codereview.chromium.org/2681353002/diff/20001/ios/chrome/common/BUILD.gn
File ios/chrome/common/BUILD.gn (right):

https://codereview.chromium.org/2681353002/diff/20001/ios/chrome/common/BUILD...
ios/chrome/common/BUILD.gn:32: source_set("noarc_unit_tests") {
Can you add a comment there says that this test should not use ARC? And do we
want to keep such a test when we are 100% ARC?

[commit-bot: I haz the power, 2017-02-10 15:38:09]

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

[commit-bot: I haz the power, 2017-02-10 15:38:10]

Dry run: This issue passed the CQ dry run.

[lody, 2017-02-13 10:25:38]

https://codereview.chromium.org/2681353002/diff/20001/ios/chrome/common/BUILD.gn
File ios/chrome/common/BUILD.gn (right):

https://codereview.chromium.org/2681353002/diff/20001/ios/chrome/common/BUILD...
ios/chrome/common/BUILD.gn:32: source_set("noarc_unit_tests") {
On 2017/02/10 15:36:36, sdefresne wrote:
> Can you add a comment there says that this test should not use ARC? And do we
> want to keep such a test when we are 100% ARC?

Done.

[sdefresne, 2017-02-13 14:15:58]

https://codereview.chromium.org/2681353002/diff/40001/ios/chrome/common/block...
File ios/chrome/common/block_unittest.mm (right):

https://codereview.chromium.org/2681353002/diff/40001/ios/chrome/common/block...
ios/chrome/common/block_unittest.mm:13: // This class should not use ARC. When
everything is ARC, it can be removed.
This is my first time looking at this file (since it was not in the previous
CL).

I think your comment is incorrect and we want to keep this test once we have
migrated everything with ARC. And we also want to migrate this test. It is an
important test checking that ref-counted C++ object are correctly copied when
captured by blocks.

To convert his file to ARC, we need to have deterministic way to release the
blocks. I think this is possible by using @autoreleasepool, applying the
following transformation from:

  void (^heap_block)(int) = nil;
  {
    heap_block = [^(int) {} copy];
  }
  heap_block(2);
  [heap_block release];
  EXPECT_TRUE(object->HasOneRef());

to the following:

  @autoreleasepool {
    void (^heap_block)(int) = nil;
    {
      heap_block = [^(int) {} copy];
    }
    heap_block(2);
  }
  EXPECT_TRUE(object->HasOneRef());

[sdefresne, 2017-02-14 10:04:24]

lgtm with suggested changes

https://codereview.chromium.org/2681353002/diff/40001/ios/chrome/common/BUILD.gn
File ios/chrome/common/BUILD.gn (right):

https://codereview.chromium.org/2681353002/diff/40001/ios/chrome/common/BUILD...
ios/chrome/common/BUILD.gn:53: ":common",
Add ":"noarc_unit_tests" here.

https://codereview.chromium.org/2681353002/diff/40001/ios/chrome/common/block...
File ios/chrome/common/block_unittest.mm (right):

https://codereview.chromium.org/2681353002/diff/40001/ios/chrome/common/block...
ios/chrome/common/block_unittest.mm:13: // This class should not use ARC. When
everything is ARC, it can be removed.
On 2017/02/13 14:15:57, sdefresne wrote:
> This is my first time looking at this file (since it was not in the previous
> CL).
> 
> I think your comment is incorrect and we want to keep this test once we have
> migrated everything with ARC. And we also want to migrate this test. It is an
> important test checking that ref-counted C++ object are correctly copied when
> captured by blocks.
> 
> To convert his file to ARC, we need to have deterministic way to release the
> blocks. I think this is possible by using @autoreleasepool, applying the
> following transformation from:
> 
>   void (^heap_block)(int) = nil;
>   {
>     heap_block = [^(int) {} copy];
>   }
>   heap_block(2);
>   [heap_block release];
>   EXPECT_TRUE(object->HasOneRef());
> 
> to the following:
> 
>   @autoreleasepool {
>     void (^heap_block)(int) = nil;
>     {
>       heap_block = [^(int) {} copy];
>     }
>     heap_block(2);
>   }
>   EXPECT_TRUE(object->HasOneRef());

As discussed on hangouts, remove this comment and I'll take care of converting
the test to ARC (https://codereview.chromium.org/2697613003/).

https://codereview.chromium.org/2681353002/diff/40001/ios/chrome/test/BUILD.gn
File ios/chrome/test/BUILD.gn (right):

https://codereview.chromium.org/2681353002/diff/40001/ios/chrome/test/BUILD.g...
ios/chrome/test/BUILD.gn:186: "//ios/chrome/common:noarc_unit_tests",
Remove this :-)

[lody, 2017-02-14 14:17:14]

Patchset #4 (id:60001) has been deleted

[lody, 2017-02-14 14:23:47]

https://codereview.chromium.org/2681353002/diff/40001/ios/chrome/common/BUILD.gn
File ios/chrome/common/BUILD.gn (right):

https://codereview.chromium.org/2681353002/diff/40001/ios/chrome/common/BUILD...
ios/chrome/common/BUILD.gn:53: ":common",
On 2017/02/14 10:04:24, sdefresne wrote:
> Add ":"noarc_unit_tests" here.

Done.

[lody, 2017-02-14 16:02:36]

The CQ bit was checked by lod@chromium.org

[lody, 2017-02-14 16:02:37]

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

[commit-bot: I haz the power, 2017-02-14 16:03: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, 2017-02-14 17:12:03]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1487088156694910,
"parent_rev": "de0dac51b6200239c885f63f32b0e0df9ab46143", "commit_rev":
"6a5106048c624b4ae8421a1f88e7cbe4d7e85869"}

[commit-bot: I haz the power, 2017-02-14 17:12:39]

Description was changed from

==========
[ObjC ARC] Converts ios/chrome/common:unit_tests to ARC.

Automatically generated ARCMigrate commit
Notable issues:
Separated block_unittest.mm into its own target as it checks memory management
and does not apply to an ARC environment. Bug opened at
https://bugs.chromium.org/p/chromium/issues/detail?id=690941 to create a test to
check for correct interaction of C++ and blocks in the ARC world.
BUG=624363
TEST=None
==========

to

==========
[ObjC ARC] Converts ios/chrome/common:unit_tests to ARC.

Automatically generated ARCMigrate commit
Notable issues:
Separated block_unittest.mm into its own target as it checks memory management
and does not apply to an ARC environment. Bug opened at
https://bugs.chromium.org/p/chromium/issues/detail?id=690941 to create a test to
check for correct interaction of C++ and blocks in the ARC world.
BUG=624363
TEST=None

Review-Url: https://codereview.chromium.org/2681353002
Cr-Commit-Position: refs/heads/master@{#450391}
Committed:
https://chromium.googlesource.com/chromium/src/+/6a5106048c624b4ae8421a1f88e7...
==========

[commit-bot: I haz the power, 2017-02-14 17:12:41]

Committed patchset #4 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/6a5106048c624b4ae8421a1f88e7...