Cros: Fix misleading warnings about machine id.

Drop "requested statistic not found" warnings when probing for the
existance of a statistic or a flag using HasMachine*().

To that end, allow GetMachine{Statistic,Flag}() to be called with
nullptr |result| to achieve HasMachine{Statistic,Flag}() semantics
and in that case suppress the logging of warnings about non-existant
keys.

BUG=545909

Review URL: https://codereview.chromium.org/1408133005

Cr-Commit-Position: refs/heads/master@{#355502}

4 files changed, 36 insertions, 48 deletions

diff --git a/chromeos/system/fake_statistics_provider.cc b/chromeos/system/fake_statistics_provider.cc
index 0f264c4..e9f530b 100644
--- a/chromeos/system/fake_statistics_provider.cc
+++ b/chromeos/system/fake_statistics_provider.cc
@@ -27,10 +27,6 @@ bool FakeStatisticsProvider::GetMachineStatistic(const std::string& name,
   return match != machine_statistics_.end();
 }
 
-bool FakeStatisticsProvider::HasMachineStatistic(const std::string& name) {
-  return machine_statistics_.find(name) != machine_statistics_.end();
-}
-
 bool FakeStatisticsProvider::GetMachineFlag(const std::string& name,
                                             bool* result) {
   std::map<std::string, bool>::const_iterator match = machine_flags_.find(name);
@@ -39,10 +35,6 @@ bool FakeStatisticsProvider::GetMachineFlag(const std::string& name,
   return match != machine_flags_.end();
 }
 
-bool FakeStatisticsProvider::HasMachineFlag(const std::string& name) {
-  return machine_flags_.find(name) != machine_flags_.end();
-}
-
 void FakeStatisticsProvider::Shutdown() {
 }
 
diff --git a/chromeos/system/fake_statistics_provider.h b/chromeos/system/fake_statistics_provider.h
index 7ea8ec2..0c43e88 100644
--- a/chromeos/system/fake_statistics_provider.h
+++ b/chromeos/system/fake_statistics_provider.h
@@ -25,9 +25,7 @@ class FakeStatisticsProvider : public StatisticsProvider {
       bool load_oem_manifest) override;
   bool GetMachineStatistic(const std::string& name,
                            std::string* result) override;
-  bool HasMachineStatistic(const std::string& name) override;
   bool GetMachineFlag(const std::string& name, bool* result) override;
-  bool HasMachineFlag(const std::string& name) override;
   void Shutdown() override;
 
   void SetMachineStatistic(const std::string& key, const std::string& value);
diff --git a/chromeos/system/statistics_provider.cc b/chromeos/system/statistics_provider.cc
index 6f86dfb..019177e 100644
--- a/chromeos/system/statistics_provider.cc
+++ b/chromeos/system/statistics_provider.cc
@@ -75,9 +75,9 @@ const char kKeyboardsPath[] = "keyboards";
 const char kLocalesPath[] = "locales";
 const char kTimeZonesPath[] = "time_zones";
 
-// Gets list value from given |dictionary| by given |key| and sets |result| to
-// a string with all list values joined by ','.
-// Returns true if |result| was successfully set.
+// Gets ListValue from given |dictionary| by given |key| and (unless |result| is
+// nullptr) sets |result| to a string with all list values joined by ','.
+// Returns true on success.
 bool JoinListValuesToString(const base::DictionaryValue* dictionary,
                             const std::string key,
                             std::string* result) {
@@ -99,13 +99,14 @@ bool JoinListValuesToString(const base::DictionaryValue* dictionary,
 
     buffer += value;
   }
-  *result = buffer;
+  if (result != nullptr)
+    *result = buffer;
   return true;
 }
 
-// Gets list value from given |dictionary| by given |key| and sets |result| to
-// the first value as string.
-// Returns true if |result| was successfully set.
+// Gets ListValue from given |dictionary| by given |key| and (unless |result| is
+// nullptr) sets |result| to the first value as string.  Returns true on
+// success.
 bool GetFirstListValueAsString(const base::DictionaryValue* dictionary,
                                const std::string key,
                                std::string* result) {
@@ -113,10 +114,12 @@ bool GetFirstListValueAsString(const base::DictionaryValue* dictionary,
   if (!dictionary->GetListWithoutPathExpansion(key, &list))
     return false;
 
-  if (list->GetSize() == 0)
+  std::string value;
+  if (!list->GetString(0, &value))
     return false;
-
-  return list->GetString(0, result);
+  if (result != nullptr)
+    *result = value;
+  return true;
 }
 
 bool GetKeyboardLayoutFromRegionalData(const base::DictionaryValue* region_dict,
@@ -178,9 +181,7 @@ class StatisticsProviderImpl : public StatisticsProvider {
       bool load_oem_manifest) override;
   bool GetMachineStatistic(const std::string& name,
                            std::string* result) override;
-  bool HasMachineStatistic(const std::string& name) override;
   bool GetMachineFlag(const std::string& name, bool* result) override;
-  bool HasMachineFlag(const std::string& name) override;
   void Shutdown() override;
 
   static StatisticsProviderImpl* GetInstance();
@@ -247,13 +248,13 @@ bool StatisticsProviderImpl::WaitForStatisticsLoaded() {
 
   base::TimeDelta dtime = base::Time::Now() - start_time;
   if (on_statistics_loaded_.IsSignaled()) {
-    LOG(ERROR) << "Statistics loaded after waiting "
-               << dtime.InMilliseconds() << "ms. ";
+    LOG(WARNING) << "Statistics loaded after waiting "
+                 << dtime.InMilliseconds() << "ms.";
     return true;
   }
 
   LOG(ERROR) << "Statistics not loaded after waiting "
-             << dtime.InMilliseconds() << "ms. ";
+             << dtime.InMilliseconds() << "ms.";
   return false;
 }
 
@@ -329,21 +330,18 @@ bool StatisticsProviderImpl::GetMachineStatistic(const std::string& name,
   if (iter == machine_info_.end()) {
     if (GetRegionalInformation(name, result))
       return true;
-    if (base::SysInfo::IsRunningOnChromeOS() &&
+    if (result != nullptr &&
+        base::SysInfo::IsRunningOnChromeOS() &&
         (oem_manifest_loaded_ || !HasOemPrefix(name))) {
       LOG(WARNING) << "Requested statistic not found: " << name;
     }
     return false;
   }
-  *result = iter->second;
+  if (result != nullptr)
+    *result = iter->second;
   return true;
 }
 
-bool StatisticsProviderImpl::HasMachineStatistic(const std::string& name) {
-  std::string result;
-  return GetMachineStatistic(name, &result);
-}
-
 bool StatisticsProviderImpl::GetMachineFlag(const std::string& name,
                                             bool* result) {
   VLOG(1) << "Machine Flag requested: " << name;
@@ -354,21 +352,18 @@ bool StatisticsProviderImpl::GetMachineFlag(const std::string& name,
 
   MachineFlags::const_iterator iter = machine_flags_.find(name);
   if (iter == machine_flags_.end()) {
-    if (base::SysInfo::IsRunningOnChromeOS() &&
+    if (result != nullptr &&
+        base::SysInfo::IsRunningOnChromeOS() &&
         (oem_manifest_loaded_ || !HasOemPrefix(name))) {
       LOG(WARNING) << "Requested machine flag not found: " << name;
     }
     return false;
   }
-  *result = iter->second;
+  if (result != nullptr)
+    *result = iter->second;
   return true;
 }
 
-bool StatisticsProviderImpl::HasMachineFlag(const std::string& name) {
-  bool result = false;
-  return GetMachineFlag(name, &result);
-}
-
 void StatisticsProviderImpl::Shutdown() {
   cancellation_flag_.Set();  // Cancel any pending loads
 }
@@ -550,6 +545,10 @@ StatisticsProviderImpl* StatisticsProviderImpl::GetInstance() {
       base::DefaultSingletonTraits<StatisticsProviderImpl>>::get();
 }
 
+bool StatisticsProvider::HasMachineStatistic(const std::string& name) {
+  return GetMachineStatistic(name, nullptr);
+}
+
 static StatisticsProvider* g_test_statistics_provider = NULL;
 
 // static
diff --git a/chromeos/system/statistics_provider.h b/chromeos/system/statistics_provider.h
index 2c1647e..c1b725a 100644
--- a/chromeos/system/statistics_provider.h
+++ b/chromeos/system/statistics_provider.h
@@ -84,23 +84,22 @@ class CHROMEOS_EXPORT StatisticsProvider {
       const scoped_refptr<base::TaskRunner>& file_task_runner,
       bool load_oem_manifest) = 0;
 
-  // Retrieves the named machine statistic (e.g. "hardware_class"). If |name|
-  // is found, sets |result| and returns true. Safe to call from any thread
-  // except the task runner passed to Initialize() (e.g. FILE). This may block
-  // if called early before the statistics are loaded from disk.
+  // Returns true if the named machine statistic (e.g. "hardware_class") is
+  // found and stores it in |result| (if provided). Probing for the existance of
+  // a statistic by setting |result| to nullptr supresses the usual warning in
+  // case the statistic is not found. Safe to call from any thread except the
+  // task runner passed to Initialize() (e.g. FILE). This may block if called
+  // early before the statistics are loaded from disk.
   // StartLoadingMachineStatistics() must be called before this.
   virtual bool GetMachineStatistic(const std::string& name,
                                    std::string* result) = 0;
 
-  // Checks whether a machine statistic is present.
-  virtual bool HasMachineStatistic(const std::string& name) = 0;
+  // Checks whether a machine statistic is present (without logging a warning).
+  bool HasMachineStatistic(const std::string& name);
 
   // Similar to GetMachineStatistic for boolean flags.
   virtual bool GetMachineFlag(const std::string& name, bool* result) = 0;
 
-  // Checks whether a machine flag is present.
-  virtual bool HasMachineFlag(const std::string& name) = 0;
-
   // Cancels any pending file operations.
   virtual void Shutdown() = 0;