Merge pull request #13027 from jherico/fix/web_crash2

Some fixes for QML surfaces

interface/resources/qml/controls/FlickableWebViewCore.qml

@@ -21,6 +21,7 @@ Item {
     signal newViewRequestedCallback(var request)
     signal loadingChangedCallback(var loadRequest)
 
+
     width: parent.width
 
     property bool interactive: false
@@ -29,6 +30,10 @@ Item {
         id: hifi
     }
 
+    function stop() {
+        webViewCore.stop();
+    }
+
     function unfocus() {
         webViewCore.runJavaScript("if (document.activeElement) document.activeElement.blur();", function(result) {
             console.log('unfocus completed: ', result);

interface/resources/qml/controls/WebView.qml

@@ -21,6 +21,10 @@ Item {
     property bool passwordField: false
     property alias flickable: webroot.interactive
 
+    function stop() {
+        webroot.stop();
+    }
+
     // FIXME - Keyboard HMD only: Make Interface either set keyboardRaised property directly in OffscreenQmlSurface
     // or provide HMDinfo object to QML in RenderableWebEntityItem and do the following.
     /*

libraries/entities-renderer/src/RenderableWebEntityItem.cpp

@@ -308,12 +308,7 @@ bool WebEntityRenderer::buildWebSurface(const TypedEntityPointer& entity) {
             item->setProperty(URL_PROPERTY, _lastSourceUrl);
         });
     } else if (_contentType == ContentType::QmlContent) {
-        _webSurface->load(_lastSourceUrl, [this](QQmlContext* context, QObject* item) {
-            if (item && item->objectName() == "tabletRoot") {
-                auto tabletScriptingInterface = DependencyManager::get<TabletScriptingInterface>();
-                tabletScriptingInterface->setQmlTabletRoot("com.highfidelity.interface.tablet.system", _webSurface.data());
-            }
-        });
+        _webSurface->load(_lastSourceUrl);
     }
     _fadeStartTime = usecTimestampNow();
     _webSurface->resume();
@@ -323,32 +318,21 @@ bool WebEntityRenderer::buildWebSurface(const TypedEntityPointer& entity) {
 
 void WebEntityRenderer::destroyWebSurface() {
     QSharedPointer<OffscreenQmlSurface> webSurface;
+    ContentType contentType{ ContentType::NoContent };
     withWriteLock([&] {
         webSurface.swap(_webSurface);
+        std::swap(contentType, _contentType);
     });
 
     if (webSurface) {
         --_currentWebCount;
         QQuickItem* rootItem = webSurface->getRootItem();
-        // Explicitly set the web URL to an empty string, in an effort to get a 
-        // faster shutdown of any chromium processes interacting with audio
-        if (rootItem && _contentType == ContentType::HtmlContent) {
-            rootItem->setProperty(URL_PROPERTY, "");
-        }
-
-        if (rootItem && rootItem->objectName() == "tabletRoot") {
-            auto tabletScriptingInterface = DependencyManager::get<TabletScriptingInterface>();
-            tabletScriptingInterface->setQmlTabletRoot("com.highfidelity.interface.tablet.system", nullptr);
-        }
 
         // Fix for crash in QtWebEngineCore when rapidly switching domains
         // Call stop on the QWebEngineView before destroying OffscreenQMLSurface.
-        if (rootItem) {
-            QObject* obj = rootItem->findChild<QObject*>("webEngineView");
-            if (obj) {
-                // stop loading
-                QMetaObject::invokeMethod(obj, "stop");
-            }
+        if (rootItem && contentType == ContentType::HtmlContent) {
+            // stop loading
+            QMetaObject::invokeMethod(rootItem, "stop");
         }
 
         webSurface->pause();

libraries/qml/src/qml/impl/SharedObject.cpp

@@ -90,19 +90,23 @@ SharedObject::~SharedObject() {
         _renderControl = nullptr;
     }
 
+    if (_rootItem) {
+        delete _rootItem;
+        _rootItem = nullptr;
+    }
+
     if (_quickWindow) {
         _quickWindow->destroy();
         delete _quickWindow;
         _quickWindow = nullptr;
     }
 
-    // _rootItem is parented to the quickWindow, so needs no explicit destruction
-    //if (_rootItem) {
-    //    delete _rootItem;
-    //    _rootItem = nullptr;
-    //}
-
-    releaseEngine(_qmlContext->engine());
+    if (_qmlContext) {
+        auto engine = _qmlContext->engine();
+        delete _qmlContext;
+        _qmlContext = nullptr;
+        releaseEngine(engine);
+    }
 }
 
 void SharedObject::create(OffscreenSurface* surface) {
@@ -210,9 +214,9 @@ QQmlEngine* SharedObject::acquireEngine(OffscreenSurface* surface) {
     if (!globalEngine) {
         Q_ASSERT(0 == globalEngineRefCount);
         globalEngine = new QQmlEngine();
-        surface->initializeQmlEngine(result);
-        ++globalEngineRefCount;
+        surface->initializeEngine(result);
     }
+    ++globalEngineRefCount;
     result = globalEngine;
 #else
     result = new QQmlEngine();

libraries/shared/src/Trace.h

@@ -102,6 +102,9 @@ private:
 };
 
 inline void traceEvent(const QLoggingCategory& category, const QString& name, EventType type, const QString& id = "", const QVariantMap& args = {}, const QVariantMap& extra = {}) {
+    if (!DependencyManager::isSet<Tracer>()) {
+        return;
+    }
     const auto& tracer = DependencyManager::get<Tracer>();
     if (tracer) {
         tracer->traceEvent(category, name, type, id, args, extra);

libraries/ui/src/ui/OffscreenQmlSurface.cpp

@@ -115,6 +115,7 @@ private:
 class UrlHandler : public QObject {
     Q_OBJECT
 public:
+    UrlHandler(QObject* parent = nullptr) : QObject(parent) {}
     Q_INVOKABLE bool canHandleUrl(const QString& url) {
         static auto handler = dynamic_cast<AbstractUriHandler*>(qApp);
         return handler && handler->canAcceptURL(url);
@@ -223,6 +224,17 @@ void AudioHandler::run() {
     qDebug() << "QML Audio changed to " << _newTargetDevice;
 }
 
+OffscreenQmlSurface::~OffscreenQmlSurface() {
+    clearFocusItem();
+}
+
+void OffscreenQmlSurface::clearFocusItem() {
+    if (_currentFocusItem) {
+        disconnect(_currentFocusItem, &QObject::destroyed, this, &OffscreenQmlSurface::focusDestroyed);
+    }
+    _currentFocusItem = nullptr;
+}
+
 void OffscreenQmlSurface::initializeEngine(QQmlEngine* engine) {
     Parent::initializeEngine(engine);
     new QQmlFileSelector(engine);
@@ -246,7 +258,7 @@ void OffscreenQmlSurface::initializeEngine(QQmlEngine* engine) {
 
     auto rootContext = engine->rootContext();
     rootContext->setContextProperty("GL", ::getGLContextData());
-    rootContext->setContextProperty("urlHandler", new UrlHandler());
+    rootContext->setContextProperty("urlHandler", new UrlHandler(rootContext));
     rootContext->setContextProperty("resourceDirectoryUrl", QUrl::fromLocalFile(PathUtils::resourcesPath()));
     rootContext->setContextProperty("ApplicationInterface", qApp);
     auto javaScriptToInject = getEventBridgeJavascript();
@@ -545,17 +557,15 @@ bool OffscreenQmlSurface::handlePointerEvent(const PointerEvent& event, class QT
 }
 
 void OffscreenQmlSurface::focusDestroyed(QObject* obj) {
-    if (_currentFocusItem) {
-        disconnect(_currentFocusItem, &QObject::destroyed, this, &OffscreenQmlSurface::focusDestroyed);
-    }
-    _currentFocusItem = nullptr;
+    clearFocusItem();
 }
 
 void OffscreenQmlSurface::onFocusObjectChanged(QObject* object) {
+    clearFocusItem();
+
     QQuickItem* item = static_cast<QQuickItem*>(object);
     if (!item) {
         setFocusText(false);
-        _currentFocusItem = nullptr;
         return;
     }
 
@@ -563,10 +573,6 @@ void OffscreenQmlSurface::onFocusObjectChanged(QObject* object) {
     qApp->sendEvent(object, &query);
     setFocusText(query.value(Qt::ImEnabled).toBool());
 
-    if (_currentFocusItem) {
-        disconnect(_currentFocusItem, &QObject::destroyed, this, 0);
-    }
-
     // Raise and lower keyboard for QML text fields.
     // HTML text fields are handled in emitWebEvent() methods - testing READ_ONLY_PROPERTY prevents action for HTML files.
     const char* READ_ONLY_PROPERTY = "readOnly";

libraries/ui/src/ui/OffscreenQmlSurface.h

@@ -22,7 +22,8 @@ class OffscreenQmlSurface : public hifi::qml::OffscreenSurface {
     Q_OBJECT
     Q_PROPERTY(bool focusText READ isFocusText NOTIFY focusTextChanged)
 public:
-    
+    ~OffscreenQmlSurface();
+
     static void addWhitelistContextHandler(const std::initializer_list<QUrl>& urls, const QmlContextCallback& callback);
     static void addWhitelistContextHandler(const QUrl& url, const QmlContextCallback& callback) { addWhitelistContextHandler({ { url } }, callback); };
 
@@ -58,6 +59,7 @@ public slots:
     void sendToQml(const QVariant& message);
 
 protected:
+    void clearFocusItem();
     void setFocusText(bool newFocusText);
     void initializeEngine(QQmlEngine* engine) override;
     void onRootContextCreated(QQmlContext* qmlContext) override;

tests/qml/qml/controls/WebEntityView.qml

@@ -0,0 +1,47 @@
+//
+//  WebEntityView.qml
+//
+//  Created by Kunal Gosar on 16 March 2017
+//  Copyright 2017 High Fidelity, Inc.
+//
+//  Distributed under the Apache License, Version 2.0.
+//  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
+//
+
+import QtQuick 2.5
+import QtWebEngine 1.5
+import Hifi 1.0
+
+/*
+TestItem {
+    Rectangle {
+        anchors.fill: parent
+        anchors.margins: 10
+        color: "blue"
+        property string url: ""
+        ColorAnimation on color {
+            loops: Animation.Infinite; from: "blue"; to: "yellow"; duration: 1000 
+        }
+    }
+}
+*/
+
+WebEngineView {
+    id: webViewCore
+    objectName: "webEngineView"
+    width: parent !== null ? parent.width : undefined
+    height: parent !== null ? parent.height : undefined
+
+    onFeaturePermissionRequested: {
+        grantFeaturePermission(securityOrigin, feature, true);
+    }
+
+    //disable popup
+    onContextMenuRequested: {
+        request.accepted = true;
+    }
+
+    onNewViewRequested: {
+        newViewRequestedCallback(request)
+    }
+}

tests/qml/src/main.cpp

@@ -28,52 +28,87 @@
 #include <QtGui/QImage>
 #include <QtGui/QOpenGLFunctions_4_5_Core>
 #include <QtGui/QOpenGLContext>
-
+#include <QtQuick/QQuickItem>
 #include <QtQml/QQmlContext>
 #include <QtQml/QQmlComponent>
 
+#include <SettingInterface.h>
+
 #include <gl/OffscreenGLCanvas.h>
-#include <GLMHelpers.h>
 #include <PathUtils.h>
 #include <NumericalConstants.h>
 #include <PerfStat.h>
 #include <PathUtils.h>
 #include <ViewFrustum.h>
 #include <qml/OffscreenSurface.h>
+#include <unordered_set>
+#include <array>
 
+namespace gl {
+extern void initModuleGl();
+}
 
-class OffscreenQmlSurface : public hifi::qml::OffscreenSurface {
+class QTestItem : public QQuickItem {
+    Q_OBJECT
+public:
+    QTestItem(QQuickItem* parent = nullptr) : QQuickItem(parent) { qDebug() << __FUNCTION__; }
 
+    ~QTestItem() { qDebug() << __FUNCTION__; }
+};
+
+QUrl getTestResource(const QString& relativePath) {
+    static QString dir;
+    if (dir.isEmpty()) {
+        QDir path(__FILE__);
+        path.cdUp();
+        dir = path.cleanPath(path.absoluteFilePath("../")) + "/";
+        qDebug() << "Resources Path: " << dir;
+    }
+    return QUrl::fromLocalFile(dir + relativePath);
+}
+
+#define DIVISIONS_X 5
+#define DIVISIONS_Y 5
+
+using QmlPtr = QSharedPointer<hifi::qml::OffscreenSurface>;
+using TextureAndFence = hifi::qml::OffscreenSurface::TextureAndFence;
+
+struct QmlInfo {
+    QmlPtr surface;
+    GLuint texture{ 0 };
+    uint64_t lifetime{ 0 };
 };
 
 class TestWindow : public QWindow {
-
 public:
     TestWindow();
 
-
 private:
-    using TextureAndFence = hifi::qml::OffscreenSurface::TextureAndFence;
     QOpenGLContext _glContext;
     OffscreenGLCanvas _sharedContext;
-    OffscreenQmlSurface _offscreenQml;
+    std::array<std::array<QmlInfo, DIVISIONS_Y>, DIVISIONS_X> _surfaces;
+
     QOpenGLFunctions_4_5_Core _glf;
-    uint32_t _currentTexture{ 0 };
-    GLsync _readFence{ 0 };
     std::function<void(uint32_t, void*)> _discardLamdba;
     QSize _size;
+    size_t _surfaceCount{ 0 };
     GLuint _fbo{ 0 };
     const QSize _qmlSize{ 640, 480 };
     bool _aboutToQuit{ false };
+    uint64_t _createStopTime;
     void initGl();
+    void updateSurfaces();
+    void buildSurface(QmlInfo& qmlInfo, bool allowVideo);
+    void destroySurface(QmlInfo& qmlInfo);
     void resizeWindow(const QSize& size);
     void draw();
     void resizeEvent(QResizeEvent* ev) override;
 };
 
 TestWindow::TestWindow() {
-    setSurfaceType(QSurface::OpenGLSurface);
+    Setting::init();
 
+    setSurfaceType(QSurface::OpenGLSurface);
     QSurfaceFormat format;
     format.setDepthBufferSize(24);
     format.setStencilBufferSize(8);
@@ -83,13 +118,16 @@ TestWindow::TestWindow() {
     QSurfaceFormat::setDefaultFormat(format);
     setFormat(format);
 
+    qmlRegisterType<QTestItem>("Hifi", 1, 0, "TestItem");
+
     show();
+    _createStopTime = usecTimestampNow() + (3000u * USECS_PER_SECOND);
 
     resize(QSize(800, 600));
 
     auto timer = new QTimer(this);
     timer->setTimerType(Qt::PreciseTimer);
-    timer->setInterval(5);
+    timer->setInterval(30);
     connect(timer, &QTimer::timeout, [&] { draw(); });
     timer->start();
 
@@ -97,7 +135,6 @@ TestWindow::TestWindow() {
         timer->stop();
         _aboutToQuit = true;
     });
-
 }
 
 void TestWindow::initGl() {
@@ -105,6 +142,7 @@ void TestWindow::initGl() {
     if (!_glContext.create() || !_glContext.makeCurrent(this)) {
         qFatal("Unable to intialize Window GL context");
     }
+    gl::initModuleGl();
 
     _glf.initializeOpenGLFunctions();
     _glf.glCreateFramebuffers(1, &_fbo);
@@ -113,15 +151,97 @@ void TestWindow::initGl() {
         qFatal("Unable to intialize Shared GL context");
     }
     hifi::qml::OffscreenSurface::setSharedContext(_sharedContext.getContext());
-    _discardLamdba = _offscreenQml.getDiscardLambda();
-    _offscreenQml.resize({ 640, 480 });
-    _offscreenQml.load(QUrl::fromLocalFile("C:/Users/bdavi/Git/hifi/tests/qml/qml/main.qml"));
+    _discardLamdba = hifi::qml::OffscreenSurface::getDiscardLambda();
 }
 
 void TestWindow::resizeWindow(const QSize& size) {
     _size = size;
 }
 
+static const int DEFAULT_MAX_FPS = 10;
+static const QString CONTROL_URL{ "/qml/controls/WebEntityView.qml" };
+static const char* URL_PROPERTY{ "url" };
+
+QString getSourceUrl(bool video) {
+    static const std::vector<QString> SOURCE_URLS{
+        "https://www.reddit.com/wiki/random",
+        "https://en.wikipedia.org/wiki/Wikipedia:Random",
+        "https://slashdot.org/",
+    };
+
+    static const std::vector<QString> VIDEO_SOURCE_URLS{
+        "https://www.youtube.com/watch?v=gDXwhHm4GhM",
+        "https://www.youtube.com/watch?v=Ch_hoYPPeGc",
+    };
+
+    const auto& sourceUrls = video ? VIDEO_SOURCE_URLS : SOURCE_URLS;
+    auto index = rand() % sourceUrls.size();
+    return sourceUrls[index];
+}
+
+void TestWindow::buildSurface(QmlInfo& qmlInfo, bool video) {
+    ++_surfaceCount;
+    auto lifetimeSecs = (uint32_t)(5.0f + (randFloat() * 10.0f));
+    auto lifetimeUsecs = (USECS_PER_SECOND * lifetimeSecs);
+    qmlInfo.lifetime = lifetimeUsecs + usecTimestampNow();
+    qmlInfo.texture = 0;
+    qmlInfo.surface.reset(new hifi::qml::OffscreenSurface());
+    qmlInfo.surface->load(getTestResource(CONTROL_URL), [video](QQmlContext* context, QQuickItem* item) {
+        item->setProperty(URL_PROPERTY, getSourceUrl(video));
+    });
+    qmlInfo.surface->setMaxFps(DEFAULT_MAX_FPS);
+    qmlInfo.surface->resize(_qmlSize);
+    qmlInfo.surface->resume();
+}
+
+void TestWindow::destroySurface(QmlInfo& qmlInfo) {
+    auto& surface = qmlInfo.surface;
+    auto webView = surface->getRootItem();
+    if (webView) {
+        // stop loading
+        QMetaObject::invokeMethod(webView, "stop");
+        webView->setProperty(URL_PROPERTY, "about:blank");
+    }
+    surface->pause();
+    surface.reset();
+}
+
+void TestWindow::updateSurfaces() {
+    auto now = usecTimestampNow();
+    // Fetch any new textures
+    for (size_t x = 0; x < DIVISIONS_X; ++x) {
+        for (size_t y = 0; y < DIVISIONS_Y; ++y) {
+            auto& qmlInfo = _surfaces[x][y];
+            if (!qmlInfo.surface) {
+                if (now < _createStopTime && randFloat() > 0.99f) {
+                    buildSurface(qmlInfo, x == 0 && y == 0);
+                } else {
+                    continue;
+                }
+            }
+
+            if (now > qmlInfo.lifetime) {
+                destroySurface(qmlInfo);
+                continue;
+            }
+
+            auto& surface = qmlInfo.surface;
+            auto& currentTexture = qmlInfo.texture;
+
+            TextureAndFence newTextureAndFence;
+            if (surface->fetchTexture(newTextureAndFence)) {
+                if (currentTexture != 0) {
+                    auto readFence = _glf.glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0);
+                    glFlush();
+                    _discardLamdba(currentTexture, readFence);
+                }
+                currentTexture = newTextureAndFence.first;
+                _glf.glWaitSync((GLsync)newTextureAndFence.second, 0, GL_TIMEOUT_IGNORED);
+            }
+        }
+    }
+}
+
 void TestWindow::draw() {
     if (_aboutToQuit) {
         return;
@@ -140,38 +260,30 @@ void TestWindow::draw() {
         return;
     }
 
+    updateSurfaces();
+
+    auto size = this->geometry().size();
+    auto incrementX = size.width() / DIVISIONS_X;
+    auto incrementY = size.height() / DIVISIONS_Y;
+    _glf.glViewport(0, 0, size.width(), size.height());
     _glf.glClearColor(1, 0, 0, 1);
     _glf.glClear(GL_COLOR_BUFFER_BIT);
-
-    TextureAndFence newTextureAndFence;
-    if (_offscreenQml.fetchTexture(newTextureAndFence)) {
-        if (_currentTexture) {
-            _discardLamdba(_currentTexture, _readFence);
-            _readFence = 0;
+    for (uint32_t x = 0; x < DIVISIONS_X; ++x) {
+        for (uint32_t y = 0; y < DIVISIONS_Y; ++y) {
+            auto& qmlInfo = _surfaces[x][y];
+            if (!qmlInfo.surface || !qmlInfo.texture) {
+                continue;
+            }
+            _glf.glNamedFramebufferTexture(_fbo, GL_COLOR_ATTACHMENT0, qmlInfo.texture, 0);
+            _glf.glBlitNamedFramebuffer(_fbo, 0,
+                                        // src coordinates
+                                        0, 0, _qmlSize.width() - 1, _qmlSize.height() - 1,
+                                        // dst coordinates
+                                        incrementX * x, incrementY * y, incrementX * (x + 1), incrementY * (y + 1),
+                                        // blit mask and filter
+                                        GL_COLOR_BUFFER_BIT, GL_NEAREST);
         }
-
-        _currentTexture = newTextureAndFence.first;
-        _glf.glWaitSync((GLsync)newTextureAndFence.second, 0, GL_TIMEOUT_IGNORED);
-        _glf.glNamedFramebufferTexture(_fbo, GL_COLOR_ATTACHMENT0, _currentTexture, 0);
     }
-    
-    auto diff = _size - _qmlSize;
-    diff /= 2;
-    auto qmlExtent = diff + _qmlSize;
-
-    if (_currentTexture) {
-        _glf.glBlitNamedFramebuffer(_fbo, 0, 
-            0, 0, _qmlSize.width() - 1, _qmlSize.height() - 1, 
-            diff.width(), diff.height(), qmlExtent.width() - 1, qmlExtent.height() - 2, 
-            GL_COLOR_BUFFER_BIT, GL_NEAREST);
-    }
-
-    if (_readFence) {
-        _glf.glDeleteSync(_readFence);
-    }
-    _readFence = _glf.glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0);
-    _glf.glFlush();
-
     _glContext.swapBuffers(this);
 }
 
@@ -180,11 +292,9 @@ void TestWindow::resizeEvent(QResizeEvent* ev) {
 }
 
 int main(int argc, char** argv) {
-    setupHifiApplication("QML Test");
-
     QGuiApplication app(argc, argv);
     TestWindow window;
-    app.exec();
-    return 0;
+    return app.exec();
 }
 
+#include "main.moc"