From 7e558cdaba388b17e8a52f6cf7d8f43398cb7c43 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Thu, 9 Jun 2016 11:34:29 +0200 Subject: Improve location provider thread safety MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An error or update could potentially arrive after shutdown causing segmentation faults due to using unretained pointers in Chromium callbacks. This patch switches to using Chromium WeakPtrs and Qt invokable, to ensure dead objects are not invoked. Change-Id: Icf1a22c2ee081048dcc579e74b1b5da572eaf256 Reviewed-by: Michael Brüning --- src/core/location_provider_qt.cpp | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/src/core/location_provider_qt.cpp b/src/core/location_provider_qt.cpp index 485ea8d94..7184ca12a 100644 --- a/src/core/location_provider_qt.cpp +++ b/src/core/location_provider_qt.cpp @@ -41,11 +41,13 @@ #include "type_conversion.h" #include +#include #include #include -#include "base/message_loop/message_loop.h" #include "base/bind.h" +#include "base/memory/weak_ptr.h" +#include "base/message_loop/message_loop.h" #include "content/browser/geolocation/geolocation_provider_impl.h" #include "content/public/browser/browser_thread.h" #include "content/public/browser/geolocation_provider.h" @@ -60,9 +62,9 @@ public: QtPositioningHelper(LocationProviderQt *provider); ~QtPositioningHelper(); - void start(bool highAccuracy); - void stop(); - void refresh(); + Q_INVOKABLE void start(bool highAccuracy); + Q_INVOKABLE void stop(); + Q_INVOKABLE void refresh(); private Q_SLOTS: void updatePosition(const QGeoPositionInfo &); @@ -72,6 +74,7 @@ private Q_SLOTS: private: LocationProviderQt *m_locationProvider; QGeoPositionInfoSource *m_positionInfoSource; + base::WeakPtrFactory m_locationProviderFactory; void postToLocationProvider(const base::Closure &task); friend class LocationProviderQt; @@ -80,14 +83,13 @@ private: QtPositioningHelper::QtPositioningHelper(LocationProviderQt *provider) : m_locationProvider(provider) , m_positionInfoSource(0) + , m_locationProviderFactory(provider) { Q_ASSERT(provider); } QtPositioningHelper::~QtPositioningHelper() { - if (m_locationProvider) - m_locationProvider->m_positioningHelper = 0; } static bool isHighAccuracySource(const QGeoPositionInfoSource *source) @@ -183,7 +185,8 @@ void QtPositioningHelper::updatePosition(const QGeoPositionInfo &pos) newPos.speed = pos.hasAttribute(QGeoPositionInfo::GroundSpeed) ? pos.attribute(QGeoPositionInfo::GroundSpeed) : -1; newPos.heading = pos.hasAttribute(QGeoPositionInfo::Direction) ? pos.attribute(QGeoPositionInfo::Direction) : -1; - postToLocationProvider(base::Bind(&LocationProviderQt::updatePosition, base::Unretained(m_locationProvider), newPos)); + if (m_locationProvider) + postToLocationProvider(base::Bind(&LocationProviderQt::updatePosition, m_locationProviderFactory.GetWeakPtr(), newPos)); } void QtPositioningHelper::error(QGeoPositionInfoSource::Error positioningError) @@ -200,7 +203,8 @@ void QtPositioningHelper::error(QGeoPositionInfoSource::Error positioningError) newPos.error_code = content::Geoposition::ERROR_CODE_POSITION_UNAVAILABLE; break; } - postToLocationProvider(base::Bind(&LocationProviderQt::updatePosition, base::Unretained(m_locationProvider), newPos)); + if (m_locationProvider) + postToLocationProvider(base::Bind(&LocationProviderQt::updatePosition, m_locationProviderFactory.GetWeakPtr(), newPos)); } void QtPositioningHelper::timeout() @@ -210,7 +214,8 @@ void QtPositioningHelper::timeout() // argument used in JS never comes all the way to the browser process. // Let's just treat it like any other error where the position is unavailable. newPos.error_code = content::Geoposition::ERROR_CODE_POSITION_UNAVAILABLE; - postToLocationProvider(base::Bind(&LocationProviderQt::updatePosition, base::Unretained(m_locationProvider), newPos)); + if (m_locationProvider) + postToLocationProvider(base::Bind(&LocationProviderQt::updatePosition, m_locationProviderFactory.GetWeakPtr(), newPos)); } inline void QtPositioningHelper::postToLocationProvider(const base::Closure &task) @@ -227,6 +232,7 @@ LocationProviderQt::~LocationProviderQt() { if (m_positioningHelper) { m_positioningHelper->m_locationProvider = 0; + m_positioningHelper->m_locationProviderFactory.InvalidateWeakPtrs(); m_positioningHelper->deleteLater(); } } @@ -238,23 +244,21 @@ bool LocationProviderQt::StartProvider(bool highAccuracy) m_positioningHelper = new QtPositioningHelper(this); m_positioningHelper->moveToThread(guiThread); } - BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, base::Bind(&QtPositioningHelper::start - , base::Unretained(m_positioningHelper), highAccuracy)); + + QMetaObject::invokeMethod(m_positioningHelper, "start", Qt::QueuedConnection, Q_ARG(bool, highAccuracy)); return true; } void LocationProviderQt::StopProvider() { if (m_positioningHelper) - BrowserThread::PostTask(BrowserThread::UI,FROM_HERE, base::Bind(&QtPositioningHelper::stop - , base::Unretained(m_positioningHelper))); + QMetaObject::invokeMethod(m_positioningHelper, "stop", Qt::QueuedConnection); } void LocationProviderQt::RequestRefresh() { if (m_positioningHelper) - BrowserThread::PostTask(BrowserThread::UI,FROM_HERE, base::Bind(&QtPositioningHelper::refresh - , base::Unretained(m_positioningHelper))); + QMetaObject::invokeMethod(m_positioningHelper, "refresh", Qt::QueuedConnection); } void LocationProviderQt::OnPermissionGranted() -- cgit v1.2.3