Eliminate CHECK from CryptohomeLibrary::LoadSystemSalt

chrome/browser/renderer_host/pepper/device_id_fetcher.cc

 // Copyright (c) 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/renderer_host/pepper/device_id_fetcher.h"
 
 #include "base/file_util.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/string_number_conversions.h"
 #include "chrome/browser/profiles/profile.h"
@@ skipping 18 matching lines @@
 using content::RenderProcessHost;
 
 namespace chrome {
 
 namespace {
 
 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)
     : in_progress_(false),
       render_process_id_(render_process_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 }
 
 DeviceIDFetcher::~DeviceIDFetcher() {
 }
@@ skipping 24 matching lines @@
       "",
       user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
 }
 
 // static
 base::FilePath DeviceIDFetcher::GetLegacyDeviceIDPath(
     const base::FilePath& profile_path) {
   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));
 
   Profile* profile = NULL;
   RenderProcessHost* render_process_host =
       RenderProcessHost::FromID(render_process_id_);
   if (render_process_host && render_process_host->GetBrowserContext()) {
     profile = Profile::FromBrowserContext(
         render_process_host->GetBrowserContext());
   }
@@ skipping 11 matching lines @@
     uint8_t salt_bytes[kSaltLength];
     crypto::RandBytes(salt_bytes, arraysize(salt_bytes));
     // Since it will be stored in a string pref, convert it to hex.
     salt = base::HexEncode(salt_bytes, arraysize(salt_bytes));
     profile->GetPrefs()->SetString(prefs::kDRMSalt, salt);
   }
 
 #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) {
+  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::vector<uint8> salt_bytes;
   if (!base::HexStringToBytes(salt, &salt_bytes))
     salt_bytes.clear();
-
-  // 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();
+  if (salt_bytes.size() != kSaltLength) {
+    LOG(ERROR) << "Unexpected salt bytes length: " << salt_bytes.size();
     RunCallbackOnIOThread(std::string());
     return;
   }
 
   char id_buf[256 / 8];  // 256-bits for SHA256
   std::string input = machine_id;
   input.append(kDRMIdentifierFile);
   input.append(salt_bytes.begin(), salt_bytes.end());
   crypto::SHA256HashString(input, &id_buf, sizeof(id_buf));
   std::string id = StringToLowerASCII(
       base::HexEncode(reinterpret_cast<const void*>(id_buf), sizeof(id_buf)));
   input = machine_id;
   input.append(kDRMIdentifierFile);
   input.append(id);
   crypto::SHA256HashString(input, &id_buf, sizeof(id_buf));
   id = StringToLowerASCII(base::HexEncode(
         reinterpret_cast<const void*>(id_buf),
         sizeof(id_buf)));
 
   RunCallbackOnIOThread(id);
 }
 
 // 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.
   base::FilePath id_path = GetLegacyDeviceIDPath(profile_path);
   if (base::PathExists(id_path)) {
     if (base::ReadFileToString(id_path, &id) && !id.empty()) {
       RunCallbackOnIOThread(id);
       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(
         BrowserThread::IO, FROM_HERE,
         base::Bind(&DeviceIDFetcher::RunCallbackOnIOThread, this, id));
     return;
   }
   in_progress_ = false;
   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