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

Issue 29873005: Implements RLZ lib for iOS (Closed)

Created:
7 years, 2 months ago by sdefresne
Modified:
7 years, 2 months ago
CC:
chromium-reviews
Visibility:
Public.

Description

Implements RLZ lib for iOS Use the OS X implementation of the RlzValueStore and derive the machine identifier from -[UIDevice identifierForVendor]. BUG=309629 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=230429

Patch Set 1 #

Patch Set 2 : Implements RLZ on iOS #

Patch Set 3 : Use correct access points enums on iOS #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+42 lines, -4 lines) Patch
M chrome/browser/rlz/rlz.cc View 1 2 3 chunks +10 lines, -0 lines 1 comment Download
A rlz/ios/lib/machine_id_ios.cc View 1 chunk +17 lines, -0 lines 0 comments Download
M rlz/lib/lib_values.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M rlz/lib/rlz_enums.h View 1 chunk +3 lines, -2 lines 0 comments Download
M rlz/rlz.gyp View 2 chunks +10 lines, -0 lines 0 comments Download

Messages

Total messages: 12 (0 generated)
sdefresne
7 years, 2 months ago (2013-10-22 14:47:25 UTC) #1
Nico
You checked that rlz/lib/recursive_cross_process_lock_posix.cc works on iOS, yes? And NSSearchPathForDirectoriesInDomains() works on iOS too?
7 years, 2 months ago (2013-10-22 15:19:13 UTC) #2
sdefresne
On 2013/10/22 15:19:13, Nico wrote: > You checked that rlz/lib/recursive_cross_process_lock_posix.cc works on iOS, > yes? ...
7 years, 2 months ago (2013-10-22 16:27:53 UTC) #3
Nico
lgtm Roger should approve this too though.
7 years, 2 months ago (2013-10-22 16:31:14 UTC) #4
Roger Tawa OOO till Jul 10th
lgtm, but you must also make a change to chrome\browser\rlz\rlz.cc, around line 161, so that ...
7 years, 2 months ago (2013-10-22 16:56:24 UTC) #5
sdefresne
On 2013/10/22 16:56:24, Roger Tawa wrote: > lgtm, but you must also make a change ...
7 years, 2 months ago (2013-10-23 09:10:01 UTC) #6
Roger Tawa OOO till Jul 10th
Lgtm
7 years, 2 months ago (2013-10-23 12:25:37 UTC) #7
sdefresne
On 2013/10/23 12:25:37, Roger Tawa wrote: > Lgtm Thank you. I'm not a committer, so ...
7 years, 2 months ago (2013-10-23 12:29:32 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sdefresne@chromium.org/29873005/260001
7 years, 2 months ago (2013-10-23 12:34:16 UTC) #9
commit-bot: I haz the power
Change committed as 230429
7 years, 2 months ago (2013-10-23 15:09:39 UTC) #10
Nico
https://codereview.chromium.org/29873005/diff/260001/chrome/browser/rlz/rlz.cc File chrome/browser/rlz/rlz.cc (right): https://codereview.chromium.org/29873005/diff/260001/chrome/browser/rlz/rlz.cc#newcode150 chrome/browser/rlz/rlz.cc:150: rlz_lib::CHROME_IOS_OMNIBOX; Does iOS use this class at all?
7 years, 2 months ago (2013-10-23 16:58:27 UTC) #11
sdefresne
7 years, 2 months ago (2013-10-24 08:50:38 UTC) #12
Message was sent while issue was closed.
On 2013/10/23 16:58:27, Nico wrote:
> https://codereview.chromium.org/29873005/diff/260001/chrome/browser/rlz/rlz.cc
> File chrome/browser/rlz/rlz.cc (right):
> 
>
https://codereview.chromium.org/29873005/diff/260001/chrome/browser/rlz/rlz.c...
> chrome/browser/rlz/rlz.cc:150: rlz_lib::CHROME_IOS_OMNIBOX;
> Does iOS use this class at all?

Not yet, but it will end up using it.

Powered by Google App Engine
This is Rietveld 408576698