V4 debugger: Properly count break points

We cannot just take the number of active breakpoints as ID for the next
one. It's possible to remove breakpoints after all.

Fixes: QTBUG-93404
Change-Id: Icde7a8e47c740e930f2313ffd9034b00033a54aa
Reviewed-by: hjk <hjk@qt.io>
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
(cherry picked from commit 7f12cf3346d65d0bff78fff8000ed519fbb921ba)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

4 files changed, 88 insertions, 19 deletions

diff --git a/src/plugins/qmltooling/qmldbg_debugger/qv4debuggeragent.cpp b/src/plugins/qmltooling/qmldbg_debugger/qv4debuggeragent.cpp
index c74fd35ea2..710b2716d3 100644
--- a/src/plugins/qmltooling/qmldbg_debugger/qv4debuggeragent.cpp
+++ b/src/plugins/qmltooling/qmldbg_debugger/qv4debuggeragent.cpp
@@ -163,7 +163,7 @@ int QV4DebuggerAgent::addBreakPoint(const QString &fileName, int lineNumber, boo
         for (QV4Debugger *debugger : qAsConst(m_debuggers))
             debugger->addBreakPoint(fileName, lineNumber, condition);
 
-    int id = m_breakPoints.size();
+    const int id = ++m_lastBreakPointId;
     m_breakPoints.insert(id, BreakPoint(fileName, lineNumber, enabled, condition));
     return id;
 }
diff --git a/src/plugins/qmltooling/qmldbg_debugger/qv4debuggeragent.h b/src/plugins/qmltooling/qmldbg_debugger/qv4debuggeragent.h
index ccaf76b269..58222deb58 100644
--- a/src/plugins/qmltooling/qmldbg_debugger/qv4debuggeragent.h
+++ b/src/plugins/qmltooling/qmldbg_debugger/qv4debuggeragent.h
@@ -93,6 +93,7 @@ private:
     };
 
     QHash<int, BreakPoint> m_breakPoints;
+    int m_lastBreakPointId = 0;
     bool m_breakOnThrow;
     QV4DebugServiceImpl *m_debugService;
 };
diff --git a/tests/auto/qml/debugger/qqmldebugjs/data/breakPointIds.qml b/tests/auto/qml/debugger/qqmldebugjs/data/breakPointIds.qml
new file mode 100644
index 0000000000..c3e7687831
--- /dev/null
+++ b/tests/auto/qml/debugger/qqmldebugjs/data/breakPointIds.qml
@@ -0,0 +1,15 @@
+import QtQml 2.15
+Timer {
+    Component.onCompleted: {
+        console.log('0')
+        console.log('1')
+        console.log('2')
+        console.log('3')
+        console.log('4')
+        console.log('5')
+        running = true
+    }
+
+    interval: 0
+    onTriggered: Qt.quit()
+}
diff --git a/tests/auto/qml/debugger/qqmldebugjs/tst_qqmldebugjs.cpp b/tests/auto/qml/debugger/qqmldebugjs/tst_qqmldebugjs.cpp
index 91470e0651..59cab6eb75 100644
--- a/tests/auto/qml/debugger/qqmldebugjs/tst_qqmldebugjs.cpp
+++ b/tests/auto/qml/debugger/qqmldebugjs/tst_qqmldebugjs.cpp
@@ -65,6 +65,7 @@ const char *STEPACTION_QMLFILE = "stepAction.qml";
 const char *BREAKPOINTRELOCATION_QMLFILE = "breakpointRelocation.qml";
 const char *ENCODEQMLSCOPE_QMLFILE = "encodeQmlScope.qml";
 const char *BREAKONANCHOR_QMLFILE = "breakOnAnchor.qml";
+const char *BREAKPOINTIDS_QMLFILE = "breakPointIds.qml";
 
 #undef QVERIFY
 #define QVERIFY(statement) \
@@ -156,6 +157,8 @@ private slots:
     void encodeQmlScope();
     void breakOnAnchor();
 
+    void breakPointIds();
+
 private:
     ConnectResult init(bool qmlscene, const QString &qmlFile = QString(TEST_QMLFILE),
                     bool blockMode = true, bool restrictServices = false);
@@ -165,10 +168,11 @@ private:
     void targetData();
     bool waitForClientSignal(const char *signal, int timeout = 30000);
     void checkVersionParameters();
+    int setBreakPoint(const QString &file, int sourceLine, bool enabled);
+    void clearBreakPoint(int id);
 };
 
 
-
 void tst_QQmlDebugJS::initTestCase()
 {
     QQmlDebugTest::initTestCase();
@@ -566,7 +570,8 @@ void tst_QQmlDebugJS::changeBreakpoint()
 
     int sourceLine2 = 37;
     int sourceLine1 = 38;
-    QCOMPARE(init(qmlscene, CHANGEBREAKPOINT_QMLFILE), ConnectSuccess);
+    const QString file = QLatin1String(CHANGEBREAKPOINT_QMLFILE);
+    QCOMPARE(init(qmlscene, file), ConnectSuccess);
 
     bool isStopped = false;
     QObject::connect(m_client.data(), &QV4DebugClient::stopped, this, [&]() { isStopped = true; });
@@ -589,27 +594,13 @@ void tst_QQmlDebugJS::changeBreakpoint()
         return breakpointsHit[0].toInt();
     };
 
-    auto setBreakPoint = [&](int sourceLine, bool enabled) {
-        int id = -1;
-        auto connection = QObject::connect(m_client.data(), &QV4DebugClient::result, [&]() {
-            id = extractBody().value("breakpoint").toInt();
-        });
-
-        m_client->setBreakpoint(QLatin1String(CHANGEBREAKPOINT_QMLFILE), sourceLine, -1, enabled);
-        bool success = QTest::qWaitFor([&]() { return id >= 0; });
-        Q_UNUSED(success);
-
-        QObject::disconnect(connection);
-        return id;
-    };
-
     //The breakpoints are in a timer loop so we can set them after connect().
     //Furthermore the breakpoints should be hit in the right order because setting of breakpoints
     //can only occur in the QML event loop. (see QCOMPARE for sourceLine2 below)
-    const int breakpoint1 = setBreakPoint(sourceLine1, false);
+    const int breakpoint1 = setBreakPoint(file, sourceLine1, false);
     QVERIFY(breakpoint1 >= 0);
 
-    const int breakpoint2 = setBreakPoint(sourceLine2, true);
+    const int breakpoint2 = setBreakPoint(file, sourceLine2, true);
     QVERIFY(breakpoint2 >= 0);
 
     auto verifyBreakpoint = [&](int sourceLine, int breakpointId) {
@@ -1026,6 +1017,39 @@ void tst_QQmlDebugJS::breakOnAnchor()
     QCOMPARE(breaks, 2);
 }
 
+void tst_QQmlDebugJS::breakPointIds()
+{
+    QString file(BREAKPOINTIDS_QMLFILE);
+    QCOMPARE(init(true, file), ConnectSuccess);
+
+    int breaks = 0;
+    int breakPointIds[] = { -1, -1, -1, -1, -1, -1};
+
+    QObject::connect(m_client.data(), &QV4DebugClient::stopped, this, [&]() {
+        const QJsonObject body = m_client->response().body.toObject();
+        QCOMPARE(body.value("sourceLine").toInt(), breaks + 4);
+        const QJsonArray breakpointsHit = body.value("breakpoints").toArray();
+        QVERIFY(breakpointsHit.size() > 0);
+        QCOMPARE(breakpointsHit[0].toInt(), breakPointIds[breaks]);
+        ++breaks;
+        m_client->continueDebugging(QV4DebugClient::Continue);
+    });
+
+    for (int i = 0; i < 6; ++i)
+        breakPointIds[i] = setBreakPoint(file, i + 4, true);
+
+    clearBreakPoint(breakPointIds[2]);
+    breakPointIds[2] = setBreakPoint(file, 6, true);
+
+    QTRY_COMPARE(m_process->state(), QProcess::Running);
+    m_client->connect();
+
+    QTRY_COMPARE(m_process->state(), QProcess::NotRunning);
+    QCOMPARE(m_process->exitStatus(), QProcess::NormalExit);
+
+    QCOMPARE(breaks, 6);
+}
+
 QList<QQmlDebugClient *> tst_QQmlDebugJS::createClients()
 {
     m_client = new QV4DebugClient(m_connection);
@@ -1054,6 +1078,35 @@ void tst_QQmlDebugJS::checkVersionParameters()
     QCOMPARE(body.value("ChangeBreakpoint").toBool(), true);
 }
 
+int tst_QQmlDebugJS::setBreakPoint(const QString &file, int sourceLine, bool enabled)
+{
+    int id = -1;
+    auto connection = QObject::connect(m_client.data(), &QV4DebugClient::result, [&]() {
+        id = m_client->response().body.toObject().value("breakpoint").toInt();
+    });
+
+    m_client->setBreakpoint(file, sourceLine, -1, enabled);
+    bool success = QTest::qWaitFor([&]() { return id >= 0; });
+    Q_UNUSED(success);
+
+    QObject::disconnect(connection);
+    return id;
+}
+
+void tst_QQmlDebugJS::clearBreakPoint(int id)
+{
+    bool ok = false;
+    auto connection = QObject::connect(m_client.data(), &QV4DebugClient::result, [&]() {
+        ok = true;
+    });
+
+    m_client->clearBreakpoint(id);
+    bool success = QTest::qWaitFor([&]() { return ok; });
+    Q_UNUSED(success);
+
+    QObject::disconnect(connection);
+}
+
 QTEST_MAIN(tst_QQmlDebugJS)
 
 #include "tst_qqmldebugjs.moc"