[ObjC ARC] Converts ios/chrome/browser/ui/history:history to ARC.

[ObjC ARC] Converts ios/chrome/browser/ui/history:history to ARC.

Semi-autogenerated ARCMigrate commit.
Notable issues:
 * Since one of the weak_nsobjects is exposed in a header and I change it to a __weak instead, converted two unit tests to ARC alongside this.
 * Fixed some assignments of newly created views to weak properties by introducing an intermediate local variable to retain the view until it's added as subview.

BUG=624363
TEST=None

Review-Url: https://codereview.chromium.org/2624963003
Cr-Commit-Position: refs/heads/master@{#443240}
Committed: https://chromium.googlesource.com/chromium/src/+/b103d6c66bbd7bebfe22affab2ea4930685a55bc

[stkhapugin, 2017-01-11 15:13:44]

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

[commit-bot: I haz the power, 2017-01-11 15:14:15]

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-01-11 15:21:29]

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

[commit-bot: I haz the power, 2017-01-11 15:21:30]

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

[stkhapugin, 2017-01-11 16:28:29]

Description was changed from

==========
[ObjC ARC] Converts ios/chrome/browser/ui/history:history to ARC.

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

to

==========
[ObjC ARC] Converts ios/chrome/browser/ui/history:history to ARC.

Semi-autogenerated ARCMigrate commit.
Notable issues:
 * Since one of the weak_nsobjects is exposed in a header and I change it to a
__weak instead, converted two unit tests to ARC alongside this.

BUG=624363
TEST=None
==========

[stkhapugin, 2017-01-11 16:28:37]

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

[commit-bot: I haz the power, 2017-01-11 16:28:57]

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

[stkhapugin, 2017-01-11 16:29:45]

Description was changed from

==========
[ObjC ARC] Converts ios/chrome/browser/ui/history:history to ARC.

Semi-autogenerated ARCMigrate commit.
Notable issues:
 * Since one of the weak_nsobjects is exposed in a header and I change it to a
__weak instead, converted two unit tests to ARC alongside this.

BUG=624363
TEST=None
==========

to

==========
[ObjC ARC] Converts ios/chrome/browser/ui/history:history to ARC.

Semi-autogenerated ARCMigrate commit.
Notable issues:
 * Since one of the weak_nsobjects is exposed in a header and I change it to a
__weak instead, converted two unit tests to ARC alongside this.
 * Fixed some assignments of newly created views to weak properties by
introducing an intermediate local variable to retain the view until it's added
as subview.

BUG=624363
TEST=None
==========

[stkhapugin, 2017-01-11 16:45:41]

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

[stkhapugin, 2017-01-11 16:45:41]

PTAL. Sorry the CL is so big, but it's mostly trivial.

[commit-bot: I haz the power, 2017-01-11 17:27:59]

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

[commit-bot: I haz the power, 2017-01-11 17:28:00]

Dry run: This issue passed the CQ dry run.

[lpromero, 2017-01-11 18:03:52]

lgtm

https://codereview.chromium.org/2624963003/diff/20001/ios/chrome/browser/ui/h...
File ios/chrome/browser/ui/history/favicon_view_provider.h (right):

https://codereview.chromium.org/2624963003/diff/20001/ios/chrome/browser/ui/h...
ios/chrome/browser/ui/history/favicon_view_provider.h:41: @property(weak,
nonatomic, readonly) FaviconView* faviconView;
Seems backed by a scoped_nsobject, so is it really weak?

https://codereview.chromium.org/2624963003/diff/20001/ios/chrome/browser/ui/h...
File ios/chrome/browser/ui/history/history_collection_view_controller.mm
(right):

https://codereview.chromium.org/2624963003/diff/20001/ios/chrome/browser/ui/h...
ios/chrome/browser/ui/history/history_collection_view_controller.mm:42: #import
"ios/third_party/material_components_ios/src/components/ActivityIndicator/src/MDCActivityIndicator.h"
Sort

https://codereview.chromium.org/2624963003/diff/20001/ios/chrome/browser/ui/h...
ios/chrome/browser/ui/history/history_collection_view_controller.mm:91:
@property(nonatomic, strong) ContextMenuCoordinator* contextMenuCoordinator;
assign to strong?

https://codereview.chromium.org/2624963003/diff/20001/ios/chrome/browser/ui/h...
ios/chrome/browser/ui/history/history_collection_view_controller.mm:181:
UILongPressGestureRecognizer* longPressRecognizer = [
Are there other usage of scoped_nsobject in this file? If not, consider removing
the import.

https://codereview.chromium.org/2624963003/diff/20001/ios/chrome/browser/ui/h...
ios/chrome/browser/ui/history/history_collection_view_controller.mm:724:
params.view.reset(self.collectionView);
Is this a scoped_nsobject? ARC will do the right thing here?

https://codereview.chromium.org/2624963003/diff/20001/ios/chrome/browser/ui/h...
File ios/chrome/browser/ui/history/history_entry_item.mm (right):

https://codereview.chromium.org/2624963003/diff/20001/ios/chrome/browser/ui/h...
ios/chrome/browser/ui/history/history_entry_item.mm:226: // Redeclare as
readwrite to allow property releaser to handle these properties.
Remove the part of the comment about the property releaser.

https://codereview.chromium.org/2624963003/diff/20001/ios/chrome/browser/ui/h...
File ios/chrome/browser/ui/history/tab_history_cell.h (right):

https://codereview.chromium.org/2624963003/diff/20001/ios/chrome/browser/ui/h...
ios/chrome/browser/ui/history/tab_history_cell.h:15: @property(weak, nonatomic,
readonly) UILabel* titleLabel;
Weird how it puts it at the beginning. Anything we can/should do there?

[stkhapugin, 2017-01-12 14:15:44]

Thanks, PTAL. I've addressed your comments and removed the rest of
scoped_nsobject/WeakNSObject boilerplate.

https://codereview.chromium.org/2624963003/diff/20001/ios/chrome/browser/ui/h...
File ios/chrome/browser/ui/history/favicon_view_provider.h (right):

https://codereview.chromium.org/2624963003/diff/20001/ios/chrome/browser/ui/h...
ios/chrome/browser/ui/history/favicon_view_provider.h:41: @property(weak,
nonatomic, readonly) FaviconView* faviconView;
On 2017/01/11 18:03:51, lpromero wrote:
> Seems backed by a scoped_nsobject, so is it really weak?

Done.

https://codereview.chromium.org/2624963003/diff/20001/ios/chrome/browser/ui/h...
File ios/chrome/browser/ui/history/history_collection_view_controller.mm
(right):

https://codereview.chromium.org/2624963003/diff/20001/ios/chrome/browser/ui/h...
ios/chrome/browser/ui/history/history_collection_view_controller.mm:42: #import
"ios/third_party/material_components_ios/src/components/ActivityIndicator/src/MDCActivityIndicator.h"
On 2017/01/11 18:03:51, lpromero wrote:
> Sort

Done.

https://codereview.chromium.org/2624963003/diff/20001/ios/chrome/browser/ui/h...
ios/chrome/browser/ui/history/history_collection_view_controller.mm:91:
@property(nonatomic, strong) ContextMenuCoordinator* contextMenuCoordinator;
On 2017/01/11 18:03:51, lpromero wrote:
> assign to strong?

Seems to be the intended use. Nothing retains contextMenuCoordinator with ARC.
Before it was assigned to an assign property with a retain count of +1 (see line
740) and never released.

https://codereview.chromium.org/2624963003/diff/20001/ios/chrome/browser/ui/h...
ios/chrome/browser/ui/history/history_collection_view_controller.mm:181:
UILongPressGestureRecognizer* longPressRecognizer = [
On 2017/01/11 18:03:51, lpromero wrote:
> Are there other usage of scoped_nsobject in this file? If not, consider
removing
> the import.

The import is already removed.

https://codereview.chromium.org/2624963003/diff/20001/ios/chrome/browser/ui/h...
ios/chrome/browser/ui/history/history_collection_view_controller.mm:724:
params.view.reset(self.collectionView);
On 2017/01/11 18:03:52, lpromero wrote:
> Is this a scoped_nsobject? ARC will do the right thing here?

Yes and yes: 

This will call reset(x) on scoped_nsobject. Since this is called from ARC, it
will call the ARC version of this method (ifdef-ed) which retains the argument.
Just like any method call, the reset() will be called with an autoreleased
parameter. Hence the scoped_nsobject will own 1 refcount, as expected.

https://codereview.chromium.org/2624963003/diff/20001/ios/chrome/browser/ui/h...
File ios/chrome/browser/ui/history/history_entry_item.mm (right):

https://codereview.chromium.org/2624963003/diff/20001/ios/chrome/browser/ui/h...
ios/chrome/browser/ui/history/history_entry_item.mm:226: // Redeclare as
readwrite to allow property releaser to handle these properties.
On 2017/01/11 18:03:52, lpromero wrote:
> Remove the part of the comment about the property releaser.

Done.

https://codereview.chromium.org/2624963003/diff/20001/ios/chrome/browser/ui/h...
File ios/chrome/browser/ui/history/tab_history_cell.h (right):

https://codereview.chromium.org/2624963003/diff/20001/ios/chrome/browser/ui/h...
ios/chrome/browser/ui/history/tab_history_cell.h:15: @property(weak, nonatomic,
readonly) UILabel* titleLabel;
On 2017/01/11 18:03:52, lpromero wrote:
> Weird how it puts it at the beginning. Anything we can/should do there?

I don't think we're consistent with property qualifier ordering anywhere, but
I'll reorder the strong above to have consistency at least in one file.

[stkhapugin, 2017-01-12 14:15:53]

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

[commit-bot: I haz the power, 2017-01-12 14:16:12]

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

[lpromero, 2017-01-12 14:18:07]

lgtm

[commit-bot: I haz the power, 2017-01-12 15:12:03]

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

[commit-bot: I haz the power, 2017-01-12 15:12:04]

Dry run: This issue passed the CQ dry run.

[stkhapugin, 2017-01-12 15:44:58]

The CQ bit was checked by stkhapugin@chromium.org

[commit-bot: I haz the power, 2017-01-12 15:45:31]

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-01-12 15:50:31]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1484235898753080,
"parent_rev": "a8b222fa08f6024ff547f1f35ee219abde6ee511", "commit_rev":
"b103d6c66bbd7bebfe22affab2ea4930685a55bc"}

[commit-bot: I haz the power, 2017-01-12 15:51:07]

Description was changed from

==========
[ObjC ARC] Converts ios/chrome/browser/ui/history:history to ARC.

Semi-autogenerated ARCMigrate commit.
Notable issues:
 * Since one of the weak_nsobjects is exposed in a header and I change it to a
__weak instead, converted two unit tests to ARC alongside this.
 * Fixed some assignments of newly created views to weak properties by
introducing an intermediate local variable to retain the view until it's added
as subview.

BUG=624363
TEST=None
==========

to

==========
[ObjC ARC] Converts ios/chrome/browser/ui/history:history to ARC.

Semi-autogenerated ARCMigrate commit.
Notable issues:
 * Since one of the weak_nsobjects is exposed in a header and I change it to a
__weak instead, converted two unit tests to ARC alongside this.
 * Fixed some assignments of newly created views to weak properties by
introducing an intermediate local variable to retain the view until it's added
as subview.

BUG=624363
TEST=None

Review-Url: https://codereview.chromium.org/2624963003
Cr-Commit-Position: refs/heads/master@{#443240}
Committed:
https://chromium.googlesource.com/chromium/src/+/b103d6c66bbd7bebfe22affab2ea...
==========

[commit-bot: I haz the power, 2017-01-12 15:51:08]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/b103d6c66bbd7bebfe22affab2ea...