QmlDebugger: Refactored QmlEngine and QmlCppEngine

Reviewed by:  hjk

src/libs/qmljsdebugclient/qdeclarativedebug.cpp

@@ -49,6 +49,7 @@ class QDeclarativeEngineDebugClient : public QDeclarativeDebugClient
 {
 public:
     QDeclarativeEngineDebugClient(QDeclarativeDebugConnection *client, QDeclarativeEngineDebugPrivate *p);
+    ~QDeclarativeEngineDebugClient();
 
 protected:
     virtual void statusChanged(Status status);
@@ -96,6 +97,10 @@ QDeclarativeEngineDebugClient::QDeclarativeEngineDebugClient(QDeclarativeDebugCo
 {
 }
 
+QDeclarativeEngineDebugClient::~QDeclarativeEngineDebugClient()
+{
+}
+
 void QDeclarativeEngineDebugClient::statusChanged(Status status)
 {
     if (priv)

src/libs/qmljsdebugclient/qdeclarativedebugclient.cpp

@@ -59,7 +59,7 @@ public:
     QDeclarativeDebugClientPrivate();
 
     QString name;
-    QDeclarativeDebugConnection *client;
+    QDeclarativeDebugConnection *connection;
 };
 
 class QDeclarativeDebugConnectionPrivate : public QObject
@@ -199,7 +199,7 @@ QDeclarativeDebugConnection::~QDeclarativeDebugConnection()
 {
     QHash<QString, QDeclarativeDebugClient*>::iterator iter = d->plugins.begin();
     for (; iter != d->plugins.end(); ++iter) {
-         iter.value()->d_func()->client = 0;
+         iter.value()->d_func()->connection = 0;
          iter.value()->statusChanged(QDeclarativeDebugClient::NotConnected);
     }
 }
@@ -210,50 +210,54 @@ bool QDeclarativeDebugConnection::isConnected() const
 }
 
 QDeclarativeDebugClientPrivate::QDeclarativeDebugClientPrivate()
-: client(0)
+: connection(0)
 {
 }
 
 QDeclarativeDebugClient::QDeclarativeDebugClient(const QString &name, 
                                            QDeclarativeDebugConnection *parent)
-: QObject(parent), d(new QDeclarativeDebugClientPrivate())
+: QObject(parent), d_ptr(new QDeclarativeDebugClientPrivate())
 {
+    Q_D(QDeclarativeDebugClient);
     d->name = name;
-    d->client = parent;
+    d->connection = parent;
 
-    if (!d->client)
+    if (!d->connection)
         return;
 
-    if (d->client->d->plugins.contains(name)) {
+    if (d->connection->d->plugins.contains(name)) {
         qWarning() << "QDeclarativeDebugClient: Conflicting plugin name" << name;
-        d->client = 0;
+        d->connection = 0;
     } else {
-        d->client->d->plugins.insert(name, this);
-        d->client->d->advertisePlugins();
+        d->connection->d->plugins.insert(name, this);
+        d->connection->d->advertisePlugins();
     }
 }
 
 QDeclarativeDebugClient::~QDeclarativeDebugClient()
 {
-    if (d->client && d->client->d) {
-        d->client->d->plugins.remove(d->name);
-        d->client->d->advertisePlugins();
+    Q_D(QDeclarativeDebugClient);
+    if (d->connection && d->connection->d) {
+        d->connection->d->plugins.remove(d->name);
+        d->connection->d->advertisePlugins();
     }
 }
 
 QString QDeclarativeDebugClient::name() const
 {
+    Q_D(const QDeclarativeDebugClient);
     return d->name;
 }
 
 QDeclarativeDebugClient::Status QDeclarativeDebugClient::status() const
 {
-    if (!d->client
-        || !d->client->isConnected()
-        || !d->client->d->gotHello)
+    Q_D(const QDeclarativeDebugClient);
+    if (!d->connection
+        || !d->connection->isConnected()
+        || !d->connection->d->gotHello)
         return NotConnected;
 
-    if (d->client->d->serverPlugins.contains(d->name))
+    if (d->connection->d->serverPlugins.contains(d->name))
         return Enabled;
 
     return Unavailable;
@@ -261,13 +265,14 @@ QDeclarativeDebugClient::Status QDeclarativeDebugClient::status() const
 
 void QDeclarativeDebugClient::sendMessage(const QByteArray &message)
 {
+    Q_D(QDeclarativeDebugClient);
     if (status() != Enabled)
         return;
 
     QPacket pack;
     pack << d->name << message;
-    d->client->d->protocol->send(pack);
-    d->client->flush();
+    d->connection->d->protocol->send(pack);
+    d->connection->flush();
 }
 
 void QDeclarativeDebugClient::statusChanged(Status)

src/libs/qmljsdebugclient/qdeclarativedebugclient_p.h

@@ -90,7 +90,7 @@ protected:
 private:
     friend class QDeclarativeDebugConnection;
     friend class QDeclarativeDebugConnectionPrivate;
-    QScopedPointer<QDeclarativeDebugClientPrivate> d;
+    QScopedPointer<QDeclarativeDebugClientPrivate> d_ptr;
 };
 
 }

src/plugins/debugger/debuggerengine.cpp

@@ -758,7 +758,7 @@ void DebuggerEngine::notifyEngineRunFailed()
     d->m_progress.reportCanceled();
     d->m_progress.reportFinished();
     setState(EngineRunFailed);
-    d->queueShutdownEngine();
+    d->queueShutdownInferior();
 }
 
 void DebuggerEngine::notifyEngineRunAndInferiorRunOk()
@@ -980,8 +980,8 @@ void DebuggerEngine::notifyInferiorExited()
 
 void DebuggerEngine::setState(DebuggerState state, bool forced)
 {
-    //qDebug() << "STATUS CHANGE: FROM " << stateName(d->m_state)
-    //         << " TO " << stateName(state);
+    qDebug() << "STATUS CHANGE: FROM " << stateName(d->m_state)
+             << " TO " << stateName(state);
 
     DebuggerState oldState = d->m_state;
     d->m_state = state;

src/plugins/debugger/qml/qmlcppengine.cpp

@@ -30,27 +30,24 @@ DebuggerEngine *createQmlCppEngine(const DebuggerStartParameters &sp)
 
 } // namespace Internal
 
-struct QmlCppEnginePrivate
-{
+class QmlCppEnginePrivate {
+public:
     QmlCppEnginePrivate();
+    ~QmlCppEnginePrivate() {}
 
+    friend class QmlCppEngine;
+private:
     QmlEngine *m_qmlEngine;
     DebuggerEngine *m_cppEngine;
     DebuggerEngine *m_activeEngine;
-    bool m_shutdownOk;
-    bool m_shutdownDeferred;
-    bool m_shutdownDone;
-    bool m_isInitialStartup;
+    DebuggerState m_errorState;
 };
 
-QmlCppEnginePrivate::QmlCppEnginePrivate()
-    : m_qmlEngine(0)
-    , m_cppEngine(0)
-    , m_activeEngine(0)
-    , m_shutdownOk(true)
-    , m_shutdownDeferred(false)
-    , m_shutdownDone(false)
-    , m_isInitialStartup(true)
+QmlCppEnginePrivate::QmlCppEnginePrivate() :
+    m_qmlEngine(0),
+    m_cppEngine(0),
+    m_activeEngine(0),
+    m_errorState(InferiorRunOk)
 {
 }
 
@@ -58,7 +55,6 @@ QmlCppEngine::QmlCppEngine(const DebuggerStartParameters &sp)
     : DebuggerEngine(sp), d(new QmlCppEnginePrivate)
 {
     d->m_qmlEngine = qobject_cast<QmlEngine*>(Internal::createQmlEngine(sp));
-    d->m_qmlEngine->setAttachToRunningExternalApp(true);
 
     if (startParameters().cppEngineType == GdbEngineType) {
         d->m_cppEngine = Internal::createGdbEngine(sp);
@@ -87,10 +83,10 @@ QmlCppEngine::QmlCppEngine(const DebuggerStartParameters &sp)
 
 QmlCppEngine::~QmlCppEngine()
 {
-    delete d->m_qmlEngine;
-    d->m_qmlEngine = 0;
-    delete d->m_cppEngine;
-    d->m_cppEngine = 0;
+    if (d->m_qmlEngine)
+        delete d->m_qmlEngine;
+    if (d->m_cppEngine)
+        delete d->m_cppEngine;
 }
 
 void QmlCppEngine::editorChanged(Core::IEditor *editor)
@@ -128,6 +124,15 @@ void QmlCppEngine::setActiveEngine(DebuggerLanguage language)
     }
 }
 
+DebuggerLanguage QmlCppEngine::activeEngine() const
+{
+    if (d->m_activeEngine == d->m_cppEngine)
+        return CppLanguage;
+    if (d->m_activeEngine == d->m_qmlEngine)
+        return QmlLanguage;
+    return AnyLanguage;
+}
+
 void QmlCppEngine::setToolTipExpression(const QPoint & mousePos,
         TextEditor::ITextEditor *editor, int cursorPos)
 {
@@ -332,17 +337,31 @@ void QmlCppEngine::executeReturn()
 
 void QmlCppEngine::continueInferior()
 {
-    d->m_activeEngine->continueInferior();
+    if (d->m_activeEngine->state() == InferiorStopOk) {
+        d->m_activeEngine->continueInferior();
+    } else {
+        notifyInferiorRunRequested();
+    }
 }
 
 void QmlCppEngine::interruptInferior()
 {
-    d->m_activeEngine->interruptInferior();
+    if (d->m_activeEngine->state() == InferiorRunOk) {
+        d->m_activeEngine->requestInterruptInferior();
+    } else {
+        if (d->m_activeEngine->state() == InferiorStopOk && (!checkErrorState(InferiorStopFailed))) {
+            notifyInferiorStopOk();
+        }
+    }
 }
 
 void QmlCppEngine::requestInterruptInferior()
 {
-    d->m_activeEngine->requestInterruptInferior();
+    DebuggerEngine::requestInterruptInferior();
+
+    if (d->m_activeEngine->state() == InferiorRunOk) {
+        d->m_activeEngine->requestInterruptInferior();
+    }
 }
 
 void QmlCppEngine::executeRunToLine(const QString &fileName, int lineNumber)
@@ -375,6 +394,50 @@ void QmlCppEngine::frameDown()
     d->m_activeEngine->frameDown();
 }
 
+/////////////////////////////////////////////////////////
+
+bool QmlCppEngine::checkErrorState(const DebuggerState stateToCheck)
+{
+    if (d->m_errorState != stateToCheck)
+        return false;
+
+    // reset state ( so that more than one error can accumulate over time )
+    d->m_errorState = InferiorRunOk;
+    switch (stateToCheck) {
+    case InferiorRunOk:
+        // nothing to do
+        break;
+    case EngineRunFailed:
+        notifyEngineRunFailed();
+        break;
+    case EngineSetupFailed:
+        notifyEngineSetupFailed();
+        break;
+    case EngineShutdownFailed:
+        notifyEngineShutdownFailed();
+        break;
+    case InferiorSetupFailed:
+        notifyInferiorSetupFailed();
+        break;
+    case InferiorRunFailed:
+        notifyInferiorRunFailed();
+        break;
+    case InferiorUnrunnable:
+        notifyInferiorUnrunnable();
+        break;
+    case InferiorStopFailed:
+        notifyInferiorStopFailed();
+        break;
+    case InferiorShutdownFailed:
+        notifyInferiorShutdownFailed();
+        break;
+    default:
+        // unexpected
+        break;
+    }
+    return true;
+}
+
 void QmlCppEngine::notifyInferiorRunOk()
 {
     DebuggerEngine::notifyInferiorRunOk();
@@ -390,56 +453,68 @@ void QmlCppEngine::setupEngine()
 
 void QmlCppEngine::setupInferior()
 {
-    // called through notifyEngineSetupOk
+    if (!checkErrorState(InferiorSetupFailed)) {
+        notifyInferiorSetupOk();
+    }
 }
 
 void QmlCppEngine::runEngine()
 {
-    // should never happen
-    showMessage(QString(Q_FUNC_INFO), LogError);
+    if (!checkErrorState(EngineRunFailed)) {
+        if (d->m_errorState == InferiorRunOk) {
+            switch (d->m_activeEngine->state()) {
+            case InferiorRunOk:
+                notifyEngineRunAndInferiorRunOk();
+                break;
+            case InferiorStopOk:
+                notifyEngineRunAndInferiorStopOk();
+                break;
+            default: // not supported?
+                notifyEngineRunFailed();
+                break;
+            }
+        } else {
+            notifyEngineRunFailed();
+        }
+    }
 }
 
 void QmlCppEngine::shutdownInferior()
 {
-    // user wants to stop inferior: always use cpp engine for this.
-    if (d->m_activeEngine == d->m_qmlEngine) {
-        d->m_activeEngine = d->m_cppEngine;
-
-        // we end up in this state after trying to shut down while debugging qml.
-        // b/c qml does not shutdown by itself, restore previous state and continue.
-        if (d->m_qmlEngine->state() == InferiorShutdownRequested) {
-            d->m_qmlEngine->setState(InferiorStopOk, true);
-        }
-
-        if (d->m_qmlEngine->state() == InferiorStopOk) {
-            d->m_qmlEngine->continueInferior();
+    if (!checkErrorState(InferiorShutdownFailed)) {
+        if (d->m_cppEngine->state() == InferiorStopOk) {
+            d->m_cppEngine->shutdownInferior();
+        } else {
+            notifyInferiorShutdownOk();
         }
     }
-    if (d->m_cppEngine->state() == InferiorRunOk) {
-        // first interrupt c++ engine; when done, we can shutdown.
-        d->m_shutdownDeferred = true;
-        d->m_cppEngine->requestInterruptInferior();
-    }
-    if (!d->m_shutdownDeferred)
-        d->m_cppEngine->shutdownInferior();
 }
 
-void QmlCppEngine::shutdownEngine()
+void QmlCppEngine::initEngineShutdown()
 {
-    d->m_cppEngine->shutdownEngine();
-    d->m_qmlEngine->shutdownEngineAsSlave();
-    notifyEngineShutdownOk();
+    if (d->m_qmlEngine->state() != DebuggerFinished) {
+        d->m_qmlEngine->quitDebugger();
+    } else
+    if (d->m_cppEngine->state() != DebuggerFinished) {
+        d->m_cppEngine->quitDebugger();
+    } else
+    if (state() == EngineSetupRequested) {
+        if (!checkErrorState(EngineSetupFailed)) {
+            notifyEngineSetupOk();
+        }
+    } else
+    if (state() == InferiorStopRequested) {
+        checkErrorState(InferiorStopFailed);
+    } else {
+        quitDebugger();
+    }
 }
 
-void QmlCppEngine::finishDebugger()
+void QmlCppEngine::shutdownEngine()
 {
-    if (!d->m_shutdownDone) {
-        d->m_shutdownDone = true;
-        if (d->m_shutdownOk) {
-            notifyEngineShutdownOk();
-        } else {
-            notifyEngineShutdownFailed();
-        }
+    if (!checkErrorState(EngineShutdownFailed)) {
+        showStatusMessage(tr("Debugging finished"));
+        notifyEngineShutdownOk();
     }
 }
 
@@ -450,227 +525,103 @@ void QmlCppEngine::setupSlaveEngineOnTimer()
 
 void QmlCppEngine::setupSlaveEngine()
 {
-    if (state() == InferiorRunRequested)
+    if (d->m_qmlEngine->state() == DebuggerNotReady)
         d->m_qmlEngine->startDebugger(runControl());
 }
 
 void QmlCppEngine::masterEngineStateChanged(const DebuggerState &newState)
 {
-    //qDebug() << "gdb state set to" << newState;
-
-    switch(newState) {
-    case EngineSetupRequested:
-        // go through this state
-        break;
-
-    case EngineSetupFailed:
-        notifyEngineSetupFailed();
-        break;
-
-    case EngineSetupOk:
-        notifyEngineSetupOk();
-        break;
-
-    case InferiorSetupRequested:
-        // go through this state
-        break;
-
-    case InferiorSetupFailed:
-        notifyInferiorSetupFailed();
-        break;
-
-    case EngineRunRequested:
-        setState(newState);
-        break;
-
-    case EngineRunFailed:
-        notifyEngineRunFailed();
-        break;
+    if (newState == InferiorStopOk) {
+        setActiveEngine(CppLanguage);
+    }
+    engineStateChanged(newState);
+}
 
-    case InferiorUnrunnable:
-        setState(newState);
-        break;
+void QmlCppEngine::slaveEngineStateChanged(const DebuggerState &newState)
+{
+    if (newState == InferiorStopOk) {
+        setActiveEngine(QmlLanguage);
+    }
+    engineStateChanged(newState);
+}
 
-    case InferiorRunRequested:
-        setState(newState);
-        break;
 
+void QmlCppEngine::engineStateChanged(const DebuggerState &newState)
+{
+    switch (newState) {
     case InferiorRunOk:
+        // startup?
         if (d->m_qmlEngine->state() == DebuggerNotReady) {
-            if (d->m_isInitialStartup) {
-                d->m_isInitialStartup = false;
-                setupSlaveEngineOnTimer();
-            } else {
-                setupSlaveEngine();
-            }
-        } else {
+            setupSlaveEngine();
+        } else
+        if (d->m_cppEngine->state() == DebuggerNotReady) {
+            setupEngine();
+        } else
+        if (state() == EngineSetupRequested) {
+            notifyEngineSetupOk();
+        } else
+        // breakpoint?
+        if (state() == InferiorStopOk) {
+            continueInferior();
+        } else
+        if (state() == InferiorStopRequested) {
+            checkErrorState(InferiorStopFailed);
+        } else
+        if (state() == InferiorRunRequested && (!checkErrorState(InferiorRunFailed)) && (!checkErrorState(InferiorUnrunnable))) {
             notifyInferiorRunOk();
         }
         break;
 
-    case InferiorRunFailed:
-        notifyInferiorRunFailed();
+    case InferiorRunRequested:
+        // follow the inferior
+        if (state() == InferiorStopOk && checkErrorState(InferiorRunOk)) {
+            continueInferior();
+        }
         break;
 
     case InferiorStopRequested:
-        if (state() == InferiorRunRequested) {
-            // if stopping on startup, move on to normal state
-            // and go forward. Also, stop connection and continue later if needed.
-            d->m_qmlEngine->pauseConnection();
-            setState(EngineRunRequested, true);
-            notifyEngineRunAndInferiorRunOk();
-            setState(newState);
-        } else {
-            setState(newState);
+        // follow the inferior
+        if (state() == InferiorRunOk && checkErrorState(InferiorRunOk)) {
+            requestInterruptInferior();
         }
         break;
 
     case InferiorStopOk:
-        // debugger can stop while qml connection is not made yet, so we can
-        // end up in an illegal state transition here.
-        if (state() == InferiorStopRequested
-         || state() == InferiorRunFailed)
-        {
-            setState(newState);
-        } else if (state() == InferiorRunOk) {
-            // if we break on CPP side while running & engine is QML, switch.
-            if (d->m_activeEngine == d->m_qmlEngine) {
-                setActiveEngine(CppLanguage);
+        // check breakpoints
+        if (state() == InferiorRunRequested) {
+            checkErrorState(InferiorRunFailed);
+        } else
+        if (checkErrorState(InferiorRunOk)) {
+            if (state() == InferiorRunOk) {
+                requestInterruptInferior();
+            } else
+            if (state() == InferiorStopRequested) {
+                interruptInferior();
             }
-            setState(newState);
-        } else if (state() == InferiorRunRequested) {
-            setState(newState, true);
-        }
-        if (d->m_shutdownDeferred) {
-            d->m_activeEngine = d->m_cppEngine;
-            d->m_shutdownDeferred = false;
-            shutdownInferior();
         }
         break;
 
+    case EngineRunFailed:
+    case EngineSetupFailed:
+    case EngineShutdownFailed:
+    case InferiorSetupFailed:
+    case InferiorRunFailed:
+    case InferiorUnrunnable:
     case InferiorStopFailed:
-        setState(newState);
-        if (d->m_shutdownDeferred) {
-            d->m_activeEngine = d->m_cppEngine;
-            d->m_shutdownDeferred = false;
-            shutdownInferior();
-        }
-        break;
-
-    // here, we shut down the qml engine first.
-    // but due to everything being asynchronous, we cannot guarantee
-    // that it is shut down completely before gdb engine is shut down.
-    case InferiorShutdownRequested:
-        if (d->m_activeEngine == d->m_qmlEngine) {
-            d->m_activeEngine = d->m_cppEngine;
-        }
-
-        d->m_qmlEngine->shutdownInferiorAsSlave();
-        setState(newState);
-        break;
-
-    case InferiorShutdownOk:
-        setState(newState);
-        d->m_qmlEngine->shutdownEngineAsSlave();
-        break;
-
     case InferiorShutdownFailed: