From b969a9b1e02689ea692d587529462a9f0f6ee972 Mon Sep 17 00:00:00 2001 From: dante ruiz Date: Tue, 10 Sep 2019 16:50:50 -0700 Subject: [PATCH 1/2] fix tablet html loading errors --- .../ui/src/ui/TabletScriptingInterface.cpp | 60 ++++++++++++++++++- .../ui/src/ui/TabletScriptingInterface.h | 4 ++ 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/libraries/ui/src/ui/TabletScriptingInterface.cpp b/libraries/ui/src/ui/TabletScriptingInterface.cpp index c54f63690d..6c57314367 100644 --- a/libraries/ui/src/ui/TabletScriptingInterface.cpp +++ b/libraries/ui/src/ui/TabletScriptingInterface.cpp @@ -795,11 +795,25 @@ void TabletProxy::loadWebScreenOnTop(const QVariant& url) { } void TabletProxy::loadWebScreenOnTop(const QVariant& url, const QString& injectJavaScriptUrl) { + bool localSafeContext = hifi::scripting::isLocalAccessSafeThread(); if (QThread::currentThread() != thread()) { - QMetaObject::invokeMethod(this, "loadWebScreenOnTop", Q_ARG(QVariant, url), Q_ARG(QString, injectJavaScriptUrl)); + QMetaObject::invokeMethod(this, "loadHTMLSourceImpl", Q_ARG(QVariant, url), Q_ARG(QString, injectJavaScriptUrl), Q_ARG(bool, localSafeContext)); return; } + loadHTMLSourceImpl(url, injectJavaScriptUrl, localSafeContext); +} + + + +void TabletProxy::loadHTMLSourceImpl(const QVariant& url, const QString& injectJavaScriptUrl, bool localSafeContext) { + if (QThread::currentThread() != thread()) { + qCWarning(uiLogging) << __FUNCTION__ << "may not be called directly by scripts"; + return; + + } + + QObject* root = nullptr; if (!_toolbarMode && _qmlTabletRoot) { root = _qmlTabletRoot; @@ -808,22 +822,59 @@ void TabletProxy::loadWebScreenOnTop(const QVariant& url, const QString& injectJ } if (root) { + if (localSafeContext) { + hifi::scripting::setLocalAccessSafeThread(true); + } QMetaObject::invokeMethod(root, "loadQMLOnTop", Q_ARG(const QVariant&, QVariant(WEB_VIEW_SOURCE_URL))); QMetaObject::invokeMethod(root, "setShown", Q_ARG(const QVariant&, QVariant(true))); if (_toolbarMode && _desktopWindow) { QMetaObject::invokeMethod(root, "setResizable", Q_ARG(const QVariant&, QVariant(false))); } QMetaObject::invokeMethod(root, "loadWebOnTop", Q_ARG(const QVariant&, QVariant(url)), Q_ARG(const QVariant&, QVariant(injectJavaScriptUrl))); + hifi::scripting::setLocalAccessSafeThread(false); } _state = State::Web; + /*QObject* root = nullptr; + if (!_toolbarMode && _qmlTabletRoot) { + root = _qmlTabletRoot; + } else if (_toolbarMode && _desktopWindow) { + root = _desktopWindow->asQuickItem(); + } + + if (root) { + // BUGZ-1398: tablet access to local HTML files from client scripts + // Here we TEMPORARILY mark the main thread as allowed to load local file content, + // because the thread that originally made the call is so marked. + if (localSafeContext) { + hifi::scripting::setLocalAccessSafeThread(true); + } + QMetaObject::invokeMethod(root, "loadSource", Q_ARG(const QVariant&, path)); + hifi::scripting::setLocalAccessSafeThread(false); + _state = State::QML; + _currentPathLoaded = path; + QMetaObject::invokeMethod(root, "setShown", Q_ARG(const QVariant&, QVariant(true))); + if (_toolbarMode && _desktopWindow) { + QMetaObject::invokeMethod(root, "setResizable", Q_ARG(const QVariant&, QVariant(resizable))); + } + + } else { + qCDebug(uiLogging) << "tablet cannot load QML because _qmlTabletRoot is null"; + }*/ } void TabletProxy::gotoWebScreen(const QString& url, const QString& injectedJavaScriptUrl, bool loadOtherBase) { + bool localSafeContext = hifi::scripting::isLocalAccessSafeThread(); if (QThread::currentThread() != thread()) { - QMetaObject::invokeMethod(this, "gotoWebScreen", Q_ARG(QString, url), Q_ARG(QString, injectedJavaScriptUrl), Q_ARG(bool, loadOtherBase)); + QMetaObject::invokeMethod(this, "loadHTMLSourceImpl", Q_ARG(QString, url), Q_ARG(QString, injectedJavaScriptUrl), Q_ARG(bool, loadOtherBase), Q_ARG(bool, localSafeContext)); return; } + + loadHTMLSourceImpl(url, injectedJavaScriptUrl, loadOtherBase, localSafeContext); +} + +void TabletProxy::loadHTMLSourceImpl(const QString& url, const QString& injectedJavaScriptUrl, bool loadOtherBase, bool localSafeContext) { + QObject* root = nullptr; if (!_toolbarMode && _qmlTabletRoot) { root = _qmlTabletRoot; @@ -832,6 +883,9 @@ void TabletProxy::gotoWebScreen(const QString& url, const QString& injectedJavaS } if (root) { + if (localSafeContext) { + hifi::scripting::setLocalAccessSafeThread(true); + } if (loadOtherBase) { QMetaObject::invokeMethod(root, "loadTabletWebBase", Q_ARG(const QVariant&, QVariant(url)), Q_ARG(const QVariant&, QVariant(injectedJavaScriptUrl))); } else { @@ -841,6 +895,8 @@ void TabletProxy::gotoWebScreen(const QString& url, const QString& injectedJavaS if (_toolbarMode && _desktopWindow) { QMetaObject::invokeMethod(root, "setResizable", Q_ARG(const QVariant&, QVariant(false))); } + + hifi::scripting::setLocalAccessSafeThread(false); _state = State::Web; _currentPathLoaded = QVariant(url); } else { diff --git a/libraries/ui/src/ui/TabletScriptingInterface.h b/libraries/ui/src/ui/TabletScriptingInterface.h index ba02ba25b0..9a5ff9efac 100644 --- a/libraries/ui/src/ui/TabletScriptingInterface.h +++ b/libraries/ui/src/ui/TabletScriptingInterface.h @@ -298,6 +298,10 @@ public: */ Q_INVOKABLE void loadQMLSourceImpl(const QVariant& path, bool resizable, bool localSafeContext); + Q_INVOKABLE void loadHTMLSourceImpl(const QVariant& url, const QString& injectJavaScriptUrl, bool localSafeContext); + + Q_INVOKABLE void loadHTMLSourceImpl(const QString& url, const QString& injectedJavaScriptUrl, bool loadOtherBase, bool localSafeContext); + // FIXME: This currently relies on a script initializing the tablet (hence the bool denoting success); // it should be initialized internally so it cannot fail From 8491a3792385a636298c527669a5ecbaec5ed4b6 Mon Sep 17 00:00:00 2001 From: danteruiz Date: Thu, 12 Sep 2019 17:12:50 -0700 Subject: [PATCH 2/2] fixing WebEntities html --- interface/resources/qml/Web3DSurface.qml | 33 ++++++++++++++----- .../src/RenderableWebEntityItem.cpp | 10 ++++++ libraries/entities/src/WebEntityItem.cpp | 10 ++++++ libraries/entities/src/WebEntityItem.h | 3 ++ .../ui/src/ui/TabletScriptingInterface.cpp | 26 --------------- 5 files changed, 47 insertions(+), 35 deletions(-) diff --git a/interface/resources/qml/Web3DSurface.qml b/interface/resources/qml/Web3DSurface.qml index 32c19daf14..ff574ceaa5 100644 --- a/interface/resources/qml/Web3DSurface.qml +++ b/interface/resources/qml/Web3DSurface.qml @@ -12,20 +12,35 @@ import QtQuick 2.5 import "controls" as Controls -Controls.WebView { +Item { + id: root + anchors.fill: parent + property string url: "" + property string scriptUrl: null - // This is for JS/QML communication, which is unused in a Web3DOverlay, - // but not having this here results in spurious warnings about a - // missing signal - signal sendToScript(var message); + onUrlChanged: { + load(root.url, root.scriptUrl); + } - function onWebEventReceived(event) { - if (event.slice(0, 17) === "CLARA.IO DOWNLOAD") { - ApplicationInterface.addAssetToWorldFromURL(event.slice(18)); + onScriptUrlChanged: { + if (root.item) { + root.item.scriptUrl = root.scriptUrl; + } else { + load(root.url, root.scriptUrl); } } + property var item: null + + function load(url, scriptUrl) { + QmlSurface.load("./controls/WebView.qml", root, function(newItem) { + root.item = newItem + root.item.url = url + root.item.scriptUrl = scriptUrl + }) + } + Component.onCompleted: { - eventBridge.webEventReceived.connect(onWebEventReceived); + load(root.url, root.scriptUrl); } } diff --git a/libraries/entities-renderer/src/RenderableWebEntityItem.cpp b/libraries/entities-renderer/src/RenderableWebEntityItem.cpp index a1d24fe52e..b1feddfd47 100644 --- a/libraries/entities-renderer/src/RenderableWebEntityItem.cpp +++ b/libraries/entities-renderer/src/RenderableWebEntityItem.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include "EntitiesRendererLogging.h" #include @@ -180,14 +181,23 @@ void WebEntityRenderer::doRenderUpdateSynchronousTyped(const ScenePointer& scene } // This work must be done on the main thread + bool localSafeContext = entity->getLocalSafeContext(); if (!_webSurface) { + if (localSafeContext) { + ::hifi::scripting::setLocalAccessSafeThread(true); + } buildWebSurface(entity, newSourceURL); + ::hifi::scripting::setLocalAccessSafeThread(false); } if (_webSurface) { if (_webSurface->getRootItem()) { if (_contentType == ContentType::HtmlContent && _sourceURL != newSourceURL) { + if (localSafeContext) { + ::hifi::scripting::setLocalAccessSafeThread(true); + } _webSurface->getRootItem()->setProperty(URL_PROPERTY, newSourceURL); + ::hifi::scripting::setLocalAccessSafeThread(false); _sourceURL = newSourceURL; } else if (_contentType != ContentType::HtmlContent) { _sourceURL = newSourceURL; diff --git a/libraries/entities/src/WebEntityItem.cpp b/libraries/entities/src/WebEntityItem.cpp index 186a8fa8b4..a62f599e4c 100644 --- a/libraries/entities/src/WebEntityItem.cpp +++ b/libraries/entities/src/WebEntityItem.cpp @@ -15,6 +15,7 @@ #include #include +#include #include "EntitiesLogging.h" #include "EntityItemProperties.h" @@ -31,6 +32,9 @@ EntityItemPointer WebEntityItem::factory(const EntityItemID& entityID, const Ent } WebEntityItem::WebEntityItem(const EntityItemID& entityItemID) : EntityItem(entityItemID) { + // this initialzation of localSafeContext is reading a thread-local variable and that is depends on + // the ctor being executed on the same thread as the script, assuming it's being create by a script + _localSafeContext = hifi::scripting::isLocalAccessSafeThread(); _type = EntityTypes::Web; } @@ -241,6 +245,12 @@ glm::u8vec3 WebEntityItem::getColor() const { }); } +bool WebEntityItem::getLocalSafeContext() const { + return resultWithReadLock([&] { + return _localSafeContext; + }); +} + void WebEntityItem::setAlpha(float alpha) { withWriteLock([&] { _needsRenderUpdate |= _alpha != alpha; diff --git a/libraries/entities/src/WebEntityItem.h b/libraries/entities/src/WebEntityItem.h index bb1e527712..b61e2b124f 100644 --- a/libraries/entities/src/WebEntityItem.h +++ b/libraries/entities/src/WebEntityItem.h @@ -74,6 +74,8 @@ public: void setScriptURL(const QString& value); QString getScriptURL() const; + bool getLocalSafeContext() const; + static const uint8_t DEFAULT_MAX_FPS; void setMaxFPS(uint8_t value); uint8_t getMaxFPS() const; @@ -98,6 +100,7 @@ protected: uint8_t _maxFPS; WebInputMode _inputMode; bool _showKeyboardFocusHighlight; + bool _localSafeContext { false }; }; #endif // hifi_WebEntityItem_h diff --git a/libraries/ui/src/ui/TabletScriptingInterface.cpp b/libraries/ui/src/ui/TabletScriptingInterface.cpp index 6c57314367..3465138e00 100644 --- a/libraries/ui/src/ui/TabletScriptingInterface.cpp +++ b/libraries/ui/src/ui/TabletScriptingInterface.cpp @@ -834,32 +834,6 @@ void TabletProxy::loadHTMLSourceImpl(const QVariant& url, const QString& injectJ hifi::scripting::setLocalAccessSafeThread(false); } _state = State::Web; - /*QObject* root = nullptr; - if (!_toolbarMode && _qmlTabletRoot) { - root = _qmlTabletRoot; - } else if (_toolbarMode && _desktopWindow) { - root = _desktopWindow->asQuickItem(); - } - - if (root) { - // BUGZ-1398: tablet access to local HTML files from client scripts - // Here we TEMPORARILY mark the main thread as allowed to load local file content, - // because the thread that originally made the call is so marked. - if (localSafeContext) { - hifi::scripting::setLocalAccessSafeThread(true); - } - QMetaObject::invokeMethod(root, "loadSource", Q_ARG(const QVariant&, path)); - hifi::scripting::setLocalAccessSafeThread(false); - _state = State::QML; - _currentPathLoaded = path; - QMetaObject::invokeMethod(root, "setShown", Q_ARG(const QVariant&, QVariant(true))); - if (_toolbarMode && _desktopWindow) { - QMetaObject::invokeMethod(root, "setResizable", Q_ARG(const QVariant&, QVariant(resizable))); - } - - } else { - qCDebug(uiLogging) << "tablet cannot load QML because _qmlTabletRoot is null"; - }*/ } void TabletProxy::gotoWebScreen(const QString& url, const QString& injectedJavaScriptUrl, bool loadOtherBase) {