Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(189)

Issue 2513003002: [ObjC ARC] Converts ios/chrome/browser/ui/autofill:autofill to ARC.Automatically generated ARCMig… (Closed)

Created:
4 years, 1 month ago by stkhapugin
Modified:
4 years ago
Reviewers:
Justin Donnelly
CC:
chromium-reviews, rouslan+autofill_chromium.org, sebsg+autofillwatch_chromium.org, pkl (ping after 24h if needed), jdonnelly+autofillwatch_chromium.org, mathp+autofillwatch_chromium.org, mac-reviews_chromium.org, vabr+watchlistautofill_chromium.org, estade+watch_chromium.org, sdefresne+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[ObjC ARC] Converts ios/chrome/browser/ui/autofill:autofill to ARC. Automatically generated ARCMigrate commit Notable issues:None BUG=624363 TEST=None Committed: https://crrev.com/bb77ce777e93d29d2bd6969e70da4c11708c1c5a Cr-Commit-Position: refs/heads/master@{#435285}

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+9 lines, -0 lines) Patch
M ios/chrome/browser/ui/autofill/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M ios/chrome/browser/ui/autofill/autofill_client_ios.mm View 1 chunk +4 lines, -0 lines 1 comment Download
M ios/chrome/browser/ui/autofill/autofill_save_card_infobar.mm View 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 16 (8 generated)
stkhapugin
PTAL. Passes downstream tests.
4 years, 1 month ago (2016-11-18 18:22:35 UTC) #4
Justin Donnelly
Sorry for the slow response, I was on vacation. This change looks good in principle ...
4 years ago (2016-11-28 18:10:19 UTC) #7
Justin Donnelly
Looking at the other changes in this bug, it appears that this is the expected ...
4 years ago (2016-11-29 17:22:43 UTC) #8
stkhapugin
Thanks! Yes, this is expected to set arc_enabled for C++ classes. If it wasn't for ...
4 years ago (2016-11-30 15:31:43 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2513003002/1
4 years ago (2016-11-30 15:32:15 UTC) #11
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years ago (2016-11-30 16:28:34 UTC) #13
commit-bot: I haz the power
Patchset 1 (id:??) landed as https://crrev.com/bb77ce777e93d29d2bd6969e70da4c11708c1c5a Cr-Commit-Position: refs/heads/master@{#435285}
4 years ago (2016-11-30 16:31:29 UTC) #15
sdefresne
4 years ago (2016-11-30 18:02:28 UTC) #16
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/2543683002/ by sdefresne@chromium.org.

The reason for reverting is: This break the roll because
id<AutofillClientIOSBridge> was a weak reference and is now turned into a strong
reference.

Since the id<AutofillClientIOSBridge> owns the AutofillClientIOS in the
downstream tree this causes a reference cycle and cause a
SigninManagerBase::Observer to survive the destruction of SigninManagerBase..

Powered by Google App Engine
This is Rietveld 408576698