Eliminate CHECK from CryptohomeLibrary::LoadSystemSalt

chrome/browser/renderer_host/pepper/device_id_fetcher.cc

Index: chrome/browser/renderer_host/pepper/device_id_fetcher.cc
diff --git a/chrome/browser/renderer_host/pepper/device_id_fetcher.cc b/chrome/browser/renderer_host/pepper/device_id_fetcher.cc
index 585d62d19867b1f376748584d5f65a52931cc22b..7e666908337f39a0053098f4aa724a122cd58cb4 100644
--- a/chrome/browser/renderer_host/pepper/device_id_fetcher.cc
+++ b/chrome/browser/renderer_host/pepper/device_id_fetcher.cc
@@ -36,6 +36,28 @@ const char kDRMIdentifierFile[] = "Pepper DRM ID.0";
 
 const uint32_t kSaltLength = 32;
 
+void GetMachineIDAsync(const DeviceIDFetcher::IDCallback& callback) {
+  std::string result;
+#if defined(OS_WIN) && defined(ENABLE_RLZ)
+  rlz_lib::GetMachineId(&result);
+#elif defined(OS_CHROMEOS)
+  result = chromeos::CryptohomeLibrary::Get()->GetSystemSalt();
+  if (result.empty()) {

[zel, 2013/09/20 16:28:05]

this logic should move to async version of CryptohomeLibrary::GetSystemSalt()

[stevenjb, 2013/09/20 16:42:23]

Agreed. This will require some significant changes to the login code however.

+    // cryptohome must not be running; re-request after a delay.
+    const int64 kRequestSystemSaltDelayMs = 500;
+    base::MessageLoop::current()->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&GetMachineIDAsync, callback),
+        base::TimeDelta::FromMilliseconds(kRequestSystemSaltDelayMs));
+    return;
+  }
+#else
+  // Not implemented for other platforms.
+  NOTREACHED();
+#endif
+  callback.Run(result);
+}
+
 }  // namespace
 
 DeviceIDFetcher::DeviceIDFetcher(int render_process_id)
@@ -80,8 +102,6 @@ base::FilePath DeviceIDFetcher::GetLegacyDeviceIDPath(
   return profile_path.AppendASCII(kDRMIdentifierFile);
 }
 
-// TODO(raymes): Change this to just return the device id salt and call it with
-// PostTaskAndReply once the legacy ChromeOS codepath is removed.
 void DeviceIDFetcher::CheckPrefsOnUIThread() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
@@ -113,27 +133,35 @@ void DeviceIDFetcher::CheckPrefsOnUIThread() {
 #if defined(OS_CHROMEOS)
   // Try the legacy path first for ChromeOS. We pass the new salt in as well
   // in case the legacy id doesn't exist.
-  BrowserThread::PostBlockingPoolTask(FROM_HERE,
-      base::Bind(&DeviceIDFetcher::ComputeOnBlockingPool, this,
+  BrowserThread::PostBlockingPoolTask(
+      FROM_HERE,
+      base::Bind(&DeviceIDFetcher::LegacyComputeOnBlockingPool,
+                 this,
                  profile->GetPath(), salt));
 #else
-  BrowserThread::PostTask(
-      BrowserThread::IO, FROM_HERE,
-      base::Bind(&DeviceIDFetcher::ComputeOnIOThread, this, salt));
+  // Get the machine ID and call ComputeOnUIThread with salt + machine_id.
+  GetMachineIDAsync(base::Bind(&DeviceIDFetcher::ComputeOnUIThread,
+                               this, salt));
 #endif
 }
 
+void DeviceIDFetcher::ComputeOnUIThread(const std::string& salt,
+                                        const std::string& machine_id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-void DeviceIDFetcher::ComputeOnIOThread(const std::string& salt) {
-  std::vector<uint8> salt_bytes;
-  if (!base::HexStringToBytes(salt, &salt_bytes))
-    salt_bytes.clear();
+  if (machine_id.empty()) {
+    LOG(ERROR) << "Empty machine id";
+    RunCallbackOnIOThread(std::string());
+    return;
+  }
 
   // Build the identifier as follows:
   // SHA256(machine-id||service||SHA256(machine-id||service||salt))
-  std::string machine_id = GetMachineID();
-  if (machine_id.empty() || salt_bytes.size() != kSaltLength) {
-    NOTREACHED();
+  std::vector<uint8> salt_bytes;
+  if (!base::HexStringToBytes(salt, &salt_bytes))
+    salt_bytes.clear();
+  if (salt_bytes.size() != kSaltLength) {
+    LOG(ERROR) << "Unexpected salt bytes length: " << salt_bytes.size();
     RunCallbackOnIOThread(std::string());
     return;
   }
@@ -159,8 +187,9 @@ void DeviceIDFetcher::ComputeOnIOThread(const std::string& salt) {
 // TODO(raymes): This is temporary code to migrate ChromeOS devices to the new
 // scheme for generating device IDs. Delete this once we are sure most ChromeOS
 // devices have been migrated.
-void DeviceIDFetcher::ComputeOnBlockingPool(const base::FilePath& profile_path,
-                                            const std::string& salt) {
+void DeviceIDFetcher::LegacyComputeOnBlockingPool(
+    const base::FilePath& profile_path,
+    const std::string& salt) {
   std::string id;
   // First check if the legacy device ID file exists on ChromeOS. If it does, we
   // should just return that.
@@ -171,13 +200,15 @@ void DeviceIDFetcher::ComputeOnBlockingPool(const base::FilePath& profile_path,
       return;
     }
   }
-  // If we didn't find an ID, go back to the new code path to generate an ID.
+  // If we didn't find an ID, get the machine ID and call the new code path to
+  // generate an ID.
   BrowserThread::PostTask(
-      BrowserThread::IO, FROM_HERE,
-      base::Bind(&DeviceIDFetcher::ComputeOnIOThread, this, salt));
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&GetMachineIDAsync,
+                 base::Bind(&DeviceIDFetcher::ComputeOnUIThread,
+                            this, salt)));
 }
 
-
 void DeviceIDFetcher::RunCallbackOnIOThread(const std::string& id) {
   if (!BrowserThread::CurrentlyOn(BrowserThread::IO)) {
     BrowserThread::PostTask(
@@ -189,19 +220,4 @@ void DeviceIDFetcher::RunCallbackOnIOThread(const std::string& id) {
   callback_.Run(id);
 }
 
-std::string DeviceIDFetcher::GetMachineID() {
-#if defined(OS_WIN) && defined(ENABLE_RLZ)
-  std::string result;
-  rlz_lib::GetMachineId(&result);
-  return result;
-#elif defined(OS_CHROMEOS)
-  chromeos::CryptohomeLibrary* c_home = chromeos::CryptohomeLibrary::Get();
-  return c_home->GetSystemSalt();
-#else
-  // Not implemented for other platforms.
-  NOTREACHED();
-  return "";
-#endif
-}
-
 }  // namespace chrome