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

Issue 2455503007: [ios] Adds VoiceSearchBar creation methods to VoiceSearchProvider. (Closed)

Created:
4 years, 1 month ago by rohitrao (ping after 24h)
Modified:
4 years, 1 month ago
Reviewers:
kkhorimoto, sdefresne
CC:
chromium-reviews, mac-reviews_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[ios] Adds VoiceSearchBar creation methods to VoiceSearchProvider. BUG=656617 Committed: https://crrev.com/e716b15586e831dd2939fd301127e7398f71ad03 Cr-Commit-Position: refs/heads/master@{#428076}

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+11 lines, -0 lines) Patch
M ios/public/provider/chrome/browser/voice/voice_search_provider.h View 2 chunks +6 lines, -0 lines 2 comments Download
M ios/public/provider/chrome/browser/voice/voice_search_provider.mm View 1 chunk +5 lines, -0 lines 0 comments Download

Messages

Total messages: 12 (4 generated)
rohitrao (ping after 24h)
4 years, 1 month ago (2016-10-27 15:32:14 UTC) #2
rohitrao (ping after 24h)
+kkhorimoto
4 years, 1 month ago (2016-10-27 16:54:38 UTC) #4
kkhorimoto
lgtm https://codereview.chromium.org/2455503007/diff/1/ios/public/provider/chrome/browser/voice/voice_search_provider.h File ios/public/provider/chrome/browser/voice/voice_search_provider.h (right): https://codereview.chromium.org/2455503007/diff/1/ios/public/provider/chrome/browser/voice/voice_search_provider.h#newcode41 ios/public/provider/chrome/browser/voice/voice_search_provider.h:41: NS_RETURNS_RETAINED; is NS_RETURNS_RETAINED an ARC-related annotation? It might ...
4 years, 1 month ago (2016-10-27 17:22:48 UTC) #5
rohitrao (ping after 24h)
https://codereview.chromium.org/2455503007/diff/1/ios/public/provider/chrome/browser/voice/voice_search_provider.h File ios/public/provider/chrome/browser/voice/voice_search_provider.h (right): https://codereview.chromium.org/2455503007/diff/1/ios/public/provider/chrome/browser/voice/voice_search_provider.h#newcode41 ios/public/provider/chrome/browser/voice/voice_search_provider.h:41: NS_RETURNS_RETAINED; On 2016/10/27 17:22:47, kkhorimoto_ wrote: > is NS_RETURNS_RETAINED ...
4 years, 1 month ago (2016-10-27 17:47:29 UTC) #6
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/2455503007/1
4 years, 1 month ago (2016-10-27 17:48:21 UTC) #8
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 1 month ago (2016-10-27 18:03:50 UTC) #9
commit-bot: I haz the power
Patchset 1 (id:??) landed as https://crrev.com/e716b15586e831dd2939fd301127e7398f71ad03 Cr-Commit-Position: refs/heads/master@{#428076}
4 years, 1 month ago (2016-10-27 18:30:40 UTC) #11
sdefresne
4 years, 1 month ago (2016-10-28 11:41:42 UTC) #12
Message was sent while issue was closed.
On 2016/10/27 17:47:29, rohitrao wrote:
>
https://codereview.chromium.org/2455503007/diff/1/ios/public/provider/chrome/...
> File ios/public/provider/chrome/browser/voice/voice_search_provider.h (right):
> 
>
https://codereview.chromium.org/2455503007/diff/1/ios/public/provider/chrome/...
> ios/public/provider/chrome/browser/voice/voice_search_provider.h:41:
> NS_RETURNS_RETAINED;
> On 2016/10/27 17:22:47, kkhorimoto_ wrote:
> > is NS_RETURNS_RETAINED an ARC-related annotation?  It might be worthwhile
> > sending out a PSA on when to use these for interfacing between C++ and ARC
> code
> 
> I've been cargo-culting it.  Will check with stk about sending out a PSA.  I
> think it's helpful for him as he manually migrates files, but it may not
> directly affect correctness?

It does affect correctness. Without this annotation, ARC assumes that the caller
does not owns the returned reference (see
https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/Me...)
and will not call release on it thus causing a leak.

It is only important if the caller or callee is compiled with ARC while the
other is not. If both are compiled with ARC or without ARC, then it is not
really an issue (as the refcount is either not managed by ARC or fully managed
by ARC).

Powered by Google App Engine
This is Rietveld 408576698