Skip to content

Commit

Permalink
Fix race between orientation sensor fallback and listener removal
Browse files Browse the repository at this point in the history
This change fixes a DCHECK which can fire due to the internal state of
the DeviceOrientationEventPump becoming invalid when a page removes its
event listener before the failure to initialize the relative orientation
sensor is received.

This should've been handled by the existing code in DidStartIfPossible()
however since DeviceSensorEntry::HandleSensorError() updates the sensor
state to kNotInitialized the information that the event listener was
removed and so the sensor should move into the kSuspended state after
initialization was lost, resulting in the fallback sensor instead
entering the kActive state. DidStartIfPossible() now checks the pump
state, which will be set to kStopped, instead of the DeviceSensorEntry
state.

New unit tests have been added to exercise some of these "stopped while
initializing" cases more carefully.

Bug: 1409077
Change-Id: I3e7b6adf12dbb04e012486cef86d8b70a7f9378d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4190679
Auto-Submit: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Matt Reynolds <mattreynolds@chromium.org>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1096396}
  • Loading branch information
reillyeon authored and Chromium LUCI CQ committed Jan 24, 2023
1 parent 60b350f commit 873bd10
Show file tree
Hide file tree
Showing 7 changed files with 166 additions and 95 deletions.
4 changes: 4 additions & 0 deletions services/device/public/cpp/test/fake_sensor_and_provider.cc
Expand Up @@ -104,6 +104,10 @@ void FakeSensorProvider::GetSensor(mojom::SensorType type,
return;
}

if (sensor_requested_callback_) {
std::move(sensor_requested_callback_).Run(type);
}

SensorReadingSharedBuffer* buffer = GetSensorReadingSharedBufferForType(type);

std::unique_ptr<FakeSensor> sensor;
Expand Down
8 changes: 8 additions & 0 deletions services/device/public/cpp/test/fake_sensor_and_provider.h
Expand Up @@ -70,6 +70,13 @@ class FakeSensorProvider : public mojom::SensorProvider {
void Bind(mojo::PendingReceiver<mojom::SensorProvider> receiver);
bool is_bound() const;

// Configures a callback which is invoked when GetSensor() is called, allowing
// tests to take an action before the response callback is invoked.
void set_sensor_requested_callback(
base::OnceCallback<void(mojom::SensorType)> callback) {
sensor_requested_callback_ = std::move(callback);
}

void set_ambient_light_sensor_is_available(
bool ambient_light_sensor_is_available) {
ambient_light_sensor_is_available_ = ambient_light_sensor_is_available;
Expand Down Expand Up @@ -149,6 +156,7 @@ class FakeSensorProvider : public mojom::SensorProvider {
SensorReading gyroscope_reading_;
SensorReading relative_orientation_sensor_reading_;
SensorReading absolute_orientation_sensor_reading_;
base::OnceCallback<void(mojom::SensorType)> sensor_requested_callback_;
bool ambient_light_sensor_is_available_ = true;
bool accelerometer_is_available_ = true;
bool linear_acceleration_sensor_is_available_ = true;
Expand Down
Expand Up @@ -144,8 +144,7 @@ void DeviceOrientationEventPump::DidStartIfPossible() {
// then fall back to using absolute_orientation_sensor_.
attempted_to_fall_back_to_absolute_orientation_sensor_ = true;
absolute_orientation_sensor_->Start(sensor_provider_.get());
if (relative_orientation_sensor_->state() ==
DeviceSensorEntry::State::kShouldSuspend) {
if (state() == PumpState::kStopped) {
// If SendStopMessage() was called before the OnSensorCreated() callback
// registered that relative_orientation_sensor_ was not able to connect
// then absolute_orientation_sensor_ needs to be Stop()'d so that it
Expand Down
Expand Up @@ -6,6 +6,8 @@
#define THIRD_PARTY_BLINK_RENDERER_MODULES_DEVICE_ORIENTATION_DEVICE_ORIENTATION_EVENT_PUMP_H_

#include "third_party/blink/renderer/modules/device_orientation/device_sensor_event_pump.h"

#include "third_party/blink/renderer/modules/device_orientation/device_sensor_entry.h"
#include "third_party/blink/renderer/modules/modules_export.h"
#include "third_party/blink/renderer/platform/heap/garbage_collected.h"

Expand Down Expand Up @@ -43,6 +45,14 @@ class MODULES_EXPORT DeviceOrientationEventPump
void SendStartMessage(LocalFrame& frame) override;
void SendStopMessage() override;

DeviceSensorEntry::State GetRelativeSensorStateForTesting() {
return relative_orientation_sensor_->state();
}

DeviceSensorEntry::State GetAbsoluteSensorStateForTesting() {
return absolute_orientation_sensor_->state();
}

protected:
// DeviceSensorEventPump:
void FireEvent(TimerBase*) override;
Expand Down

0 comments on commit 873bd10

Please sign in to comment.