Skip to content

Commit

Permalink
[Android] Move the android Device Sensors API to the UI thread.
Browse files Browse the repository at this point in the history
Move the Java-implementation of Device Motion/Orientation/Light APIs
to run on the UI thread. This is to avoid a blocking call from the
IO thread to the UI thread inside DeviceSensors.java. That call can
lead to deadlock and hence is removed in this patch.

BUG=455767

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

Cr-Commit-Position: refs/heads/master@{#316562}
  • Loading branch information
timvolodine authored and Commit bot committed Feb 17, 2015
1 parent 137699b commit c166ffe
Show file tree
Hide file tree
Showing 9 changed files with 145 additions and 20 deletions.
4 changes: 4 additions & 0 deletions content/browser/device_sensors/data_fetcher_shared_memory.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ class CONTENT_EXPORT DataFetcherSharedMemory
DataFetcherSharedMemory();
~DataFetcherSharedMemory() override;

#if defined(OS_ANDROID)
void Shutdown() override;
#endif

private:
bool Start(ConsumerType consumer_type, void* buffer) override;
bool Stop(ConsumerType consumer_type) override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,9 @@ bool DataFetcherSharedMemory::Stop(ConsumerType consumer_type) {
return false;
}

void DataFetcherSharedMemory::Shutdown() {
DataFetcherSharedMemoryBase::Shutdown();
SensorManagerAndroid::GetInstance()->Shutdown();
}

} // namespace content
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ bool DataFetcherSharedMemoryBase::StopFetchingDeviceData(
return true;
}

void DataFetcherSharedMemoryBase::StopFetchingAllDeviceData() {
void DataFetcherSharedMemoryBase::Shutdown() {
StopFetchingDeviceData(CONSUMER_TYPE_MOTION);
StopFetchingDeviceData(CONSUMER_TYPE_ORIENTATION);
StopFetchingDeviceData(CONSUMER_TYPE_LIGHT);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class CONTENT_EXPORT DataFetcherSharedMemoryBase {

// Should be called before destruction to make sure all active
// sensors are unregistered.
void StopFetchingAllDeviceData();
virtual void Shutdown();

// Returns the shared memory handle of the device sensor data
// duplicated into the given process. This method should only be
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ DeviceInertialSensorService::GetSharedMemoryHandleForProcess(

void DeviceInertialSensorService::Shutdown() {
if (data_fetcher_) {
data_fetcher_->StopFetchingAllDeviceData();
data_fetcher_->Shutdown();
data_fetcher_.reset();
}
is_shutdown_ = true;
Expand All @@ -103,7 +103,7 @@ void DeviceInertialSensorService::Shutdown() {
void DeviceInertialSensorService::SetDataFetcherForTesting(
DataFetcherSharedMemory* test_data_fetcher) {
if (data_fetcher_)
data_fetcher_->StopFetchingAllDeviceData();
data_fetcher_->Shutdown();
data_fetcher_.reset(test_data_fetcher);
}

Expand Down
119 changes: 113 additions & 6 deletions content/browser/device_sensors/sensor_manager_android.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@
#include <string.h>

#include "base/android/jni_android.h"
#include "base/bind.h"
#include "base/memory/singleton.h"
#include "base/metrics/histogram.h"
#include "content/browser/device_sensors/inertial_sensor_consts.h"
#include "content/public/browser/browser_thread.h"
#include "jni/DeviceSensors_jni.h"

using base::android::AttachCurrentThread;
Expand Down Expand Up @@ -41,7 +43,8 @@ SensorManagerAndroid::SensorManagerAndroid()
is_light_buffer_ready_(false),
is_motion_buffer_ready_(false),
is_orientation_buffer_ready_(false),
is_using_backup_sensors_for_orientation_(false) {
is_using_backup_sensors_for_orientation_(false),
is_shutdown_(false) {
memset(received_motion_data_, 0, sizeof(received_motion_data_));
device_sensors_.Reset(Java_DeviceSensors_getInstance(
AttachCurrentThread(), base::android::GetApplicationContext()));
Expand Down Expand Up @@ -196,7 +199,25 @@ bool SensorManagerAndroid::isUsingBackupSensorsForOrientation() {

bool SensorManagerAndroid::StartFetchingDeviceLightData(
DeviceLightHardwareBuffer* buffer) {
if (BrowserThread::CurrentlyOn(BrowserThread::UI)) {
StartFetchingLightDataOnUI(buffer);
} else {
BrowserThread::PostTask(
BrowserThread::UI, FROM_HERE,
base::Bind(&SensorManagerAndroid::StartFetchingLightDataOnUI,
base::Unretained(this),
buffer));
}
return true;
}

void SensorManagerAndroid::StartFetchingLightDataOnUI(
DeviceLightHardwareBuffer* buffer) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
DCHECK(buffer);
if (is_shutdown_)
return;

{
base::AutoLock autolock(light_buffer_lock_);
device_light_buffer_ = buffer;
Expand All @@ -207,10 +228,25 @@ bool SensorManagerAndroid::StartFetchingDeviceLightData(
base::AutoLock autolock(light_buffer_lock_);
SetLightBufferValue(std::numeric_limits<double>::infinity());
}
return success;
}

void SensorManagerAndroid::StopFetchingDeviceLightData() {
if (BrowserThread::CurrentlyOn(BrowserThread::UI)) {
StopFetchingLightDataOnUI();
return;
}

BrowserThread::PostTask(
BrowserThread::UI, FROM_HERE,
base::Bind(&SensorManagerAndroid::StopFetchingLightDataOnUI,
base::Unretained(this)));
}

void SensorManagerAndroid::StopFetchingLightDataOnUI() {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
if (is_shutdown_)
return;

Stop(kTypeLight);
{
base::AutoLock autolock(light_buffer_lock_);
Expand All @@ -226,17 +262,36 @@ void SensorManagerAndroid::SetLightBufferValue(double lux) {
device_light_buffer_->data.value = lux;
device_light_buffer_->seqlock.WriteEnd();
}

// --- Device Motion

bool SensorManagerAndroid::StartFetchingDeviceMotionData(
DeviceMotionHardwareBuffer* buffer) {
if (BrowserThread::CurrentlyOn(BrowserThread::UI)) {
StartFetchingMotionDataOnUI(buffer);
} else {
BrowserThread::PostTask(
BrowserThread::UI, FROM_HERE,
base::Bind(&SensorManagerAndroid::StartFetchingMotionDataOnUI,
base::Unretained(this),
buffer));
}
return true;
}

void SensorManagerAndroid::StartFetchingMotionDataOnUI(
DeviceMotionHardwareBuffer* buffer) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
DCHECK(buffer);
if (is_shutdown_)
return;

{
base::AutoLock autolock(motion_buffer_lock_);
device_motion_buffer_ = buffer;
ClearInternalMotionBuffers();
}
bool success = Start(kTypeMotion);
Start(kTypeMotion);

// If no motion data can ever be provided, the number of active device motion
// sensors will be zero. In that case flag the shared memory buffer
Expand All @@ -246,10 +301,25 @@ bool SensorManagerAndroid::StartFetchingDeviceMotionData(
base::AutoLock autolock(motion_buffer_lock_);
CheckMotionBufferReadyToRead();
}
return success;
}

void SensorManagerAndroid::StopFetchingDeviceMotionData() {
if (BrowserThread::CurrentlyOn(BrowserThread::UI)) {
StopFetchingMotionDataOnUI();
return;
}

BrowserThread::PostTask(
BrowserThread::UI, FROM_HERE,
base::Bind(&SensorManagerAndroid::StopFetchingMotionDataOnUI,
base::Unretained(this)));
}

void SensorManagerAndroid::StopFetchingMotionDataOnUI() {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
if (is_shutdown_)
return;

Stop(kTypeMotion);
{
base::AutoLock autolock(motion_buffer_lock_);
Expand Down Expand Up @@ -308,7 +378,25 @@ void SensorManagerAndroid::SetOrientationBufferReadyStatus(bool ready) {

bool SensorManagerAndroid::StartFetchingDeviceOrientationData(
DeviceOrientationHardwareBuffer* buffer) {
if (BrowserThread::CurrentlyOn(BrowserThread::UI)) {
StartFetchingOrientationDataOnUI(buffer);
} else {
BrowserThread::PostTask(
BrowserThread::UI, FROM_HERE,
base::Bind(&SensorManagerAndroid::StartFetchingOrientationDataOnUI,
base::Unretained(this),
buffer));
}
return true;
}

void SensorManagerAndroid::StartFetchingOrientationDataOnUI(
DeviceOrientationHardwareBuffer* buffer) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
DCHECK(buffer);
if (is_shutdown_)
return;

{
base::AutoLock autolock(orientation_buffer_lock_);
device_orientation_buffer_ = buffer;
Expand All @@ -328,11 +416,25 @@ bool SensorManagerAndroid::StartFetchingDeviceOrientationData(
is_using_backup_sensors_for_orientation_ =
isUsingBackupSensorsForOrientation();
}

return success;
}

void SensorManagerAndroid::StopFetchingDeviceOrientationData() {
if (BrowserThread::CurrentlyOn(BrowserThread::UI)) {
StopFetchingOrientationDataOnUI();
return;
}

BrowserThread::PostTask(
BrowserThread::UI, FROM_HERE,
base::Bind(&SensorManagerAndroid::StopFetchingOrientationDataOnUI,
base::Unretained(this)));
}

void SensorManagerAndroid::StopFetchingOrientationDataOnUI() {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
if (is_shutdown_)
return;

Stop(kTypeOrientation);
{
base::AutoLock autolock(orientation_buffer_lock_);
Expand All @@ -343,4 +445,9 @@ void SensorManagerAndroid::StopFetchingDeviceOrientationData() {
}
}

void SensorManagerAndroid::Shutdown() {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
is_shutdown_ = true;
}

} // namespace content
13 changes: 13 additions & 0 deletions content/browser/device_sensors/sensor_manager_android.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ class CONTENT_EXPORT SensorManagerAndroid {
DeviceOrientationHardwareBuffer* buffer);
void StopFetchingDeviceOrientationData();

void Shutdown();

protected:
enum EventType {
// These constants should match DEVICE_ORIENTATION, DEVICE_MOTION and
Expand All @@ -69,6 +71,16 @@ class CONTENT_EXPORT SensorManagerAndroid {
virtual void Stop(EventType event_type);
virtual int GetNumberActiveDeviceMotionSensors();

void StartFetchingLightDataOnUI(DeviceLightHardwareBuffer* buffer);
void StopFetchingLightDataOnUI();

void StartFetchingMotionDataOnUI(DeviceMotionHardwareBuffer* buffer);
void StopFetchingMotionDataOnUI();

void StartFetchingOrientationDataOnUI(
DeviceOrientationHardwareBuffer* buffer);
void StopFetchingOrientationDataOnUI();

private:
friend struct DefaultSingletonTraits<SensorManagerAndroid>;

Expand Down Expand Up @@ -104,6 +116,7 @@ class CONTENT_EXPORT SensorManagerAndroid {
base::Lock orientation_buffer_lock_;

bool is_using_backup_sensors_for_orientation_;
bool is_shutdown_;

DISALLOW_COPY_AND_ASSIGN(SensorManagerAndroid);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "base/memory/scoped_ptr.h"
#include "base/memory/weak_ptr.h"
#include "content/browser/device_sensors/inertial_sensor_consts.h"
#include "content/public/test/test_browser_thread_bundle.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace content {
Expand All @@ -16,7 +17,7 @@ namespace {

class FakeSensorManagerAndroid : public SensorManagerAndroid {
public:
FakeSensorManagerAndroid() { }
FakeSensorManagerAndroid() {}
~FakeSensorManagerAndroid() override {}

int GetNumberActiveDeviceMotionSensors() override {
Expand All @@ -29,7 +30,6 @@ class FakeSensorManagerAndroid : public SensorManagerAndroid {

protected:
bool Start(EventType event_type) override { return true; }

void Stop(EventType event_type) override {}

private:
Expand All @@ -47,6 +47,7 @@ class AndroidSensorManagerTest : public testing::Test {
scoped_ptr<DeviceLightHardwareBuffer> light_buffer_;
scoped_ptr<DeviceMotionHardwareBuffer> motion_buffer_;
scoped_ptr<DeviceOrientationHardwareBuffer> orientation_buffer_;
content::TestBrowserThreadBundle thread_bundle_;
};

TEST_F(AndroidSensorManagerTest, ThreeDeviceMotionSensorsActive) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.concurrent.Callable;

/**
* Android implementation of the device {motion|orientation|light} APIs.
Expand Down Expand Up @@ -394,13 +393,9 @@ private SensorManagerProxy getSensorManagerProxy() {
return mSensorManagerProxy;
}

SensorManager sensorManager = ThreadUtils.runOnUiThreadBlockingNoException(
new Callable<SensorManager>() {
@Override
public SensorManager call() {
return (SensorManager) mAppContext.getSystemService(Context.SENSOR_SERVICE);
}
});
ThreadUtils.assertOnUiThread();
SensorManager sensorManager =
(SensorManager) mAppContext.getSystemService(Context.SENSOR_SERVICE);

if (sensorManager != null) {
mSensorManagerProxy = new SensorManagerProxyImpl(sensorManager);
Expand Down

0 comments on commit c166ffe

Please sign in to comment.