QmlDesigner: Prevent calling the node instance view directly

Views should not write to the node instance view directly. Instead they
should use the model for manipulation.

Change-Id: I3941cc691addab939b8032952230cbe03951730b
Reviewed-by: Tim Jenssen <tim.jenssen@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>

share/qtcreator/qml/qmlpuppet/interfaces/nodeinstanceglobal.h

@@ -50,4 +50,13 @@ enum class View3DActionType {
     SyncBackgroundColor,
     GetNodeAtPos
 };
+
+constexpr bool isNanotraceEnabled()
+{
+#ifdef NANOTRACE_ENABLED
+    return true;
+#else
+    return false;
+#endif
+}
 }

src/plugins/qmldesigner/components/componentcore/viewmanager.cpp

@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0+ OR GPL-3.0 WITH Qt-GPL-exception-1.0
 
 #include "viewmanager.h"
+#include "modelnodecontextmenu_helper.h"
 
 #include <abstractview.h>
 #include <assetslibraryview.h>
@@ -106,6 +107,8 @@ ViewManager::ViewManager(AsynchronousImageCache &imageCache,
                                                                ->mainWidget())
             designModeWidget->showDockWidget("TextEditor");
     });
+
+    registerNanotraceActions();
 }
 
 ViewManager::~ViewManager() = default;
@@ -117,7 +120,7 @@ DesignDocument *ViewManager::currentDesignDocument() const
 
 void ViewManager::attachNodeInstanceView()
 {
-    if (nodeInstanceView()->isAttached())
+    if (d->nodeInstanceView.isAttached())
         return;
 
     QElapsedTimer time;
@@ -229,6 +232,43 @@ QList<AbstractView *> ViewManager::standardViews() const
     return list;
 }
 
+void ViewManager::registerNanotraceActions()
+{
+    if constexpr (isNanotraceEnabled()) {
+        auto handleShutdownNanotraceAction = [](const SelectionContext &) {};
+        auto shutdownNanotraceIcon = []() { return QIcon(); };
+        auto startNanotraceAction = new ModelNodeAction("Start Nanotrace",
+                                                        QObject::tr("Start Nanotrace"),
+                                                        shutdownNanotraceIcon(),
+                                                        QObject::tr("Start Nanotrace"),
+                                                        ComponentCoreConstants::eventListCategory,
+                                                        QKeySequence(),
+                                                        220,
+                                                        handleShutdownNanotraceAction);
+
+        QObject::connect(startNanotraceAction->defaultAction(), &QAction::triggered, [&]() {
+            d->nodeInstanceView.startNanotrace();
+        });
+
+        d->designerActionManagerView.designerActionManager().addDesignerAction(startNanotraceAction);
+
+        auto shutDownNanotraceAction = new ModelNodeAction("ShutDown Nanotrace",
+                                                           QObject::tr("Shut Down Nanotrace"),
+                                                           shutdownNanotraceIcon(),
+                                                           QObject::tr("Shut Down Nanotrace"),
+                                                           ComponentCoreConstants::eventListCategory,
+                                                           QKeySequence(),
+                                                           220,
+                                                           handleShutdownNanotraceAction);
+
+        QObject::connect(shutDownNanotraceAction->defaultAction(), &QAction::triggered, [&]() {
+            d->nodeInstanceView.endNanotrace();
+        });
+
+        d->designerActionManagerView.designerActionManager().addDesignerAction(shutDownNanotraceAction);
+    }
+}
+
 void ViewManager::resetPropertyEditorView()
 {
     d->propertyEditorView.resetView();
@@ -411,7 +451,7 @@ void ViewManager::nextFileIsCalledInternally()
     crumbleBar()->nextFileIsCalledInternally();
 }
 
-NodeInstanceView *ViewManager::nodeInstanceView() const
+const AbstractView *ViewManager::view() const
 {
     return &d->nodeInstanceView;
 }

src/plugins/qmldesigner/components/componentcore/viewmanager.h

@@ -71,7 +71,7 @@ public:
     void pushInFileComponentOnCrumbleBar(const ModelNode &modelNode);
     void nextFileIsCalledInternally();
 
-    NodeInstanceView *nodeInstanceView() const;
+    const AbstractView *view() const;
 
     void exportAsImage();
     void reformatFileUsingTextEditorView();
@@ -108,6 +108,8 @@ private: // functions
     void switchStateEditorViewToSavedState();
     QList<AbstractView *> standardViews() const;
 
+    void registerNanotraceActions();
+
 private: // variables
     std::unique_ptr<ViewManagerData> d;
 };

src/plugins/qmldesigner/components/edit3d/edit3dview.cpp

@@ -840,8 +840,7 @@ void Edit3DView::dropMaterial(const ModelNode &matNode, const QPointF &pos)
 void Edit3DView::dropBundleMaterial(const QPointF &pos)
 {
     m_nodeAtPosReqType = NodeAtPosReqType::BundleMaterialDrop;
-    QmlDesignerPlugin::instance()->viewManager().nodeInstanceView()->view3DAction(
+    emitView3DAction(View3DActionType::GetNodeAtPos, pos);
-        View3DActionType::GetNodeAtPos, pos);
 }
 
 } // namespace QmlDesigner

src/plugins/qmldesigner/components/integration/designdocument.cpp

@@ -144,7 +144,7 @@ bool DesignDocument::loadInFileComponent(const ModelNode &componentNode)
 
 const AbstractView *DesignDocument::view() const
 {
-    return viewManager().nodeInstanceView();
+    return viewManager().view();
 }
 
 std::unique_ptr<Model> DesignDocument::createInFileComponentModel()

src/plugins/qmldesigner/components/stateseditor/stateseditorimageprovider.cpp

@@ -49,7 +49,7 @@ QImage StatesEditorImageProvider::requestImage(const QString &id, QSize *size, c
     return image;
 }
 
-void StatesEditorImageProvider::setNodeInstanceView(NodeInstanceView *nodeInstanceView)
+void StatesEditorImageProvider::setNodeInstanceView(const NodeInstanceView *nodeInstanceView)
 {
     m_nodeInstanceView = nodeInstanceView;
 }

src/plugins/qmldesigner/components/stateseditor/stateseditorimageprovider.h

@@ -20,10 +20,10 @@ public:
 
     QImage requestImage(const QString &id, QSize *size, const QSize &requestedSize) override;
 
-    void setNodeInstanceView(NodeInstanceView *nodeInstanceView);
+    void setNodeInstanceView(const NodeInstanceView *nodeInstanceView);
 
 private:
-    QPointer<NodeInstanceView> m_nodeInstanceView;
+    QPointer<const NodeInstanceView> m_nodeInstanceView;
 };
 
 } // namespace Internal

src/plugins/qmldesigner/components/stateseditor/stateseditorwidget.cpp

@@ -59,7 +59,7 @@ void StatesEditorWidget::setCurrentStateInternalId(int internalId)
     rootObject()->setProperty("currentStateInternalId", internalId);
 }
 
-void StatesEditorWidget::setNodeInstanceView(NodeInstanceView *nodeInstanceView)
+void StatesEditorWidget::setNodeInstanceView(const NodeInstanceView *nodeInstanceView)
 {
     m_imageProvider->setNodeInstanceView(nodeInstanceView);
 }

src/plugins/qmldesigner/components/stateseditor/stateseditorwidget.h

@@ -30,7 +30,7 @@ public:
 
     int currentStateInternalId() const;
     void setCurrentStateInternalId(int internalId);
-    void setNodeInstanceView(NodeInstanceView *nodeInstanceView);
+    void setNodeInstanceView(const NodeInstanceView *nodeInstanceView);
 
     void showAddNewStatesButton(bool showAddNewStatesButton);
 

src/plugins/qmldesigner/components/stateseditornew/stateseditorimageprovider.cpp

@@ -71,7 +71,7 @@ QImage StatesEditorImageProvider::requestImage(const QString &id, QSize *size, c
     return image;
 }
 
-void StatesEditorImageProvider::setNodeInstanceView(NodeInstanceView *nodeInstanceView)
+void StatesEditorImageProvider::setNodeInstanceView(const NodeInstanceView *nodeInstanceView)
 {
     m_nodeInstanceView = nodeInstanceView;
 }

src/plugins/qmldesigner/components/stateseditornew/stateseditorimageprovider.h

@@ -43,10 +43,10 @@ public:
 
     QImage requestImage(const QString &id, QSize *size, const QSize &requestedSize) override;
 
-    void setNodeInstanceView(NodeInstanceView *nodeInstanceView);
+    void setNodeInstanceView(const NodeInstanceView *nodeInstanceView);
 
 private:
-    QPointer<NodeInstanceView> m_nodeInstanceView;
+    QPointer<const NodeInstanceView> m_nodeInstanceView;
 };
 
 } // namespace Internal

src/plugins/qmldesigner/components/stateseditornew/stateseditorwidget.cpp

@@ -82,7 +82,7 @@ void StatesEditorWidget::setCurrentStateInternalId(int internalId)
     rootObject()->setProperty("currentStateInternalId", internalId);
 }
 
-void StatesEditorWidget::setNodeInstanceView(NodeInstanceView *nodeInstanceView)
+void StatesEditorWidget::setNodeInstanceView(const NodeInstanceView *nodeInstanceView)
 {
     m_imageProvider->setNodeInstanceView(nodeInstanceView);
 }

src/plugins/qmldesigner/components/stateseditornew/stateseditorwidget.h

@@ -55,7 +55,7 @@ public:
 
     int currentStateInternalId() const;
     void setCurrentStateInternalId(int internalId);
-    void setNodeInstanceView(NodeInstanceView *nodeInstanceView);
+    void setNodeInstanceView(const NodeInstanceView *nodeInstanceView);
 
     void showAddNewStatesButton(bool showAddNewStatesButton);
 

src/plugins/qmldesigner/designercore/include/abstractview.h

@@ -150,8 +150,6 @@ public:
     void emitNodeAtPosResult(const ModelNode &modelNode, const QVector3D &pos3d);
     void emitView3DAction(View3DActionType type, const QVariant &value);
 
-    void sendTokenToInstances(const QString &token, int number, const QVector<ModelNode> &nodeVector);
-
     virtual void modelAttached(Model *model);
     virtual void modelAboutToBeDetached(Model *model);
 
@@ -230,7 +228,7 @@ public:
     ModelNode materialLibraryNode();
     void assignMaterialTo3dModel(const ModelNode &modelNode, const ModelNode &materialNode = {});
 
-    NodeInstanceView *nodeInstanceView() const;
+    const NodeInstanceView *nodeInstanceView() const;
     RewriterView *rewriterView() const;
 
     void setCurrentStateNode(const ModelNode &node);

src/plugins/qmldesigner/designercore/include/model.h

@@ -127,7 +127,7 @@ public:
     RewriterView *rewriterView() const;
     void setRewriterView(RewriterView *rewriterView);
 
-    NodeInstanceView *nodeInstanceView() const;
+    const NodeInstanceView *nodeInstanceView() const;
     void setNodeInstanceView(NodeInstanceView *nodeInstanceView);
 
     Model *metaInfoProxyModel() const;

src/plugins/qmldesigner/designercore/include/nodeinstanceview.h

@@ -98,8 +98,8 @@ public:
     NodeInstance instanceForModelNode(const ModelNode &node) const ;
     bool hasInstanceForModelNode(const ModelNode &node) const;
 
-    NodeInstance instanceForId(qint32 id);
+    NodeInstance instanceForId(qint32 id) const;
-    bool hasInstanceForId(qint32 id);
+    bool hasInstanceForId(qint32 id) const;
 
     QRectF sceneRect() const;
 
@@ -131,13 +131,14 @@ public:
 
     void sendInputEvent(QInputEvent *e) const;
     void view3DAction(View3DActionType type, const QVariant &value) override;
-    void requestModelNodePreviewImage(const ModelNode &node, const ModelNode &renderNode);
+    void requestModelNodePreviewImage(const ModelNode &node, const ModelNode &renderNode) const;
     void edit3DViewResized(const QSize &size) const;
 
     void handlePuppetToCreatorCommand(const PuppetToCreatorCommand &command) override;
 
-    QVariant previewImageDataForGenericNode(const ModelNode &modelNode, const ModelNode &renderNode);
+    QVariant previewImageDataForGenericNode(const ModelNode &modelNode,
-    QVariant previewImageDataForImageNode(const ModelNode &modelNode);
+                                            const ModelNode &renderNode) const;
+    QVariant previewImageDataForImageNode(const ModelNode &modelNode) const;
 
     void setCrashCallback(std::function<void()> crashCallback)
     {
@@ -221,7 +222,7 @@ private: // functions
         QString id;
         QString info;
     };
-    QVariant modelNodePreviewImageDataToVariant(const ModelNodePreviewImageData &imageData);
+    QVariant modelNodePreviewImageDataToVariant(const ModelNodePreviewImageData &imageData) const;
     void updatePreviewImageForNode(const ModelNode &modelNode, const QImage &image);
 
     void updateWatcher(const QString &path);
@@ -250,7 +251,7 @@ private:
     QList<NodeInstance> loadInstancesFromCache(const QList<ModelNode> &nodeList,
                                                const NodeInstanceCacheData &cache);
 
-    QHash<QString, ModelNodePreviewImageData> m_imageDataMap;
+    mutable QHash<QString, ModelNodePreviewImageData> m_imageDataMap;
 
     NodeInstance m_rootNodeInstance;
     NodeInstance m_activeStateInstance;

src/plugins/qmldesigner/designercore/include/qmlmodelnodefacade.h

@@ -25,8 +25,8 @@ public:
 
     AbstractView *view() const;
     Model *model() const;
-    static NodeInstanceView *nodeInstanceView(const ModelNode &modelNode);
+    static const NodeInstanceView *nodeInstanceView(const ModelNode &modelNode);
-    NodeInstanceView *nodeInstanceView() const;
+    const NodeInstanceView *nodeInstanceView() const;
     bool isRootNode() const;
 
     static void enableUglyWorkaroundForIsValidQmlModelNodeFacadeInTests();

src/plugins/qmldesigner/designercore/instances/nodeinstanceview.cpp

@@ -734,9 +734,9 @@ void NodeInstanceView::currentStateChanged(const ModelNode &node)
     NodeInstance newStateInstance = instanceForModelNode(node);
 
     if (newStateInstance.isValid() && node.metaInfo().isQtQuickState())
-        nodeInstanceView()->activateState(newStateInstance);
+        activateState(newStateInstance);
     else
-        nodeInstanceView()->activateBaseState();
+        activateBaseState();
 }
 
 void NodeInstanceView::sceneCreated(const SceneCreatedCommand &) {}
@@ -780,7 +780,7 @@ bool NodeInstanceView::hasInstanceForModelNode(const ModelNode &node) const
     return m_nodeInstanceHash.contains(node);
 }
 
-NodeInstance NodeInstanceView::instanceForId(qint32 id)
+NodeInstance NodeInstanceView::instanceForId(qint32 id) const
 {
     if (id < 0 || !hasModelNodeForInternalId(id))
         return NodeInstance();
@@ -788,7 +788,7 @@ NodeInstance NodeInstanceView::instanceForId(qint32 id)
     return m_nodeInstanceHash.value(modelNodeForInternalId(id));
 }
 
-bool NodeInstanceView::hasInstanceForId(qint32 id)
+bool NodeInstanceView::hasInstanceForId(qint32 id) const
 {
     if (id < 0 || !hasModelNodeForInternalId(id))
         return false;
@@ -796,7 +796,6 @@ bool NodeInstanceView::hasInstanceForId(qint32 id)
     return m_nodeInstanceHash.contains(modelNodeForInternalId(id));
 }
 
-
 /*!
     Returns the root node instance of this view.
 
@@ -1743,7 +1742,8 @@ void NodeInstanceView::view3DAction(View3DActionType type, const QVariant &value
     m_nodeInstanceServer->view3DAction({type, value});
 }
 
-void NodeInstanceView::requestModelNodePreviewImage(const ModelNode &node, const ModelNode &renderNode)
+void NodeInstanceView::requestModelNodePreviewImage(const ModelNode &node,
+                                                    const ModelNode &renderNode) const
 {
     if (m_nodeInstanceServer && node.isValid()) {
         auto instance = instanceForModelNode(node);
@@ -1779,7 +1779,7 @@ void NodeInstanceView::timerEvent(QTimerEvent *event)
         restartProcess();
 }
 
-QVariant NodeInstanceView::modelNodePreviewImageDataToVariant(const ModelNodePreviewImageData &imageData)
+QVariant NodeInstanceView::modelNodePreviewImageDataToVariant(const ModelNodePreviewImageData &imageData) const
 {
     static QPixmap placeHolder;
     if (placeHolder.isNull()) {
@@ -1803,7 +1803,7 @@ QVariant NodeInstanceView::modelNodePreviewImageDataToVariant(const ModelNodePre
     return map;
 }
 
-QVariant NodeInstanceView::previewImageDataForImageNode(const ModelNode &modelNode)
+QVariant NodeInstanceView::previewImageDataForImageNode(const ModelNode &modelNode) const
 {
     if (!modelNode.isValid())
         return {};
@@ -1922,7 +1922,8 @@ void NodeInstanceView::endNanotrace()
     m_connectionManager.writeCommand(QVariant::fromValue(EndNanotraceCommand()) );
 }
 
-QVariant NodeInstanceView::previewImageDataForGenericNode(const ModelNode &modelNode, const ModelNode &renderNode)
+QVariant NodeInstanceView::previewImageDataForGenericNode(const ModelNode &modelNode,
+                                                          const ModelNode &renderNode) const
 {
     if (!modelNode.isValid())
         return {};

src/plugins/qmldesigner/designercore/model/abstractview.cpp

@@ -528,7 +528,7 @@ bool AbstractView::hasModelNodeForInternalId(qint32 internalId) const
     return model()->d->hasNodeForInternalId(internalId);
 }
 
-NodeInstanceView *AbstractView::nodeInstanceView() const
+const NodeInstanceView *AbstractView::nodeInstanceView() const
 {
     if (model())
         return model()->d->nodeInstanceView();
@@ -737,12 +737,6 @@ void AbstractView::emitRewriterBeginTransaction()
         model()->d->notifyRewriterBeginTransaction();
 }
 
-void AbstractView::sendTokenToInstances(const QString &token, int number, const QVector<ModelNode> &nodeVector)
-{
-    if (nodeInstanceView())
-        nodeInstanceView()->sendToken(token, number, nodeVector);
-}
-
 void AbstractView::emitInstanceToken(const QString &token, int number, const QVector<ModelNode> &nodeVector)
 {
     if (nodeInstanceView())

src/plugins/qmldesigner/designercore/model/model.cpp

@@ -1672,7 +1672,7 @@ void Model::setRewriterView(RewriterView *rewriterView)
     d->setRewriterView(rewriterView);
 }
 
-NodeInstanceView *Model::nodeInstanceView() const
+const NodeInstanceView *Model::nodeInstanceView() const
 {
     return d->nodeInstanceView();
 }

src/plugins/qmldesigner/designercore/model/qmlmodelnodefacade.cpp

@@ -19,12 +19,12 @@ Model *QmlModelNodeFacade::model() const
     return m_modelNode.model();
 }
 
-NodeInstanceView *QmlModelNodeFacade::nodeInstanceView(const ModelNode &modelNode)
+const NodeInstanceView *QmlModelNodeFacade::nodeInstanceView(const ModelNode &modelNode)
 {
     return modelNode.model()->nodeInstanceView();
 }
 
-NodeInstanceView *QmlModelNodeFacade::nodeInstanceView() const
+const NodeInstanceView *QmlModelNodeFacade::nodeInstanceView() const
 {
     return nodeInstanceView(m_modelNode);
 }

src/plugins/qmldesigner/qmldesignerplugin.cpp

@@ -229,39 +229,6 @@ bool QmlDesignerPlugin::initialize(const QStringList & /*arguments*/, QString *e
     if (QFontDatabase::addApplicationFont(fontPath) < 0)
         qCWarning(qmldesignerLog) << "Could not add font " << fontPath << "to font database";
 
-#ifdef NANOTRACE_ENABLED
-    auto handleShutdownNanotraceAction = [](const SelectionContext &) {};
-    auto shutdownNanotraceIcon = []() { return QIcon(); };
-    auto startNanotraceAction = new ModelNodeAction("Start Nanotrace",
-                     QObject::tr("Start Nanotrace"),
-                     shutdownNanotraceIcon(),
-                     QObject::tr("Start Nanotrace"),
-                     ComponentCoreConstants::eventListCategory,
-                     QKeySequence(),
-                     220,
-                     handleShutdownNanotraceAction);
-
-    connect(startNanotraceAction->defaultAction(), &QAction::triggered, [this]() {
-        d->viewManager.nodeInstanceView()->startNanotrace();
-    });
-
-    designerActionManager().addDesignerAction(startNanotraceAction);
-
-    auto shutDownNanotraceAction = new ModelNodeAction("ShutDown Nanotrace",
-                      QObject::tr("Shut Down Nanotrace"),
-                      shutdownNanotraceIcon(),
-                      QObject::tr("Shut Down Nanotrace"),
-                      ComponentCoreConstants::eventListCategory,
-                      QKeySequence(),
-                      220,
-                      handleShutdownNanotraceAction);
-
-    connect(shutDownNanotraceAction->defaultAction(), &QAction::triggered, [this]() {
-        d->viewManager.nodeInstanceView()->endNanotrace();
-    });
-
-    designerActionManager().addDesignerAction(shutDownNanotraceAction);
-#endif
     //TODO Move registering those types out of the property editor, since they are used also in the states editor
     Quick2PropertyEditorView::registerQmlTypes();
 

tests/auto/qml/qmldesigner/testview.cpp

@@ -198,7 +198,7 @@ QString TestView::lastFunction() const
     return m_methodCalls.last().name;
 }
 
-QmlDesigner::NodeInstanceView *TestView::nodeInstanceView() const
+const QmlDesigner::NodeInstanceView *TestView::nodeInstanceView() const
 {
     return QmlDesigner::AbstractView::nodeInstanceView();
 

tests/auto/qml/qmldesigner/testview.h

@@ -77,7 +77,7 @@ public:
 
     QString lastFunction() const;
 
-    QmlDesigner::NodeInstanceView *nodeInstanceView() const;
+    const QmlDesigner::NodeInstanceView *nodeInstanceView() const;
 
     QmlDesigner::QmlObjectNode rootQmlObjectNode() const;