Implement a salt for MediaSource IDs that can be cleared by a user.

chrome/browser/media/media_device_id_salt.cc

+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+#include "chrome/browser/media/media_device_id_salt.h"
+
+#include "base/logging.h"
+#include "base/prefs/pref_service.h"
+#include "base/rand_util.h"
+#include "chrome/browser/profiles/profile_io_data.h"
+#include "chrome/common/pref_names.h"
+#include "content/public/browser/browser_thread.h"
+
+using content::BrowserThread;
+
+namespace {
+
+std::string CreateSalt() {

[Jói, 2013/10/31 16:14:47]

I would be surprised if we don't already have a function somewhere to create a
random string. Maybe in //crypto?

[perkj_chrome, 2013/10/31 17:05:44]

Yes- but it turned out that crypto::RandBytes in random.h returns something that
can not be stored in UTF8. |media_device_id_salt_| got very sad then.

[Jói, 2013/10/31 17:19:34]

OK.

+  const size_t kSaltLength = 8;
+  static const char kAlphabet[] = "0123456789"
+        "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
+
+  std::string salt(kSaltLength, ' ');
+  for (size_t i = 0; i < salt.size(); ++i) {
+    int random_char = base::RandInt(0, sizeof(kAlphabet) - 1);
+    salt[i] = kAlphabet[random_char];
+  }
+  return salt;
+}
+
+}  // namespace
+
+MediaDeviceIDSalt::MediaDeviceIDSalt(PrefService* pref_service,
+                                     bool incognito) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  if (!incognito) {
+    media_device_id_salt_.Init(prefs::kMediaDeviceIdSalt, pref_service,
+                               base::Bind(&MediaDeviceIDSalt::OnValueReset,
+                                          base::Unretained(this)));
+    salt_ = media_device_id_salt_.GetValue();
+  }
+
+  if (salt_.empty()) {
+    salt_ = CreateSalt();
+  }
+
+  if (!incognito) {
+    media_device_id_salt_.SetValue(salt_);
+  }
+}
+
+MediaDeviceIDSalt::~MediaDeviceIDSalt() {
+}
+
+const std::string& MediaDeviceIDSalt::GetSalt() const {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DVLOG(1) << "GetSalt " << salt_;
+  return salt_;
+}
+
+void MediaDeviceIDSalt::RegisterProfilePrefs(
+    user_prefs::PrefRegistrySyncable* registry) {
+  registry->RegisterStringPref(
+      prefs::kMediaDeviceIdSalt,
+      std::string(),
+      user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
+}
+
+void MediaDeviceIDSalt::Reset(PrefService* pref_service) {

[Jói, 2013/10/31 16:14:47]

I was a bit surprised this was static, but I see now how you're doing it. Seems
fine by me; the other approach would have been to have Reset be a member
variable, and have BrowsingDataRemover look up the MediaDeviceIDSalt object on
which to call Reset from the Profile for which it is clearing data. That way,
you wouldn't have to have a StringPrefMember, you would just call
pref_service->ClearPref and pref_service->SetPref in your Reset member when
setting the simple string member salt_.

[perkj_chrome, 2013/10/31 17:05:44]

Sure- that would simplify things even more.
How would I find the MediaDeviceIDSalt object? Can I get to the ProfielIOData
object from a Profile* ?

[Jói, 2013/10/31 17:19:34]

ProfileIOData::FromResourceContext (and you can do profile->GetResourceContext()
since it is a BrowserContext).

+  pref_service->ClearPref(prefs::kMediaDeviceIdSalt);
+}
+
+void MediaDeviceIDSalt::OnValueReset(const std::string& name) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DVLOG(1) << "OnValueReset " << name;
+  media_device_id_salt_.SetValue(CreateSalt());
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&MediaDeviceIDSalt::OnValueResetOnIOThread,
+                 base::Unretained(this), media_device_id_salt_.GetValue()));

[perkj_chrome, 2013/10/31 15:31:48]

Is Unretained OK here or should MediaDeviceSalt be ref counted?

[Jói, 2013/10/31 16:14:47]

Good question. I think it may be OK since probably any message posted from UI to
IO to destroy the IO side of ProfileIOData wouldn't have been posted by this
point, so your message would be processed first, so the IO side of ProfileIOData
would still be guaranteed to exist. I'm not 100% sure though, would need to look
through the code, and this is a guarantee that won't necessarily hold when code
changes.

I'm starting to think we should just use a lock to protect access to the salt_
member and access it directly from either UI or IO threads. There will almost
never be contention for accessing it (the only time it might possibly be
accessed around the same time by multiple threads is when you're resetting it)
so the lock would be very close to free.

+}
+
+void MediaDeviceIDSalt::OnValueResetOnIOThread(const std::string salt) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DVLOG(1) << "OnValueResetOnIOThread " << salt;
+  salt_ = salt;
+}