From 28f25489f888abbdfbe02f20cdb215410b73834d Mon Sep 17 00:00:00 2001
From: Brad Davis <bdavis@saintandreas.org>
Date: Mon, 21 Mar 2016 18:46:04 -0700
Subject: [PATCH 1/2] Fix occasional crash on creating overlay tool windows

---
 interface/resources/qml/ToolWindow.qml | 68 +++++++++++++++-----------
 libraries/ui/src/QmlWindowClass.cpp    |  3 --
 2 files changed, 39 insertions(+), 32 deletions(-)

diff --git a/interface/resources/qml/ToolWindow.qml b/interface/resources/qml/ToolWindow.qml
index 404d1c1e78..09fbf78077 100644
--- a/interface/resources/qml/ToolWindow.qml
+++ b/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),
diff --git a/libraries/ui/src/QmlWindowClass.cpp b/libraries/ui/src/QmlWindowClass.cpp
index 5bd40b10a9..e986ea0441 100644
--- a/libraries/ui/src/QmlWindowClass.cpp
+++ b/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);

From a30df07d53443e89aaf37680eb5dc6535c3b69c0 Mon Sep 17 00:00:00 2001
From: Brad Davis <bdavis@saintandreas.org>
Date: Tue, 22 Mar 2016 09:15:29 -0700
Subject: [PATCH 2/2] PR feeback, precendence

---
 interface/resources/qml/ToolWindow.qml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/interface/resources/qml/ToolWindow.qml b/interface/resources/qml/ToolWindow.qml
index 09fbf78077..75aa50aa34 100644
--- a/interface/resources/qml/ToolWindow.qml
+++ b/interface/resources/qml/ToolWindow.qml
@@ -97,7 +97,7 @@ Windows.Window {
     function findFreeTab() {
         for (var i = 0; i < tabView.count; ++i) {
             var tab = tabView.getTab(i);
-            if (tab && !tab.originalUrl || tab.originalUrl === "") {
+            if (tab && (!tab.originalUrl || tab.originalUrl === "")) {
                 return i;
             }
         }