Merge branch 'master' of https://github.com/highfidelity/hifi into red

libraries/animation/src/Rig.cpp

@@ -20,6 +20,7 @@
 #include <GeometryUtil.h>
 #include <NumericalConstants.h>
 #include <DebugDraw.h>
+#include <ScriptValueUtils.h>
 #include <shared/NsightHelpers.h>
 
 #include "AnimationLogging.h"
@@ -796,23 +797,35 @@ void Rig::computeMotionAnimationState(float deltaTime, const glm::vec3& worldPos
 
 // Allow script to add/remove handlers and report results, from within their thread.
 QScriptValue Rig::addAnimationStateHandler(QScriptValue handler, QScriptValue propertiesList) { // called in script thread
-    QMutexLocker locker(&_stateMutex);
+
-    // Find a safe id, even if there are lots of many scripts add and remove handlers repeatedly.
+    // validate argument types
-    while (!_nextStateHandlerId || _stateHandlers.contains(_nextStateHandlerId)) { // 0 is unused, and don't reuse existing after wrap.
+    if (handler.isFunction() && (isListOfStrings(propertiesList) || propertiesList.isUndefined() || propertiesList.isNull())) {
-      _nextStateHandlerId++;
+        QMutexLocker locker(&_stateMutex);
+        // Find a safe id, even if there are lots of many scripts add and remove handlers repeatedly.
+        while (!_nextStateHandlerId || _stateHandlers.contains(_nextStateHandlerId)) { // 0 is unused, and don't reuse existing after wrap.
+            _nextStateHandlerId++;
+        }
+        StateHandler& data = _stateHandlers[_nextStateHandlerId];
+        data.function = handler;
+        data.useNames = propertiesList.isArray();
+        if (data.useNames) {
+            data.propertyNames = propertiesList.toVariant().toStringList();
+        }
+        return QScriptValue(_nextStateHandlerId); // suitable for giving to removeAnimationStateHandler
+    } else {
+        qCWarning(animation) << "Rig::addAnimationStateHandler invalid arguments, expected (function, string[])";
+        return QScriptValue(QScriptValue::UndefinedValue);
     }
-    StateHandler& data = _stateHandlers[_nextStateHandlerId];
-    data.function = handler;
-    data.useNames = propertiesList.isArray();
-    if (data.useNames) {
-        data.propertyNames = propertiesList.toVariant().toStringList();
-    }
-    return QScriptValue(_nextStateHandlerId); // suitable for giving to removeAnimationStateHandler
 }
 
 void Rig::removeAnimationStateHandler(QScriptValue identifier) { // called in script thread
-    QMutexLocker locker(&_stateMutex);
+    // validate arguments
-    _stateHandlers.remove(identifier.isNumber() ? identifier.toInt32() : 0); // silently continues if handler not present. 0 is unused
+    if (identifier.isNumber()) {
+        QMutexLocker locker(&_stateMutex);
+        _stateHandlers.remove(identifier.toInt32()); // silently continues if handler not present. 0 is unused
+    } else {
+        qCWarning(animation) << "Rig::removeAnimationStateHandler invalid argument, expected a number";
+    }
 }
 
 void Rig::animationStateHandlerResult(int identifier, QScriptValue result) { // called synchronously from script

libraries/networking/src/ResourceCache.cpp

@@ -101,8 +101,7 @@ void ResourceCache::refresh(const QUrl& url) {
     if (resource) {
         resource->refresh();
     } else {
-        QWriteLocker locker(&_resourcesLock);
+        removeResource(url);
-        _resources.remove(url);
         resetResourceCounters();
     }
 }
@@ -196,11 +195,8 @@ void ResourceCache::addUnusedResource(const QSharedPointer<Resource>& resource)
     // If it doesn't fit or its size is unknown, remove it from the cache.
     if (resource->getBytes() == 0 || resource->getBytes() > _unusedResourcesMaxSize) {
         resource->setCache(nullptr);
-
+        removeResource(resource->getURL(), resource->getBytes());
-        _totalResourcesSize -= resource->getBytes();
-        _resources.remove(resource->getURL());
         resetResourceCounters();
-
         return;
     }
     reserveUnusedResource(resource->getBytes());
@@ -233,8 +229,7 @@ void ResourceCache::reserveUnusedResource(qint64 resourceSize) {
         it.value()->setCache(nullptr);
         auto size = it.value()->getBytes();
 
-        _totalResourcesSize -= size;
+        removeResource(it.value()->getURL(), size);
-        _resources.remove(it.value()->getURL());
 
         _unusedResourcesSize -= size;
         _unusedResources.erase(it);
@@ -259,6 +254,12 @@ void ResourceCache::resetResourceCounters() {
     emit dirty();
 }
 
+void ResourceCache::removeResource(const QUrl& url, qint64 size) {
+    QWriteLocker locker(&_resourcesLock);
+    _resources.remove(url);
+    _totalResourcesSize -= size;
+}
+
 void ResourceCache::updateTotalSize(const qint64& oldSize, const qint64& newSize) {
     _totalResourcesSize += (newSize - oldSize);
     emit dirty();
@@ -454,12 +455,12 @@ void Resource::refresh() {
 }
 
 void Resource::allReferencesCleared() {
-    if (_cache && isCacheable()) {
+    if (QThread::currentThread() != thread()) {
-        if (QThread::currentThread() != thread()) {
+        QMetaObject::invokeMethod(this, "allReferencesCleared");
-            QMetaObject::invokeMethod(this, "allReferencesCleared");
+        return;
-            return;
+    }
-        }
 
+    if (_cache && isCacheable()) {
         // create and reinsert new shared pointer 
         QSharedPointer<Resource> self(this, &Resource::allReferencesCleared);
         setSelf(self);
@@ -467,8 +468,13 @@ void Resource::allReferencesCleared() {
 
         // add to the unused list
         _cache->addUnusedResource(self);
-
     } else {
+        if (_cache) {
+            // remove from the cache
+            _cache->removeResource(getURL(), getBytes());
+            _cache->resetResourceCounters();
+        }
+
         deleteLater();
     }
 }

libraries/networking/src/ResourceCache.h

@@ -153,6 +153,7 @@ private:
     void reserveUnusedResource(qint64 resourceSize);
     void clearUnusedResource();
     void resetResourceCounters();
+    void removeResource(const QUrl& url, qint64 size = 0);
 
     QReadWriteLock _resourcesLock { QReadWriteLock::Recursive };
     QHash<QUrl, QWeakPointer<Resource>> _resources;

libraries/script-engine/src/ScriptEngine.cpp

@@ -784,7 +784,13 @@ void ScriptEngine::callAnimationStateHandler(QScriptValue callback, AnimVariantM
     callingArguments << javascriptParameters;
     assert(currentEntityIdentifier.isInvalidID()); // No animation state handlers from entity scripts.
     QScriptValue result = callback.call(QScriptValue(), callingArguments);
-    resultHandler(result);
+
+    // validate result from callback function.
+    if (result.isValid() && result.isObject()) {
+        resultHandler(result);
+    } else {
+        qCWarning(scriptengine) << "ScriptEngine::callAnimationStateHandler invalid return argument from callback, expected an object";
+    }
 }
 
 void ScriptEngine::timerFired() {

libraries/shared/src/ScriptValueUtils.cpp

@@ -0,0 +1,34 @@
+//
+//  ScriptValueUtils.cpp
+//  libraries/shared/src
+//
+//  Created by Anthony Thibault on 4/15/16.
+//  Copyright 2016 High Fidelity, Inc.
+//
+//  Utilities for working with QtScriptValues
+//
+//  Distributed under the Apache License, Version 2.0.
+//  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
+//
+
+#include "ScriptValueUtils.h"
+
+bool isListOfStrings(const QScriptValue& arg) {
+    if (!arg.isArray()) {
+        return false;
+    }
+
+    auto lengthProperty = arg.property("length");
+    if (!lengthProperty.isNumber()) {
+        return false;
+    }
+
+    int length = lengthProperty.toInt32();
+    for (int i = 0; i < length; i++) {
+        if (!arg.property(i).isString()) {
+            return false;
+        }
+    }
+
+    return true;
+}

libraries/shared/src/ScriptValueUtils.h

@@ -0,0 +1,21 @@
+//
+//  ScriptValueUtils.h
+//  libraries/shared/src
+//
+//  Created by Anthony Thibault on 4/15/16.
+//  Copyright 2016 High Fidelity, Inc.
+//
+//  Utilities for working with QtScriptValues
+//
+//  Distributed under the Apache License, Version 2.0.
+//  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
+//
+
+#ifndef hifi_ScriptValueUtils_h
+#define hifi_ScriptValueUtils_h
+
+#include <QScriptValue>
+
+bool isListOfStrings(const QScriptValue& value);
+
+#endif // #define hifi_ScriptValueUtils_h

libraries/shared/src/shared/RateCounter.h

@@ -22,10 +22,10 @@ class RateCounter {
 public:
     void increment(size_t count = 1) {
         auto now = usecTimestampNow();
-        auto currentIntervalMs = (uint32_t)((now - _start) / USECS_PER_MSEC);
+        float currentIntervalMs = (now - _start) / (float) USECS_PER_MSEC;
-        if (currentIntervalMs > INTERVAL) {
+        if (currentIntervalMs > (float) INTERVAL) {
             float currentCount = _count;
-            float intervalSeconds = (float)currentIntervalMs / (float)MSECS_PER_SECOND;
+            float intervalSeconds = currentIntervalMs / (float) MSECS_PER_SECOND;
             _rate = roundf(currentCount / intervalSeconds * _scale) / _scale;
             _start = now;
             _count = 0;