Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2854133002:
[ios] ARCMigrate ios/chrome/browser/tabs/tab_model.mm to ARC. (Closed)
|
Created:
3 years, 7 months ago by sdefresne Modified:
3 years, 7 months ago CC:
chromium-reviews, marq+watch_chromium.org, ios-reviews+chrome_chromium.org, noyau+watch_chromium.org, ios-reviews_chromium.org, pkl (ping after 24h if needed) Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
Description[ios] ARCMigrate ios/chrome/browser/tabs/tab_model.mm to ARC.
Semi-automatic conversion of ios/chrome/browser/tabs/tab_model.mm
to ARC (Automatic Reference Counting).
Manual changes:
- added a @autoreleasepool in -postNotificationName:withTab: to
respect the old behaviour that the method did not extend the
lifetime of the Tab object (by ensuring the NSDictionary is
deallocated by the end of the method)
- merge "tabs_internal" and "tabs_internal_arc" targets.
BUG=None
Review-Url: https://codereview.chromium.org/2854133002
Cr-Commit-Position: refs/heads/master@{#471723}
Committed: https://chromium.googlesource.com/chromium/src/+/4162a4dc23269a244e6a007e1126eeb4ad45ebf0
Patch Set 1 #
Total comments: 4
Patch Set 2 : Address comments. #
Total comments: 1
Patch Set 3 : Rebase #Patch Set 4 : Rebase #Patch Set 5 : Rebase #Messages
Total messages: 28 (20 generated)
The CQ bit was checked by sdefresne@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
sdefresne@chromium.org changed reviewers: + rohitrao@chromium.org, stkhapugin@chromium.org
rohitrao: please take a look. stkhapugin: FYI
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_cronet on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...) linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
lgtm https://codereview.chromium.org/2854133002/diff/1/ios/chrome/browser/tabs/tab... File ios/chrome/browser/tabs/tab_model.mm (right): https://codereview.chromium.org/2854133002/diff/1/ios/chrome/browser/tabs/tab... ios/chrome/browser/tabs/tab_model.mm:228: nit: remove newline https://codereview.chromium.org/2854133002/diff/1/ios/chrome/browser/tabs/tab... ios/chrome/browser/tabs/tab_model.mm:704: @autoreleasepool { Thank you very much for doing this. If you also know what is the reason for this tab lifetime requirement, can you please file a bug and leave a todo here to untangle this?
https://codereview.chromium.org/2854133002/diff/1/ios/chrome/browser/tabs/tab... File ios/chrome/browser/tabs/tab_model.mm (right): https://codereview.chromium.org/2854133002/diff/1/ios/chrome/browser/tabs/tab... ios/chrome/browser/tabs/tab_model.mm:228: On 2017/05/03 11:56:12, stkhapugin wrote: > nit: remove newline Done. https://codereview.chromium.org/2854133002/diff/1/ios/chrome/browser/tabs/tab... ios/chrome/browser/tabs/tab_model.mm:704: @autoreleasepool { On 2017/05/03 11:56:12, stkhapugin wrote: > Thank you very much for doing this. If you also know what is the reason for this > tab lifetime requirement, can you please file a bug and leave a todo here to > untangle this? This method was never invoked, so instead I just removed it.
The CQ bit was checked by sdefresne@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
lgtm https://codereview.chromium.org/2854133002/diff/20001/ios/chrome/browser/tabs... File ios/chrome/browser/tabs/tab_model.mm (right): https://codereview.chromium.org/2854133002/diff/20001/ios/chrome/browser/tabs... ios/chrome/browser/tabs/tab_model.mm:668: objects:(id __unsafe_unretained*)objects Why is this change necessary?
CL description can be updates now that postNotification is gone.
The CQ bit was checked by sdefresne@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by sdefresne@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from stkhapugin@chromium.org, rohitrao@chromium.org Link to the patchset: https://codereview.chromium.org/2854133002/#ps60001 (title: "Rebase")
The CQ bit was unchecked by commit-bot@chromium.org
This CL has an open dependency (Issue 2845353002 Patch 20001). Please resolve the dependency and try again. If you are sure that there is no real dependency, please use one of the options listed in https://goo.gl/9Es4OR to land the CL.
The CQ bit was checked by sdefresne@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from stkhapugin@chromium.org, rohitrao@chromium.org Link to the patchset: https://codereview.chromium.org/2854133002/#ps80001 (title: "Rebase")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 80001, "attempt_start_ts": 1494841317789420,
"parent_rev": "f723e76be72fbbf475f4f0017a2191d986315428", "commit_rev":
"4162a4dc23269a244e6a007e1126eeb4ad45ebf0"}
Message was sent while issue was closed.
Description was changed from
==========
[ios] ARCMigrate ios/chrome/browser/tabs/tab_model.mm to ARC.
Semi-automatic conversion of ios/chrome/browser/tabs/tab_model.mm
to ARC (Automatic Reference Counting).
Manual changes:
- added a @autoreleasepool in -postNotificationName:withTab: to
respect the old behaviour that the method did not extend the
lifetime of the Tab object (by ensuring the NSDictionary is
deallocated by the end of the method)
- merge "tabs_internal" and "tabs_internal_arc" targets.
BUG=None
==========
to
==========
[ios] ARCMigrate ios/chrome/browser/tabs/tab_model.mm to ARC.
Semi-automatic conversion of ios/chrome/browser/tabs/tab_model.mm
to ARC (Automatic Reference Counting).
Manual changes:
- added a @autoreleasepool in -postNotificationName:withTab: to
respect the old behaviour that the method did not extend the
lifetime of the Tab object (by ensuring the NSDictionary is
deallocated by the end of the method)
- merge "tabs_internal" and "tabs_internal_arc" targets.
BUG=None
Review-Url: https://codereview.chromium.org/2854133002
Cr-Commit-Position: refs/heads/master@{#471723}
Committed:
https://chromium.googlesource.com/chromium/src/+/4162a4dc23269a244e6a007e1126...
==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/4162a4dc23269a244e6a007e1126... |
