Clean up handling of invalid HWID when running ChromeOS in VMs

chrome/browser/chromeos/login/hwid_checker.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/chromeos/login/hwid_checker.h"
 
 #include <cstdio>
 
 #include "base/command_line.h"
 #include "base/logging.h"
@@ skipping 109 matching lines @@
     return true;
   chromeos::system::StatisticsProvider* stats =
       chromeos::system::StatisticsProvider::GetInstance();
 
   std::string hwid;
   if (!stats->GetMachineStatistic(chromeos::system::kHardwareClassKey, &hwid)) {
     LOG(ERROR) << "Couldn't get machine statistic 'hardware_class'.";
     return false;
   }
   if (!chromeos::IsHWIDCorrect(hwid)) {
-    // Log the system vendor info to see what the system vendor is on the GCE
-    // VMs. This info will be used to filter out error messages on VMs. See
-    // http://crbug.com/585514 and http://crbug.com/585515 for more info.
-    std::string system_vendor;
-    stats->GetMachineStatistic(chromeos::system::kSystemVendorKey,
-                               &system_vendor);
-    std::string firmware_type;
-    const bool non_chrome_firmware =
-        stats->GetMachineStatistic(chromeos::system::kFirmwareTypeKey,
-                                   &firmware_type) &&
-        firmware_type == system::kFirmwareTypeValueNonchrome;
-    if (non_chrome_firmware) {
-      LOG(WARNING) << "Detected non-Chrome firmware with malformed HWID '"
-                   << hwid << "', assuming VM environment. "
-                   << "The system vendor is '" << system_vendor << "'.";
-    } else {
-      LOG(ERROR) << "Machine has malformed HWID '" << hwid << "'. "
-                 << "The system vendor is '" << system_vendor << "'.";
-      return false;
+    if (stats->IsRunningOnVm()) {

[achuithb, 2016/08/12 20:39:02]

Does this need to be nested inside the !IsHWIDCorrect stmt?

[norvez, 2016/08/13 00:06:59]

Dunno, that order looks right to me. The complete absence of the
'hardware_class' property is a classic "This should never happen" so I don't
like to preempt it with IsRunningOnVm(), it's the kind of condition we really
want to know about. And once we've retrieved the HWID it makes sense to check
its correctness before falling back to the VM  check in case the HWID is
invalid.

[achuithb, 2016/08/15 17:15:05]

No, to be honest I would prefer this to be moved to line 123.

Nesting this inside IsHWIDCorrect only makes sense to people aware that the
current implementation of IsRunningOnVm uses HWID to determine if we're in a VM.
First, this is an implementation detail in the chromeos source, and secondly,
this could change, for instance, we could use image_to_vm.sh to write a special
file instead of relying on hwid.

[norvez, 2016/08/16 10:36:19]

Done.

+      return true;
     }
+    LOG(ERROR) << "Machine has malformed HWID '" << hwid << "'. ";
+    return false;
   }
   return true;
 }
 
 } // namespace chromeos