Merge pull request #7422 from jherico/tabs

Fix occasional crash on creating overlay tool windows

interface/resources/qml/ToolWindow.qml

@@ -17,7 +17,6 @@ Windows.Window {
     visible: false
     width: 384; height: 640;
     title: "Tools"
-    property string newTabSource
     property alias tabView: tabView
     onParentChanged: {
         if (parent) {
@@ -32,26 +31,21 @@ Windows.Window {
         property alias y: toolWindow.y
     }
 
-    property var webTabCreator: Component {
-        Controls.WebView {
-            id: webView
-            property string originalUrl;
-
-            // Both toolWindow.newTabSource and url can change, so we need
-            // to store the original url here, without creating any bindings
-            Component.onCompleted: {
-                originalUrl = toolWindow.newTabSource;
-                url = originalUrl;
-            }
-        }
-    }
-
     TabView {
         anchors.fill: parent
         id: tabView;
-        onCountChanged: {
-            if (0 == count) {
-                toolWindow.visible = false
+        Repeater {
+            model: 4
+            Tab {
+                active: true
+                enabled: false;
+                // we need to store the original url here for future identification
+                property string originalUrl: "";
+                onEnabledChanged: toolWindow.updateVisiblity();
+                Controls.WebView {
+                    id: webView;
+                    anchors.fill: parent
+                }
             }
         }
     }
@@ -68,8 +62,7 @@ Windows.Window {
     function findIndexForUrl(source) {
         for (var i = 0; i < tabView.count; ++i) {
             var tab = tabView.getTab(i);
-            if (tab && tab.item && tab.item.originalUrl &&
-                tab.item.originalUrl === source) {
+            if (tab.originalUrl === source) {
                 return i;
             }
         }
@@ -101,21 +94,32 @@ Windows.Window {
         }
     }
 
+    function findFreeTab() {
+        for (var i = 0; i < tabView.count; ++i) {
+            var tab = tabView.getTab(i);
+            if (tab && (!tab.originalUrl || tab.originalUrl === "")) {
+                return i;
+            }
+        }
+        console.warn("Could not find free tab");
+        return -1;
+    }
+
     function removeTabForUrl(source) {
         var index = findIndexForUrl(source);
         if (index < 0) {
             return;
         }
+
         var tab = tabView.getTab(index);
-        tab.enabledChanged.disconnect(updateVisiblity);
-        tabView.removeTab(index);
-        console.log("Updating visibility based on child tab removed");
-        updateVisiblity();
+        tab.title = "";
+        tab.originalUrl = "";
+        tab.enabled = false;
     }
 
     function addWebTab(properties) {
         if (!properties.source) {
-            console.warn("Attempted to open Web Tool Pane without URl")
+            console.warn("Attempted to open Web Tool Pane without URL")
             return;
         }
 
@@ -125,11 +129,17 @@ Windows.Window {
             return tabView.getTab(existingTabIndex);
         }
 
-        var title = properties.title || "Unknown";
-        newTabSource = properties.source;
-        var newTab = tabView.addTab(title, webTabCreator);
+        var freeTabIndex = findFreeTab();
+        if (freeTabIndex === -1) {
+            console.warn("Unable to add new tab");
+            return;
+        }
+
+        var newTab = tabView.getTab(freeTabIndex);
+        newTab.title = properties.title || "Unknown";
+        newTab.originalUrl = properties.source;
+        newTab.item.url = properties.source;
         newTab.active = true;
-        newTab.enabled = false;
 
         if (properties.width) {
             tabView.width = Math.min(Math.max(tabView.width, properties.width),

libraries/ui/src/QmlWindowClass.cpp

@@ -172,7 +172,6 @@ QScriptValue QmlWindowClass::internalConstructor(const QString& qmlSource,
             setupServer();
             retVal = builder(newTab);
             retVal->_toolWindow = true;
-            offscreenUi->getRootContext()->engine()->setObjectOwnership(retVal->_qmlWindow, QQmlEngine::CppOwnership);
             registerObject(url.toLower(), retVal);
             return QVariant();
         });
@@ -330,10 +329,8 @@ void QmlWindowClass::close() {
     if (_qmlWindow) {
         if (_toolWindow) {
             auto offscreenUi = DependencyManager::get<OffscreenUi>();
-            auto qmlWindow = _qmlWindow;
             offscreenUi->executeOnUiThread([=] {
                 auto toolWindow = offscreenUi->getToolWindow();
-                offscreenUi->getRootContext()->engine()->setObjectOwnership(qmlWindow, QQmlEngine::JavaScriptOwnership);
                 auto invokeResult = QMetaObject::invokeMethod(toolWindow, "removeTabForUrl", Qt::DirectConnection,
                     Q_ARG(QVariant, _source));
                 Q_ASSERT(invokeResult);