Fix invalid PlatformSensorReaderWinrt creation due to bad switch case

The PlatformSensorReaderWinrtBase::IsSensorCreateSuccess function has a
switch case that determines whether a PlatformSensorReader was
successfully created.

The kErrorGetMinReportIntervalFailed (which is not fatal) and
kErrorISensorWinrtStaticsActivationFailedcases (which is fatal) cases
should have been swapped.

This is causing a non-null PlatformSensorReader to be returned, but
the underlying WinRT sensor object was null which triggers a null
pointer deference the next time it is used.

The fix is to swap the switch cases.

Verified by running the new unit test.

Bug: 1020987
Change-Id: Ia9ac960fb95de0b028677c1ff97b632aecd0b6f2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1904503
Commit-Queue: Wenjie Shi <wensh@microsoft.com>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713672}

services/device/generic_sensor/platform_sensor_reader_winrt.cc

@@ -126,35 +126,6 @@ void PlatformSensorReaderWinrtBase<
   client_ = client;
 }
 
-// static
-template <wchar_t const* runtime_class_id,
-          class ISensorWinrtStatics,
-          class ISensorWinrtClass,
-          class ISensorReadingChangedHandler,
-          class ISensorReadingChangedEventArgs>
-bool PlatformSensorReaderWinrtBase<runtime_class_id,
-                                   ISensorWinrtStatics,
-                                   ISensorWinrtClass,
-                                   ISensorReadingChangedHandler,
-                                   ISensorReadingChangedEventArgs>::
-    IsSensorCreateSuccess(SensorWinrtCreateFailure create_return_code) {
-  switch (create_return_code) {
-    case SensorWinrtCreateFailure::kErrorISensorWinrtStaticsActivationFailed:
-      // Failing to query the default report interval is not a fatal error. The
-      // consumer (PlatformSensorWin) should be able to handle this and return a
-      // default report interval instead.
-      FALLTHROUGH;
-    case SensorWinrtCreateFailure::kOk:
-      return true;
-    case SensorWinrtCreateFailure::kErrorGetDefaultSensorFailed:
-      FALLTHROUGH;
-    case SensorWinrtCreateFailure::kErrorDefaultSensorNull:
-      FALLTHROUGH;
-    case SensorWinrtCreateFailure::kErrorGetMinReportIntervalFailed:
-      return false;
-  }
-}
-
 template <wchar_t const* runtime_class_id,
           class ISensorWinrtStatics,
           class ISensorWinrtClass,
@@ -183,35 +154,42 @@ template <wchar_t const* runtime_class_id,
           class ISensorWinrtClass,
           class ISensorReadingChangedHandler,
           class ISensorReadingChangedEventArgs>
-SensorWinrtCreateFailure
-PlatformSensorReaderWinrtBase<runtime_class_id,
-                              ISensorWinrtStatics,
-                              ISensorWinrtClass,
-                              ISensorReadingChangedHandler,
-                              ISensorReadingChangedEventArgs>::Initialize() {
+bool PlatformSensorReaderWinrtBase<
+    runtime_class_id,
+    ISensorWinrtStatics,
+    ISensorWinrtClass,
+    ISensorReadingChangedHandler,
+    ISensorReadingChangedEventArgs>::Initialize() {
   ComPtr<ISensorWinrtStatics> sensor_statics;
 
   HRESULT hr = get_sensor_factory_callback_.Run(&sensor_statics);
 
-  if (FAILED(hr))
-    return SensorWinrtCreateFailure::kErrorISensorWinrtStaticsActivationFailed;
+  if (FAILED(hr)) {
+    DLOG(ERROR) << "Failed to get sensor activation factory: "
+                << logging::SystemErrorCodeToString(hr);
+    return false;
+  }
 
   hr = sensor_statics->GetDefault(&sensor_);
   base::UmaHistogramSparse("Sensors.Windows.WinRT.Activation.Result", hr);
   if (FAILED(hr)) {
-    return SensorWinrtCreateFailure::kErrorGetDefaultSensorFailed;
+    DLOG(ERROR) << "Failed to query default sensor: "
+                << logging::SystemErrorCodeToString(hr);
+    return false;
   }
 
   // GetDefault() returns null if the sensor does not exist
-  if (!sensor_)
-    return SensorWinrtCreateFailure::kErrorDefaultSensorNull;
+  if (!sensor_) {
+    VLOG(1) << "Sensor does not exist on system";
+    return false;
+  }
 
   minimum_report_interval_ = GetMinimumReportIntervalFromSensor();
 
   if (minimum_report_interval_.is_zero())
-    return SensorWinrtCreateFailure::kErrorGetMinReportIntervalFailed;
+    DLOG(WARNING) << "Failed to get sensor minimum report interval";
 
-  return SensorWinrtCreateFailure::kOk;
+  return true;
 }
 
 template <wchar_t const* runtime_class_id,
@@ -348,7 +326,7 @@ PlatformSensorReaderWinrtBase<
 std::unique_ptr<PlatformSensorReaderWinBase>
 PlatformSensorReaderWinrtLightSensor::Create() {
   auto light_sensor = std::make_unique<PlatformSensorReaderWinrtLightSensor>();
-  if (IsSensorCreateSuccess(light_sensor->Initialize())) {
+  if (light_sensor->Initialize()) {
     return light_sensor;
   }
   return nullptr;
@@ -406,7 +384,7 @@ std::unique_ptr<PlatformSensorReaderWinBase>
 PlatformSensorReaderWinrtAccelerometer::Create() {
   auto accelerometer =
       std::make_unique<PlatformSensorReaderWinrtAccelerometer>();
-  if (IsSensorCreateSuccess(accelerometer->Initialize())) {
+  if (accelerometer->Initialize()) {
     return accelerometer;
   }
   return nullptr;
@@ -486,7 +464,7 @@ HRESULT PlatformSensorReaderWinrtAccelerometer::OnReadingChangedCallback(
 std::unique_ptr<PlatformSensorReaderWinBase>
 PlatformSensorReaderWinrtGyrometer::Create() {
   auto gyrometer = std::make_unique<PlatformSensorReaderWinrtGyrometer>();
-  if (IsSensorCreateSuccess(gyrometer->Initialize())) {
+  if (gyrometer->Initialize()) {
     return gyrometer;
   }
   return nullptr;
@@ -565,7 +543,7 @@ HRESULT PlatformSensorReaderWinrtGyrometer::OnReadingChangedCallback(
 std::unique_ptr<PlatformSensorReaderWinBase>
 PlatformSensorReaderWinrtMagnetometer::Create() {
   auto magnetometer = std::make_unique<PlatformSensorReaderWinrtMagnetometer>();
-  if (IsSensorCreateSuccess(magnetometer->Initialize())) {
+  if (magnetometer->Initialize()) {
     return magnetometer;
   }
   return nullptr;
@@ -642,7 +620,7 @@ std::unique_ptr<PlatformSensorReaderWinBase>
 PlatformSensorReaderWinrtAbsOrientationEulerAngles::Create() {
   auto inclinometer =
       std::make_unique<PlatformSensorReaderWinrtAbsOrientationEulerAngles>();
-  if (IsSensorCreateSuccess(inclinometer->Initialize())) {
+  if (inclinometer->Initialize()) {
     return inclinometer;
   }
   return nullptr;
@@ -720,7 +698,7 @@ std::unique_ptr<PlatformSensorReaderWinBase>
 PlatformSensorReaderWinrtAbsOrientationQuaternion::Create() {
   auto orientation =
       std::make_unique<PlatformSensorReaderWinrtAbsOrientationQuaternion>();
-  if (IsSensorCreateSuccess(orientation->Initialize())) {
+  if (orientation->Initialize()) {
     return orientation;
   }
   return nullptr;

services/device/generic_sensor/platform_sensor_reader_winrt.h

@@ -32,14 +32,6 @@ class PlatformSensorReaderWinrtFactory {
       mojom::SensorType type);
 };
 
-enum SensorWinrtCreateFailure {
-  kOk = 0,
-  kErrorISensorWinrtStaticsActivationFailed = 1,
-  kErrorGetDefaultSensorFailed = 2,
-  kErrorDefaultSensorNull = 3,
-  kErrorGetMinReportIntervalFailed = 4
-};
-
 // Base class that contains common helper functions used between all low
 // level sensor types based on the Windows.Devices.Sensors API. Derived
 // classes will specialize the template into a specific sensor. See
@@ -65,11 +57,15 @@ class PlatformSensorReaderWinrtBase : public PlatformSensorReaderWinBase {
 
   // Allows tests to specify their own implementation of the underlying sensor.
   // This function should be called before Initialize().
-  void InitForTests(GetSensorFactoryFunctor get_sensor_factory_callback) {
+  void InitForTesting(GetSensorFactoryFunctor get_sensor_factory_callback) {
     get_sensor_factory_callback_ = get_sensor_factory_callback;
   }
 
-  SensorWinrtCreateFailure Initialize() WARN_UNUSED_RESULT;
+  // Returns true if the underlying WinRT sensor object is valid, meant
+  // for testing purposes.
+  bool IsUnderlyingWinrtObjectValidForTesting() { return sensor_; }
+
+  bool Initialize() WARN_UNUSED_RESULT;
 
   bool StartSensor(const PlatformSensorConfiguration& configuration) override
       WARN_UNUSED_RESULT;
@@ -80,11 +76,6 @@ class PlatformSensorReaderWinrtBase : public PlatformSensorReaderWinBase {
   PlatformSensorReaderWinrtBase();
   virtual ~PlatformSensorReaderWinrtBase();
 
-  // Determines if the SensorWinrtCreateFailure code means a WinRT sensor
-  // was successfully created or not.
-  static bool IsSensorCreateSuccess(
-      SensorWinrtCreateFailure create_return_code);
-
   // Derived classes should implement this function to handle sensor specific
   // parsing of the sensor reading.
   virtual HRESULT OnReadingChangedCallback(