From baa4dc3383ccb6057b886b1b68230f0ab6141dab Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Wed, 14 Mar 2018 11:38:46 -0700 Subject: [PATCH] Don't leave trash web processes when changing web entity urls --- .../src/RenderableWebEntityItem.cpp | 114 ++++++++++++------ .../src/RenderableWebEntityItem.h | 14 ++- 2 files changed, 83 insertions(+), 45 deletions(-) diff --git a/libraries/entities-renderer/src/RenderableWebEntityItem.cpp b/libraries/entities-renderer/src/RenderableWebEntityItem.cpp index f7aaa43b7d..52b9364e98 100644 --- a/libraries/entities-renderer/src/RenderableWebEntityItem.cpp +++ b/libraries/entities-renderer/src/RenderableWebEntityItem.cpp @@ -46,6 +46,19 @@ static int YOUTUBE_MAX_FPS = 30; static QTouchDevice _touchDevice; +WebEntityRenderer::ContentType WebEntityRenderer::getContentType(const QString& urlString) { + if (urlString.isEmpty()) { + return ContentType::NoContent; + } + + const QUrl url(urlString); + if (url.scheme() == "http" || url.scheme() == "https" || + urlString.toLower().endsWith(".htm") || urlString.toLower().endsWith(".html")) { + return ContentType::HtmlContent; + } + return ContentType::QmlContent; +} + WebEntityRenderer::WebEntityRenderer(const EntityItemPointer& entity) : Parent(entity) { static std::once_flag once; std::call_once(once, [&]{ @@ -123,13 +136,45 @@ void WebEntityRenderer::onTimeout() { } void WebEntityRenderer::doRenderUpdateSynchronousTyped(const ScenePointer& scene, Transaction& transaction, const TypedEntityPointer& entity) { - withWriteLock([&] { - // This work must be done on the main thread - if (!hasWebSurface()) { - // If we couldn't create a new web surface, exit - if (!buildWebSurface(entity)) { - return; + // If the content type has changed, or the old content type was QML, we need to + // destroy the existing surface (because surfaces don't support changing the root + // object, so subsequent loads of content just overlap the existing content + bool urlChanged = false; + { + auto newSourceUrl = entity->getSourceUrl(); + auto newContentType = getContentType(newSourceUrl); + auto currentContentType = ContentType::NoContent; + withReadLock([&] { + urlChanged = _lastSourceUrl != newSourceUrl; + currentContentType = _contentType; + }); + + if (urlChanged) { + if (newContentType != ContentType::HtmlContent || currentContentType != ContentType::HtmlContent) { + destroyWebSurface(); } + + withWriteLock([&] { + _lastSourceUrl = newSourceUrl; + _contentType = newContentType; + }); + } + } + + + withWriteLock([&] { + if (_contentType == ContentType::NoContent) { + return; + } + + // This work must be done on the main thread + // If we couldn't create a new web surface, exit + if (!hasWebSurface() && !buildWebSurface(entity)) { + return; + } + + if (urlChanged) { + _webSurface->getRootItem()->setProperty("url", _lastSourceUrl); } if (_contextPosition != entity->getWorldPosition()) { @@ -138,11 +183,6 @@ void WebEntityRenderer::doRenderUpdateSynchronousTyped(const ScenePointer& scene _webSurface->getSurfaceContext()->setContextProperty("globalPosition", vec3toVariant(_contextPosition)); } - if (_lastSourceUrl != entity->getSourceUrl()) { - _lastSourceUrl = entity->getSourceUrl(); - loadSourceURL(); - } - _lastDPI = entity->getDPI(); _lastLocked = entity->getLocked(); @@ -232,9 +272,6 @@ bool WebEntityRenderer::buildWebSurface(const TypedEntityPointer& entity) { // Let us interact with the keyboard surfaceContext->setContextProperty("tabletInterface", DependencyManager::get().data()); }); - _fadeStartTime = usecTimestampNow(); - loadSourceURL(); - _webSurface->resume(); // forward web events to EntityScriptingInterface auto entities = DependencyManager::get(); @@ -243,6 +280,29 @@ bool WebEntityRenderer::buildWebSurface(const TypedEntityPointer& entity) { emit entities->webEventReceived(entityItemID, message); }); + if (_contentType == ContentType::HtmlContent) { + // We special case YouTube URLs since we know they are videos that we should play with at least 30 FPS. + // FIXME this doesn't handle redirects or shortened URLs, consider using a signaling method from the + // web entity + if (QUrl(_lastSourceUrl).host().endsWith("youtube.com", Qt::CaseInsensitive)) { + _webSurface->setMaxFps(YOUTUBE_MAX_FPS); + } else { + _webSurface->setMaxFps(DEFAULT_MAX_FPS); + } + _webSurface->load("controls/WebEntityView.qml", [this](QQmlContext* context, QObject* item) { + item->setProperty("url", _lastSourceUrl); + }); + } else if (_contentType == ContentType::QmlContent) { + _webSurface->load(_lastSourceUrl, [this](QQmlContext* context, QObject* item) { + if (item && item->objectName() == "tabletRoot") { + auto tabletScriptingInterface = DependencyManager::get(); + tabletScriptingInterface->setQmlTabletRoot("com.highfidelity.interface.tablet.system", _webSurface.data()); + } + }); + } + _fadeStartTime = usecTimestampNow(); + _webSurface->resume(); + return true; } @@ -289,32 +349,6 @@ glm::vec2 WebEntityRenderer::getWindowSize(const TypedEntityPointer& entity) con return dims; } -void WebEntityRenderer::loadSourceURL() { - const QUrl sourceUrl(_lastSourceUrl); - if (sourceUrl.scheme() == "http" || sourceUrl.scheme() == "https" || - _lastSourceUrl.toLower().endsWith(".htm") || _lastSourceUrl.toLower().endsWith(".html")) { - _contentType = htmlContent; - - // We special case YouTube URLs since we know they are videos that we should play with at least 30 FPS. - if (sourceUrl.host().endsWith("youtube.com", Qt::CaseInsensitive)) { - _webSurface->setMaxFps(YOUTUBE_MAX_FPS); - } else { - _webSurface->setMaxFps(DEFAULT_MAX_FPS); - } - - _webSurface->load("controls/WebEntityView.qml", [this](QQmlContext* context, QObject* item) { - item->setProperty("url", _lastSourceUrl); - }); - } else { - _contentType = qmlContent; - _webSurface->load(_lastSourceUrl); - if (_webSurface->getRootItem() && _webSurface->getRootItem()->objectName() == "tabletRoot") { - auto tabletScriptingInterface = DependencyManager::get(); - tabletScriptingInterface->setQmlTabletRoot("com.highfidelity.interface.tablet.system", _webSurface.data()); - } - } -} - void WebEntityRenderer::hoverEnterEntity(const PointerEvent& event) { if (!_lastLocked && _webSurface) { PointerEvent webEvent = event; diff --git a/libraries/entities-renderer/src/RenderableWebEntityItem.h b/libraries/entities-renderer/src/RenderableWebEntityItem.h index 309e750f53..3100014e9b 100644 --- a/libraries/entities-renderer/src/RenderableWebEntityItem.h +++ b/libraries/entities-renderer/src/RenderableWebEntityItem.h @@ -47,15 +47,19 @@ private: bool buildWebSurface(const TypedEntityPointer& entity); void destroyWebSurface(); bool hasWebSurface(); - void loadSourceURL(); glm::vec2 getWindowSize(const TypedEntityPointer& entity) const; + int _geometryId{ 0 }; - enum contentType { - htmlContent, - qmlContent + enum class ContentType { + NoContent, + HtmlContent, + QmlContent }; - contentType _contentType; + + static ContentType getContentType(const QString& urlString); + + ContentType _contentType{ ContentType::NoContent }; QSharedPointer _webSurface; glm::vec3 _contextPosition; gpu::TexturePointer _texture;