Do not show "Error calling GetScreenBrightnessPercent" in VMTests.

chromeos/dbus/power_manager_client.cc

Index: chromeos/dbus/power_manager_client.cc
diff --git a/chromeos/dbus/power_manager_client.cc b/chromeos/dbus/power_manager_client.cc
index 0bdfcbf4feaddbcf74670e6c637ddf7a6a7fbbf0..e5c9614ed5036bdd06699e9a7c7d67d0cdc33ff6 100644
--- a/chromeos/dbus/power_manager_client.cc
+++ b/chromeos/dbus/power_manager_client.cc
@@ -29,6 +29,7 @@
 #include "chromeos/dbus/power_manager/policy.pb.h"
 #include "chromeos/dbus/power_manager/power_supply_properties.pb.h"
 #include "chromeos/dbus/power_manager/suspend.pb.h"
+#include "chromeos/system/statistics_provider.h"
 #include "components/device_event_log/device_event_log.h"
 #include "dbus/bus.h"
 #include "dbus/message.h"
@@ -37,6 +38,8 @@
 
 namespace chromeos {
 
+namespace {
+
 // Maximum amount of time that the power manager will wait for Chrome to
 // say that it's ready for the system to be suspended, in milliseconds.
 const int kSuspendDelayTimeoutMs = 5000;
@@ -44,6 +47,11 @@ const int kSuspendDelayTimeoutMs = 5000;
 // Human-readable description of Chrome's suspend delay.
 const char kSuspendDelayDescription[] = "chrome";
 
+// The system vendor in a VM.
+const char kVMSystemVendor[] = "QEMU";

[achuithb, 2016/05/17 20:18:51]

We have this same definition here:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/chr...

I think it may be time to add a function like
IsRunningInVM() to StatisticsProvider instead of doing this same comparison
everywhere.

+
+}  // namespace
+
 // The PowerManagerClient implementation used in production.
 class PowerManagerClientImpl : public PowerManagerClient {
  public:
@@ -458,8 +466,17 @@ class PowerManagerClientImpl : public PowerManagerClient {
       const GetScreenBrightnessPercentCallback& callback,
       dbus::Response* response) {
     if (!response) {
-      POWER_LOG(ERROR) << "Error calling "
-                       << power_manager::kGetScreenBrightnessPercentMethod;
+      chromeos::system::StatisticsProvider* stats =
+          chromeos::system::StatisticsProvider::GetInstance();
+      std::string system_vendor;
+      // Only log the error when not running in a VM. See crbug.com/585514 and
+      // crbug.com/585504 to see why we should do this.
+      if (stats->GetMachineStatistic(chromeos::system::kSystemVendorKey,
+                                     &system_vendor) &&
+          system_vendor != kVMSystemVendor) {

[achuithb, 2016/05/17 20:18:51]

Are we confident that this check works in the VM? Have you tested it on the
trybots?

[xdai1, 2016/05/17 20:35:55]

I tested this in the VMTest locally and it worked fine. How can I test it on the
trybots? 

[achuithb, 2016/05/17 21:10:47]

The previous hwid change (http://crbug.com/585515) - did it suppress the logging
as you expected in VM tests?

I see that 585515 was bulk verified, but could you please verify by looking at
VM test logs and ensuring that the message was suppressed as you'd expect? 

[xdai1, 2016/05/17 21:35:17]

I just checked and unfortunately the HWID error is still there, which means the
system vendor check does not work in real VMTests(trybots) :( 

+        POWER_LOG(ERROR) << "Error calling "
+                         << power_manager::kGetScreenBrightnessPercentMethod;
+      }
       return;
     }
     dbus::MessageReader reader(response);