From 61e6e8dfc9b39c85a4e2865012c1e47723717376 Mon Sep 17 00:00:00 2001 From: Heather Anderson Date: Wed, 22 Apr 2020 23:16:51 -0700 Subject: [PATCH 1/7] Restructure ContextAwareProfile from a (thread-blocking) polling to a threadsafe-notification --- .../ui/src/ui/types/ContextAwareProfile.cpp | 65 +++++++++++++++---- .../ui/src/ui/types/ContextAwareProfile.h | 40 ++++++++++-- 2 files changed, 89 insertions(+), 16 deletions(-) diff --git a/libraries/ui/src/ui/types/ContextAwareProfile.cpp b/libraries/ui/src/ui/types/ContextAwareProfile.cpp index f74e8754c9..d5b8d958b6 100644 --- a/libraries/ui/src/ui/types/ContextAwareProfile.cpp +++ b/libraries/ui/src/ui/types/ContextAwareProfile.cpp @@ -20,20 +20,62 @@ static const QString RESTRICTED_FLAG_PROPERTY = "RestrictFileAccess"; -ContextAwareProfile::ContextAwareProfile(QQmlContext* context) : - ContextAwareProfileParent(context), _context(context) { - assert(context); +QMutex RestrictedContextMonitor::gl_monitorMapProtect; +RestrictedContextMonitor::TMonitorMap RestrictedContextMonitor::gl_monitorMap; + +RestrictedContextMonitor::~RestrictedContextMonitor() { + gl_monitorMapProtect.lock(); + TMonitorMap::iterator lookup = gl_monitorMap.find(context); + if (lookup != gl_monitorMap.end()) { + gl_monitorMap.erase(lookup); + } + gl_monitorMapProtect.unlock(); } +RestrictedContextMonitor::TSharedPtr RestrictedContextMonitor::getMonitor(QQmlContext* context, bool createIfMissing) { + TSharedPtr monitor; + + gl_monitorMapProtect.lock(); + TMonitorMap::const_iterator lookup = gl_monitorMap.find(context); + if (lookup != gl_monitorMap.end()) { + monitor = lookup->second.lock(); + assert(monitor); + } else if(createIfMissing) { + monitor = std::make_shared(context); + monitor->selfPtr = monitor; + gl_monitorMap.insert(TMonitorMap::value_type(context, monitor)); + } + gl_monitorMapProtect.unlock(); + return monitor; +} + +ContextAwareProfile::ContextAwareProfile(QQmlContext* context) : ContextAwareProfileParent(context), _context(context) { + assert(context); + + _monitor = RestrictedContextMonitor::getMonitor(context, true); + assert(_monitor); + connect(_monitor.get(), &RestrictedContextMonitor::onIsRestrictedChanged, this, &ContextAwareProfile::onIsRestrictedChanged); + if (_monitor->isUninitialized) { + _monitor->isRestricted = isRestrictedGetProperty(); + _monitor->isUninitialized = false; + } + _isRestricted.store(_monitor->isRestricted ? 1 : 0); +} void ContextAwareProfile::restrictContext(QQmlContext* context, bool restrict) { + RestrictedContextMonitor::TSharedPtr monitor = RestrictedContextMonitor::getMonitor(context, false); + context->setContextProperty(RESTRICTED_FLAG_PROPERTY, restrict); + if (monitor && monitor->isRestricted != restrict) { + monitor->isRestricted = restrict; + monitor->onIsRestrictedChanged(restrict); + } } -bool ContextAwareProfile::isRestrictedInternal() { +bool ContextAwareProfile::isRestrictedGetProperty() { if (QThread::currentThread() != thread()) { bool restrictedResult = false; - BLOCKING_INVOKE_METHOD(this, "isRestrictedInternal", Q_RETURN_ARG(bool, restrictedResult)); + BLOCKING_INVOKE_METHOD(this, "isRestrictedGetProperty", Q_RETURN_ARG(bool, restrictedResult)); return restrictedResult; } @@ -47,11 +89,10 @@ bool ContextAwareProfile::isRestrictedInternal() { return true; } -bool ContextAwareProfile::isRestricted() { - auto now = usecTimestampNow(); - if (now > _cacheExpiry) { - _cachedValue = isRestrictedInternal(); - _cacheExpiry = now + MAX_CACHE_AGE; - } - return _cachedValue; +void ContextAwareProfile::onIsRestrictedChanged(bool newValue) { + _isRestricted.store(newValue ? 1 : 0); +} + +bool ContextAwareProfile::isRestricted() { + return _isRestricted.load() != 0; } diff --git a/libraries/ui/src/ui/types/ContextAwareProfile.h b/libraries/ui/src/ui/types/ContextAwareProfile.h index 3192d2be54..30bdad4b78 100644 --- a/libraries/ui/src/ui/types/ContextAwareProfile.h +++ b/libraries/ui/src/ui/types/ContextAwareProfile.h @@ -12,6 +12,7 @@ #define hifi_ContextAwareProfile_h #include +#include #if !defined(Q_OS_ANDROID) #include @@ -30,12 +31,39 @@ using RequestInterceptorParent = QObject; class QQmlContext; +class RestrictedContextMonitor : public QObject { + Q_OBJECT +public: + typedef std::shared_ptr TSharedPtr; + typedef std::weak_ptr TWeakPtr; + + inline RestrictedContextMonitor(QQmlContext* c) : context(c) {} + ~RestrictedContextMonitor(); + + static TSharedPtr getMonitor(QQmlContext* context, bool createIfMissing); + +signals: + void onIsRestrictedChanged(bool newValue); + +public: + TWeakPtr selfPtr; + QQmlContext* context{ nullptr }; + bool isRestricted{ true }; + bool isUninitialized{ true }; + +private: + typedef std::map TMonitorMap; + + static QMutex gl_monitorMapProtect; + static TMonitorMap gl_monitorMap; +}; + class ContextAwareProfile : public ContextAwareProfileParent { Q_OBJECT public: static void restrictContext(QQmlContext* context, bool restrict = true); bool isRestricted(); - Q_INVOKABLE bool isRestrictedInternal(); + Q_INVOKABLE bool isRestrictedGetProperty(); protected: class RequestInterceptor : public RequestInterceptorParent { @@ -48,9 +76,13 @@ protected: ContextAwareProfile(QQmlContext* parent); QQmlContext* _context{ nullptr }; - bool _cachedValue{ false }; - quint64 _cacheExpiry{ 0 }; - constexpr static quint64 MAX_CACHE_AGE = MSECS_PER_SECOND; + QAtomicInt _isRestricted{ 0 }; + +private slots: + void onIsRestrictedChanged(bool newValue); + +private: + RestrictedContextMonitor::TSharedPtr _monitor; }; #endif // hifi_FileTypeProfile_h From 84a8c640e4f68fe9a9d7104a3acabdb53b5a22d8 Mon Sep 17 00:00:00 2001 From: Heather Anderson Date: Fri, 15 May 2020 13:42:29 -0700 Subject: [PATCH 2/7] cleaned up variables names, replaced std::shared_ptr with QSharedPointer --- .../ui/src/ui/types/ContextAwareProfile.cpp | 24 +++++++++---------- .../ui/src/ui/types/ContextAwareProfile.h | 21 ++++++++-------- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/libraries/ui/src/ui/types/ContextAwareProfile.cpp b/libraries/ui/src/ui/types/ContextAwareProfile.cpp index d5b8d958b6..0374de87ff 100644 --- a/libraries/ui/src/ui/types/ContextAwareProfile.cpp +++ b/libraries/ui/src/ui/types/ContextAwareProfile.cpp @@ -25,15 +25,15 @@ RestrictedContextMonitor::TMonitorMap RestrictedContextMonitor::gl_monitorMap; RestrictedContextMonitor::~RestrictedContextMonitor() { gl_monitorMapProtect.lock(); - TMonitorMap::iterator lookup = gl_monitorMap.find(context); + TMonitorMap::iterator lookup = gl_monitorMap.find(_context); if (lookup != gl_monitorMap.end()) { gl_monitorMap.erase(lookup); } gl_monitorMapProtect.unlock(); } -RestrictedContextMonitor::TSharedPtr RestrictedContextMonitor::getMonitor(QQmlContext* context, bool createIfMissing) { - TSharedPtr monitor; +RestrictedContextMonitor::TSharedPointer RestrictedContextMonitor::getMonitor(QQmlContext* context, bool createIfMissing) { + TSharedPointer monitor; gl_monitorMapProtect.lock(); TMonitorMap::const_iterator lookup = gl_monitorMap.find(context); @@ -41,8 +41,8 @@ RestrictedContextMonitor::TSharedPtr RestrictedContextMonitor::getMonitor(QQmlCo monitor = lookup->second.lock(); assert(monitor); } else if(createIfMissing) { - monitor = std::make_shared(context); - monitor->selfPtr = monitor; + monitor = TSharedPointer::create(context); + monitor->_selfPointer = monitor; gl_monitorMap.insert(TMonitorMap::value_type(context, monitor)); } gl_monitorMapProtect.unlock(); @@ -55,19 +55,19 @@ ContextAwareProfile::ContextAwareProfile(QQmlContext* context) : ContextAwarePro _monitor = RestrictedContextMonitor::getMonitor(context, true); assert(_monitor); connect(_monitor.get(), &RestrictedContextMonitor::onIsRestrictedChanged, this, &ContextAwareProfile::onIsRestrictedChanged); - if (_monitor->isUninitialized) { - _monitor->isRestricted = isRestrictedGetProperty(); - _monitor->isUninitialized = false; + if (_monitor->_isUninitialized) { + _monitor->_isRestricted = isRestrictedGetProperty(); + _monitor->_isUninitialized = false; } - _isRestricted.store(_monitor->isRestricted ? 1 : 0); + _isRestricted.store(_monitor->_isRestricted ? 1 : 0); } void ContextAwareProfile::restrictContext(QQmlContext* context, bool restrict) { - RestrictedContextMonitor::TSharedPtr monitor = RestrictedContextMonitor::getMonitor(context, false); + RestrictedContextMonitor::TSharedPointer monitor = RestrictedContextMonitor::getMonitor(context, false); context->setContextProperty(RESTRICTED_FLAG_PROPERTY, restrict); - if (monitor && monitor->isRestricted != restrict) { - monitor->isRestricted = restrict; + if (monitor && monitor->_isRestricted != restrict) { + monitor->_isRestricted = restrict; monitor->onIsRestrictedChanged(restrict); } } diff --git a/libraries/ui/src/ui/types/ContextAwareProfile.h b/libraries/ui/src/ui/types/ContextAwareProfile.h index 30bdad4b78..ec0590eda5 100644 --- a/libraries/ui/src/ui/types/ContextAwareProfile.h +++ b/libraries/ui/src/ui/types/ContextAwareProfile.h @@ -13,6 +13,7 @@ #include #include +#include #if !defined(Q_OS_ANDROID) #include @@ -34,25 +35,25 @@ class QQmlContext; class RestrictedContextMonitor : public QObject { Q_OBJECT public: - typedef std::shared_ptr TSharedPtr; - typedef std::weak_ptr TWeakPtr; + typedef QSharedPointer TSharedPointer; + typedef QWeakPointer TWeakPointer; - inline RestrictedContextMonitor(QQmlContext* c) : context(c) {} + inline RestrictedContextMonitor(QQmlContext* c) : _context(c) {} ~RestrictedContextMonitor(); - static TSharedPtr getMonitor(QQmlContext* context, bool createIfMissing); + static TSharedPointer getMonitor(QQmlContext* context, bool createIfMissing); signals: void onIsRestrictedChanged(bool newValue); public: - TWeakPtr selfPtr; - QQmlContext* context{ nullptr }; - bool isRestricted{ true }; - bool isUninitialized{ true }; + TWeakPointer _selfPointer; + QQmlContext* _context{ nullptr }; + bool _isRestricted{ true }; + bool _isUninitialized{ true }; private: - typedef std::map TMonitorMap; + typedef std::map TMonitorMap; static QMutex gl_monitorMapProtect; static TMonitorMap gl_monitorMap; @@ -82,7 +83,7 @@ private slots: void onIsRestrictedChanged(bool newValue); private: - RestrictedContextMonitor::TSharedPtr _monitor; + RestrictedContextMonitor::TSharedPointer _monitor; }; #endif // hifi_FileTypeProfile_h From 3cff3f8b5deaba040c519adb8e43b619f5b55906 Mon Sep 17 00:00:00 2001 From: Heather Anderson Date: Mon, 25 May 2020 15:51:48 -0700 Subject: [PATCH 3/7] found and removed another STL-ism --- libraries/ui/src/ui/types/ContextAwareProfile.cpp | 4 ++-- libraries/ui/src/ui/types/ContextAwareProfile.h | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/libraries/ui/src/ui/types/ContextAwareProfile.cpp b/libraries/ui/src/ui/types/ContextAwareProfile.cpp index 0374de87ff..ef17ad7229 100644 --- a/libraries/ui/src/ui/types/ContextAwareProfile.cpp +++ b/libraries/ui/src/ui/types/ContextAwareProfile.cpp @@ -38,12 +38,12 @@ RestrictedContextMonitor::TSharedPointer RestrictedContextMonitor::getMonitor(QQ gl_monitorMapProtect.lock(); TMonitorMap::const_iterator lookup = gl_monitorMap.find(context); if (lookup != gl_monitorMap.end()) { - monitor = lookup->second.lock(); + monitor = lookup.value().lock(); assert(monitor); } else if(createIfMissing) { monitor = TSharedPointer::create(context); monitor->_selfPointer = monitor; - gl_monitorMap.insert(TMonitorMap::value_type(context, monitor)); + gl_monitorMap.insert(context, monitor); } gl_monitorMapProtect.unlock(); return monitor; diff --git a/libraries/ui/src/ui/types/ContextAwareProfile.h b/libraries/ui/src/ui/types/ContextAwareProfile.h index ec0590eda5..99fee8112d 100644 --- a/libraries/ui/src/ui/types/ContextAwareProfile.h +++ b/libraries/ui/src/ui/types/ContextAwareProfile.h @@ -12,6 +12,7 @@ #define hifi_ContextAwareProfile_h #include +#include #include #include @@ -53,7 +54,7 @@ public: bool _isUninitialized{ true }; private: - typedef std::map TMonitorMap; + typedef QMap TMonitorMap; static QMutex gl_monitorMapProtect; static TMonitorMap gl_monitorMap; From 47c96fcff56218275cf9145fe962140200b14373 Mon Sep 17 00:00:00 2001 From: Heather Anderson Date: Thu, 28 May 2020 00:03:39 -0700 Subject: [PATCH 4/7] (re-)discovered QMutexLocker, good for tightening up the code a bit --- libraries/ui/src/ui/types/ContextAwareProfile.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/libraries/ui/src/ui/types/ContextAwareProfile.cpp b/libraries/ui/src/ui/types/ContextAwareProfile.cpp index ef17ad7229..2399471e29 100644 --- a/libraries/ui/src/ui/types/ContextAwareProfile.cpp +++ b/libraries/ui/src/ui/types/ContextAwareProfile.cpp @@ -12,6 +12,7 @@ #include "ContextAwareProfile.h" #include +#include #include #include @@ -24,18 +25,17 @@ QMutex RestrictedContextMonitor::gl_monitorMapProtect; RestrictedContextMonitor::TMonitorMap RestrictedContextMonitor::gl_monitorMap; RestrictedContextMonitor::~RestrictedContextMonitor() { - gl_monitorMapProtect.lock(); + QMutexLocker locker(&gl_monitorMapProtect); TMonitorMap::iterator lookup = gl_monitorMap.find(_context); if (lookup != gl_monitorMap.end()) { gl_monitorMap.erase(lookup); } - gl_monitorMapProtect.unlock(); } RestrictedContextMonitor::TSharedPointer RestrictedContextMonitor::getMonitor(QQmlContext* context, bool createIfMissing) { TSharedPointer monitor; - gl_monitorMapProtect.lock(); + QMutexLocker locker(&gl_monitorMapProtect); TMonitorMap::const_iterator lookup = gl_monitorMap.find(context); if (lookup != gl_monitorMap.end()) { monitor = lookup.value().lock(); @@ -45,7 +45,6 @@ RestrictedContextMonitor::TSharedPointer RestrictedContextMonitor::getMonitor(QQ monitor->_selfPointer = monitor; gl_monitorMap.insert(context, monitor); } - gl_monitorMapProtect.unlock(); return monitor; } From 4f5f46a6237a693e0c7eb81154b58f11f870e159 Mon Sep 17 00:00:00 2001 From: Heather Anderson Date: Thu, 30 Jul 2020 22:49:54 -0700 Subject: [PATCH 5/7] restructured the code to using a set of ContextAwareProfile objects rather than subscribing to an intermediate object changed _isRestricted from QAtomicInt to std::atomic --- .../ui/src/ui/types/ContextAwareProfile.cpp | 87 ++++++++++--------- .../ui/src/ui/types/ContextAwareProfile.h | 48 +++------- 2 files changed, 59 insertions(+), 76 deletions(-) diff --git a/libraries/ui/src/ui/types/ContextAwareProfile.cpp b/libraries/ui/src/ui/types/ContextAwareProfile.cpp index 2399471e29..5df2d34dfa 100644 --- a/libraries/ui/src/ui/types/ContextAwareProfile.cpp +++ b/libraries/ui/src/ui/types/ContextAwareProfile.cpp @@ -12,8 +12,9 @@ #include "ContextAwareProfile.h" #include -#include +#include #include +#include #include #include @@ -21,53 +22,59 @@ static const QString RESTRICTED_FLAG_PROPERTY = "RestrictFileAccess"; -QMutex RestrictedContextMonitor::gl_monitorMapProtect; -RestrictedContextMonitor::TMonitorMap RestrictedContextMonitor::gl_monitorMap; - -RestrictedContextMonitor::~RestrictedContextMonitor() { - QMutexLocker locker(&gl_monitorMapProtect); - TMonitorMap::iterator lookup = gl_monitorMap.find(_context); - if (lookup != gl_monitorMap.end()) { - gl_monitorMap.erase(lookup); - } -} - -RestrictedContextMonitor::TSharedPointer RestrictedContextMonitor::getMonitor(QQmlContext* context, bool createIfMissing) { - TSharedPointer monitor; - - QMutexLocker locker(&gl_monitorMapProtect); - TMonitorMap::const_iterator lookup = gl_monitorMap.find(context); - if (lookup != gl_monitorMap.end()) { - monitor = lookup.value().lock(); - assert(monitor); - } else if(createIfMissing) { - monitor = TSharedPointer::create(context); - monitor->_selfPointer = monitor; - gl_monitorMap.insert(context, monitor); - } - return monitor; -} +QReadWriteLock ContextAwareProfile::gl_contextMapProtect; +ContextAwareProfile::ContextMap ContextAwareProfile::gl_contextMap; ContextAwareProfile::ContextAwareProfile(QQmlContext* context) : ContextAwareProfileParent(context), _context(context) { assert(context); - _monitor = RestrictedContextMonitor::getMonitor(context, true); - assert(_monitor); - connect(_monitor.get(), &RestrictedContextMonitor::onIsRestrictedChanged, this, &ContextAwareProfile::onIsRestrictedChanged); - if (_monitor->_isUninitialized) { - _monitor->_isRestricted = isRestrictedGetProperty(); - _monitor->_isUninitialized = false; + { // register our object for future updates + QWriteLocker guard(&gl_contextMapProtect); + ContextMap::iterator setLookup = gl_contextMap.find(_context); + if (setLookup == gl_contextMap.end()) { + setLookup = gl_contextMap.insert(_context, ContextAwareProfileSet()); + } + assert(setLookup != gl_contextMap.end()); + ContextAwareProfileSet& profileSet = setLookup.value(); + assert(profileSet.find(this) == profileSet.end()); + profileSet.insert(this); + } + + _isRestricted.store(isRestrictedGetProperty()); +} + +ContextAwareProfile::~ContextAwareProfile() { + { // deregister our object + QWriteLocker guard(&gl_contextMapProtect); + ContextMap::iterator setLookup = gl_contextMap.find(_context); + assert(setLookup != gl_contextMap.end()); + if (setLookup != gl_contextMap.end()) { + ContextAwareProfileSet& profileSet = setLookup.value(); + assert(profileSet.find(this) != profileSet.end()); + profileSet.remove(this); + if (profileSet.isEmpty()) { + gl_contextMap.erase(setLookup); + } + } } - _isRestricted.store(_monitor->_isRestricted ? 1 : 0); } void ContextAwareProfile::restrictContext(QQmlContext* context, bool restrict) { - RestrictedContextMonitor::TSharedPointer monitor = RestrictedContextMonitor::getMonitor(context, false); + // set the QML property context->setContextProperty(RESTRICTED_FLAG_PROPERTY, restrict); - if (monitor && monitor->_isRestricted != restrict) { - monitor->_isRestricted = restrict; - monitor->onIsRestrictedChanged(restrict); + + // broadcast the new value to any registered ContextAwareProfile objects + { // deregister our object + QReadLocker guard(&gl_contextMapProtect); + ContextMap::const_iterator setLookup = gl_contextMap.find(context); + if (setLookup != gl_contextMap.end()) { + const ContextAwareProfileSet& profileSet = setLookup.value(); + for (ContextAwareProfileSet::const_iterator profileIterator = profileSet.begin(); + profileIterator != profileSet.end(); profileIterator++) { + (*profileIterator)->onIsRestrictedChanged(restrict); + } + } } } @@ -89,9 +96,9 @@ bool ContextAwareProfile::isRestrictedGetProperty() { } void ContextAwareProfile::onIsRestrictedChanged(bool newValue) { - _isRestricted.store(newValue ? 1 : 0); + _isRestricted.store(newValue); } bool ContextAwareProfile::isRestricted() { - return _isRestricted.load() != 0; + return _isRestricted.load(); } diff --git a/libraries/ui/src/ui/types/ContextAwareProfile.h b/libraries/ui/src/ui/types/ContextAwareProfile.h index 99fee8112d..d8ec762858 100644 --- a/libraries/ui/src/ui/types/ContextAwareProfile.h +++ b/libraries/ui/src/ui/types/ContextAwareProfile.h @@ -11,9 +11,10 @@ #ifndef hifi_ContextAwareProfile_h #define hifi_ContextAwareProfile_h -#include +#include #include -#include +#include +#include #include #if !defined(Q_OS_ANDROID) @@ -29,37 +30,8 @@ using ContextAwareProfileParent = QObject; using RequestInterceptorParent = QObject; #endif -#include - class QQmlContext; -class RestrictedContextMonitor : public QObject { - Q_OBJECT -public: - typedef QSharedPointer TSharedPointer; - typedef QWeakPointer TWeakPointer; - - inline RestrictedContextMonitor(QQmlContext* c) : _context(c) {} - ~RestrictedContextMonitor(); - - static TSharedPointer getMonitor(QQmlContext* context, bool createIfMissing); - -signals: - void onIsRestrictedChanged(bool newValue); - -public: - TWeakPointer _selfPointer; - QQmlContext* _context{ nullptr }; - bool _isRestricted{ true }; - bool _isUninitialized{ true }; - -private: - typedef QMap TMonitorMap; - - static QMutex gl_monitorMapProtect; - static TMonitorMap gl_monitorMap; -}; - class ContextAwareProfile : public ContextAwareProfileParent { Q_OBJECT public: @@ -77,14 +49,18 @@ protected: }; ContextAwareProfile(QQmlContext* parent); - QQmlContext* _context{ nullptr }; - QAtomicInt _isRestricted{ 0 }; - -private slots: + ~ContextAwareProfile(); void onIsRestrictedChanged(bool newValue); + QQmlContext* _context{ nullptr }; + std::atomic _isRestricted{ false }; + private: - RestrictedContextMonitor::TSharedPointer _monitor; + typedef QSet ContextAwareProfileSet; + typedef QMap ContextMap; + + static QReadWriteLock gl_contextMapProtect; + static ContextMap gl_contextMap; }; #endif // hifi_FileTypeProfile_h From 0c7aab1556e1e2a9186787fd251c23245e993a8f Mon Sep 17 00:00:00 2001 From: Heather Anderson Date: Sun, 2 Aug 2020 14:38:55 -0700 Subject: [PATCH 6/7] minor code review --- .../ui/src/ui/types/ContextAwareProfile.cpp | 40 +++++++++---------- .../ui/src/ui/types/ContextAwareProfile.h | 15 +++---- 2 files changed, 27 insertions(+), 28 deletions(-) diff --git a/libraries/ui/src/ui/types/ContextAwareProfile.cpp b/libraries/ui/src/ui/types/ContextAwareProfile.cpp index 5df2d34dfa..ee823ec784 100644 --- a/libraries/ui/src/ui/types/ContextAwareProfile.cpp +++ b/libraries/ui/src/ui/types/ContextAwareProfile.cpp @@ -22,19 +22,19 @@ static const QString RESTRICTED_FLAG_PROPERTY = "RestrictFileAccess"; -QReadWriteLock ContextAwareProfile::gl_contextMapProtect; -ContextAwareProfile::ContextMap ContextAwareProfile::gl_contextMap; +QReadWriteLock ContextAwareProfile::_global_contextMapProtect; +ContextAwareProfile::ContextMap ContextAwareProfile::_global_contextMap; ContextAwareProfile::ContextAwareProfile(QQmlContext* context) : ContextAwareProfileParent(context), _context(context) { assert(context); { // register our object for future updates - QWriteLocker guard(&gl_contextMapProtect); - ContextMap::iterator setLookup = gl_contextMap.find(_context); - if (setLookup == gl_contextMap.end()) { - setLookup = gl_contextMap.insert(_context, ContextAwareProfileSet()); + QWriteLocker guard(&_global_contextMapProtect); + ContextMap::iterator setLookup = _global_contextMap.find(_context); + if (setLookup == _global_contextMap.end()) { + setLookup = _global_contextMap.insert(_context, ContextAwareProfileSet()); } - assert(setLookup != gl_contextMap.end()); + assert(setLookup != _global_contextMap.end()); ContextAwareProfileSet& profileSet = setLookup.value(); assert(profileSet.find(this) == profileSet.end()); profileSet.insert(this); @@ -45,15 +45,15 @@ ContextAwareProfile::ContextAwareProfile(QQmlContext* context) : ContextAwarePro ContextAwareProfile::~ContextAwareProfile() { { // deregister our object - QWriteLocker guard(&gl_contextMapProtect); - ContextMap::iterator setLookup = gl_contextMap.find(_context); - assert(setLookup != gl_contextMap.end()); - if (setLookup != gl_contextMap.end()) { + QWriteLocker guard(&_global_contextMapProtect); + ContextMap::iterator setLookup = _global_contextMap.find(_context); + assert(setLookup != _global_contextMap.end()); + if (setLookup != _global_contextMap.end()) { ContextAwareProfileSet& profileSet = setLookup.value(); assert(profileSet.find(this) != profileSet.end()); profileSet.remove(this); if (profileSet.isEmpty()) { - gl_contextMap.erase(setLookup); + _global_contextMap.erase(setLookup); } } } @@ -65,15 +65,13 @@ void ContextAwareProfile::restrictContext(QQmlContext* context, bool restrict) { context->setContextProperty(RESTRICTED_FLAG_PROPERTY, restrict); // broadcast the new value to any registered ContextAwareProfile objects - { // deregister our object - QReadLocker guard(&gl_contextMapProtect); - ContextMap::const_iterator setLookup = gl_contextMap.find(context); - if (setLookup != gl_contextMap.end()) { - const ContextAwareProfileSet& profileSet = setLookup.value(); - for (ContextAwareProfileSet::const_iterator profileIterator = profileSet.begin(); - profileIterator != profileSet.end(); profileIterator++) { - (*profileIterator)->onIsRestrictedChanged(restrict); - } + QReadLocker guard(&_global_contextMapProtect); + ContextMap::const_iterator setLookup = _global_contextMap.find(context); + if (setLookup != _global_contextMap.end()) { + const ContextAwareProfileSet& profileSet = setLookup.value(); + for (ContextAwareProfileSet::const_iterator profileIterator = profileSet.begin(); + profileIterator != profileSet.end(); profileIterator++) { + (*profileIterator)->onIsRestrictedChanged(restrict); } } } diff --git a/libraries/ui/src/ui/types/ContextAwareProfile.h b/libraries/ui/src/ui/types/ContextAwareProfile.h index d8ec762858..c6f3020c4f 100644 --- a/libraries/ui/src/ui/types/ContextAwareProfile.h +++ b/libraries/ui/src/ui/types/ContextAwareProfile.h @@ -12,7 +12,7 @@ #define hifi_ContextAwareProfile_h #include -#include +#include #include #include #include @@ -50,17 +50,18 @@ protected: ContextAwareProfile(QQmlContext* parent); ~ContextAwareProfile(); - void onIsRestrictedChanged(bool newValue); + +private: + typedef QSet ContextAwareProfileSet; + typedef QHash ContextMap; QQmlContext* _context{ nullptr }; std::atomic _isRestricted{ false }; -private: - typedef QSet ContextAwareProfileSet; - typedef QMap ContextMap; + static QReadWriteLock _global_contextMapProtect; + static ContextMap _global_contextMap; - static QReadWriteLock gl_contextMapProtect; - static ContextMap gl_contextMap; + void onIsRestrictedChanged(bool newValue); }; #endif // hifi_FileTypeProfile_h From 37c613c3d440a7e2a80fa0b4133a35fc89661462 Mon Sep 17 00:00:00 2001 From: Heather Anderson Date: Mon, 3 Aug 2020 20:50:10 -0700 Subject: [PATCH 7/7] more changes from code review --- .../ui/src/ui/types/ContextAwareProfile.cpp | 47 +++++++++---------- .../ui/src/ui/types/ContextAwareProfile.h | 6 +-- 2 files changed, 23 insertions(+), 30 deletions(-) diff --git a/libraries/ui/src/ui/types/ContextAwareProfile.cpp b/libraries/ui/src/ui/types/ContextAwareProfile.cpp index ee823ec784..210e1f36b1 100644 --- a/libraries/ui/src/ui/types/ContextAwareProfile.cpp +++ b/libraries/ui/src/ui/types/ContextAwareProfile.cpp @@ -22,19 +22,19 @@ static const QString RESTRICTED_FLAG_PROPERTY = "RestrictFileAccess"; -QReadWriteLock ContextAwareProfile::_global_contextMapProtect; -ContextAwareProfile::ContextMap ContextAwareProfile::_global_contextMap; +QReadWriteLock ContextAwareProfile::_contextMapProtect; +ContextAwareProfile::ContextMap ContextAwareProfile::_contextMap; ContextAwareProfile::ContextAwareProfile(QQmlContext* context) : ContextAwareProfileParent(context), _context(context) { assert(context); { // register our object for future updates - QWriteLocker guard(&_global_contextMapProtect); - ContextMap::iterator setLookup = _global_contextMap.find(_context); - if (setLookup == _global_contextMap.end()) { - setLookup = _global_contextMap.insert(_context, ContextAwareProfileSet()); + QWriteLocker guard(&_contextMapProtect); + ContextMap::iterator setLookup = _contextMap.find(_context); + if (setLookup == _contextMap.end()) { + setLookup = _contextMap.insert(_context, ContextAwareProfileSet()); } - assert(setLookup != _global_contextMap.end()); + assert(setLookup != _contextMap.end()); ContextAwareProfileSet& profileSet = setLookup.value(); assert(profileSet.find(this) == profileSet.end()); profileSet.insert(this); @@ -44,17 +44,16 @@ ContextAwareProfile::ContextAwareProfile(QQmlContext* context) : ContextAwarePro } ContextAwareProfile::~ContextAwareProfile() { - { // deregister our object - QWriteLocker guard(&_global_contextMapProtect); - ContextMap::iterator setLookup = _global_contextMap.find(_context); - assert(setLookup != _global_contextMap.end()); - if (setLookup != _global_contextMap.end()) { - ContextAwareProfileSet& profileSet = setLookup.value(); - assert(profileSet.find(this) != profileSet.end()); - profileSet.remove(this); - if (profileSet.isEmpty()) { - _global_contextMap.erase(setLookup); - } + // deregister our object + QWriteLocker guard(&_contextMapProtect); + ContextMap::iterator setLookup = _contextMap.find(_context); + assert(setLookup != _contextMap.end()); + if (setLookup != _contextMap.end()) { + ContextAwareProfileSet& profileSet = setLookup.value(); + assert(profileSet.find(this) != profileSet.end()); + profileSet.remove(this); + if (profileSet.isEmpty()) { + _contextMap.erase(setLookup); } } } @@ -65,13 +64,13 @@ void ContextAwareProfile::restrictContext(QQmlContext* context, bool restrict) { context->setContextProperty(RESTRICTED_FLAG_PROPERTY, restrict); // broadcast the new value to any registered ContextAwareProfile objects - QReadLocker guard(&_global_contextMapProtect); - ContextMap::const_iterator setLookup = _global_contextMap.find(context); - if (setLookup != _global_contextMap.end()) { + QReadLocker guard(&_contextMapProtect); + ContextMap::const_iterator setLookup = _contextMap.find(context); + if (setLookup != _contextMap.end()) { const ContextAwareProfileSet& profileSet = setLookup.value(); for (ContextAwareProfileSet::const_iterator profileIterator = profileSet.begin(); profileIterator != profileSet.end(); profileIterator++) { - (*profileIterator)->onIsRestrictedChanged(restrict); + (*profileIterator)->_isRestricted.store(restrict); } } } @@ -93,10 +92,6 @@ bool ContextAwareProfile::isRestrictedGetProperty() { return true; } -void ContextAwareProfile::onIsRestrictedChanged(bool newValue) { - _isRestricted.store(newValue); -} - bool ContextAwareProfile::isRestricted() { return _isRestricted.load(); } diff --git a/libraries/ui/src/ui/types/ContextAwareProfile.h b/libraries/ui/src/ui/types/ContextAwareProfile.h index c6f3020c4f..486ac5481a 100644 --- a/libraries/ui/src/ui/types/ContextAwareProfile.h +++ b/libraries/ui/src/ui/types/ContextAwareProfile.h @@ -58,10 +58,8 @@ private: QQmlContext* _context{ nullptr }; std::atomic _isRestricted{ false }; - static QReadWriteLock _global_contextMapProtect; - static ContextMap _global_contextMap; - - void onIsRestrictedChanged(bool newValue); + static QReadWriteLock _contextMapProtect; + static ContextMap _contextMap; }; #endif // hifi_FileTypeProfile_h