Fixing identifier for device returning a 0 identifierForVendor. (issue 11576008)

[q...@chromium.org]

Reviewers: stuartmorgan,

Description:
Fixing identifier for device returning a 0 identifierForVendor.

R=stuart...@chromium.org


Please review this at https://codereview.chromium.org/11576008/

SVN Base: svn://svn.chromium.org/chrome/trunk/src

Affected files:
M base/ios/device_util.mm
M base/ios/device_util_unittest.mm


Index: base/ios/device_util.mm
diff --git a/base/ios/device_util.mm b/base/ios/device_util.mm
index
e6801abe8466f1bef86aa71241dc80cd2c94a86f..b538ea858a5558987e268e2e7aed78ecaee805a0
100644
--- a/base/ios/device_util.mm
+++ b/base/ios/device_util.mm
@@ -24,9 +24,33 @@
namespace {

// Client ID key in the user preferences.
-NSString* const kClientIdPreferenceKey = @"ChromiumClientID";
+NSString* const kLegacyClientIdPreferenceKey = @"ChromiumClientID";
+NSString* const kClientIdPreferenceKey = @"ChromeClientID";
// Default salt for device ids.
const char kDefaultSalt[] = "Salt";
+// Zero UUID returned on buggy iOS devices.
+NSString* const kZeroUUID = @"00000000-0000-0000-0000-000000000000";
+
+NSString* GenerateClientId() {
+ NSUserDefaults* defaults = [NSUserDefaults standardUserDefaults];
+
+ // Try to migrate from legacy client id.
+ NSString* client_id = [defaults
stringForKey:kLegacyClientIdPreferenceKey];
+
+ // Some iOS6 devices return a buggy identifierForVendor:
+ // http://openradar.appspot.com/12377282. If this is the case, revert to
+ // generating a new one.
+ if (!client_id || [client_id isEqualToString:kZeroUUID]) {
+ if (base::ios::IsRunningOnIOS6OrLater()) {
+ client_id = [[[UIDevice currentDevice] identifierForVendor]
UUIDString];
+ if ([client_id isEqualToString:kZeroUUID])
+ client_id =
base::SysUTF8ToNSString(ios::device_util::GetRandomId());
+ } else {
+ client_id = base::SysUTF8ToNSString(ios::device_util::GetRandomId());
+ }
+ }
+ return client_id;
+}

} // namespace

@@ -104,10 +128,7 @@ std::string GetDeviceIdentifier(const char* salt) {
NSString* client_id = [defaults stringForKey:kClientIdPreferenceKey];

if (!client_id) {
- if (base::ios::IsRunningOnIOS6OrLater())
- client_id = [[[UIDevice currentDevice] identifierForVendor]
UUIDString];
- else
- client_id = base::SysUTF8ToNSString(GetRandomId());
+ client_id = GenerateClientId();
[defaults setObject:client_id forKey:kClientIdPreferenceKey];
[defaults synchronize];
}
Index: base/ios/device_util_unittest.mm
diff --git a/base/ios/device_util_unittest.mm
b/base/ios/device_util_unittest.mm
index
b8ba10f289d365d9d805e008882af88a29e7d2a4..90e69f71e6d323b0cbf9e93c6027c3d1a1137c08
100644
--- a/base/ios/device_util_unittest.mm
+++ b/base/ios/device_util_unittest.mm
@@ -44,7 +44,7 @@ TEST_F(DeviceUtilTest, GetDeviceIdentifier) {
EXPECT_NE(default_id, other_id);

NSUserDefaults* defaults = [NSUserDefaults standardUserDefaults];
- [defaults removeObjectForKey:@"ChromiumClientID"];
+ [defaults removeObjectForKey:@"ChromeClientID"];
[defaults synchronize];

std::string new_default_id = ios::device_util::GetDeviceIdentifier(NULL);
@@ -54,4 +54,38 @@ TEST_F(DeviceUtilTest, GetDeviceIdentifier) {
EXPECT_NE(default_id, new_default_id);
}

+TEST_F(DeviceUtilTest, CheckMigration) {
+ NSUserDefaults* defaults = [NSUserDefaults standardUserDefaults];
+ [defaults setObject:@"10000000-0000-0000-0000-000000000000"
+ forKey:@"ChromeClientID"];
+ [defaults synchronize];
+ std::string expected_id = ios::device_util::GetDeviceIdentifier(NULL);
+ [defaults removeObjectForKey:@"ChromeClientID"];
+ [defaults setObject:@"10000000-0000-0000-0000-000000000000"
+ forKey:@"ChromiumClientID"];
+ [defaults synchronize];
+ std::string new_id = ios::device_util::GetDeviceIdentifier(NULL);
+ EXPECT_EQ(expected_id, new_id);
+ [defaults removeObjectForKey:@"ChromeClientID"];
+ [defaults removeObjectForKey:@"ChromiumClientID"];
+ [defaults synchronize];
+}
+
+TEST_F(DeviceUtilTest, CheckMigrationFromZero) {
+ NSUserDefaults* defaults = [NSUserDefaults standardUserDefaults];
+ [defaults setObject:@"00000000-0000-0000-0000-000000000000"
+ forKey:@"ChromeClientID"];
+ [defaults synchronize];
+ std::string zero_id = ios::device_util::GetDeviceIdentifier(NULL);
+ [defaults removeObjectForKey:@"ChromeClientID"];
+ [defaults setObject:@"00000000-0000-0000-0000-000000000000"
+ forKey:@"ChromiumClientID"];
+ [defaults synchronize];
+ std::string new_id = ios::device_util::GetDeviceIdentifier(NULL);
+ EXPECT_NE(zero_id, new_id);
+ [defaults removeObjectForKey:@"ChromeClientID"];
+ [defaults removeObjectForKey:@"ChromiumClientID"];
+ [defaults synchronize];
+}
+
} // namespace

[stuart...@chromium.org]

LGTM

https://codereview.chromium.org/11576008/

[commi...@chromium.org]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/q...@chromium.org/11576008/1


https://chromiumcodereview.appspot.com/11576008/

[commi...@chromium.org]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/q...@chromium.org/11576008/6005


https://chromiumcodereview.appspot.com/11576008/

[commi...@chromium.org]

Change committed as 172888

https://chromiumcodereview.appspot.com/11576008/