diff --git a/src/libs/tracing/timelinenotesmodel.cpp b/src/libs/tracing/timelinenotesmodel.cpp index 13adaf72795..c73e55f07d2 100644 --- a/src/libs/tracing/timelinenotesmodel.cpp +++ b/src/libs/tracing/timelinenotesmodel.cpp @@ -76,7 +76,7 @@ int TimelineNotesModel::typeId(int index) const Q_D(const TimelineNotesModel); const TimelineNotesModelPrivate::Note ¬e = d->data[index]; const TimelineModel *model = timelineModelByModelId(note.timelineModel); - if (!model) + if (!model || note.timelineIndex >= model->count()) return -1; // This can happen if one of the timeline models has been removed return model->typeId(note.timelineIndex); } diff --git a/src/plugins/qmlprofiler/qmlprofilernotesmodel.cpp b/src/plugins/qmlprofiler/qmlprofilernotesmodel.cpp index 61c4540719e..337df16c019 100644 --- a/src/plugins/qmlprofiler/qmlprofilernotesmodel.cpp +++ b/src/plugins/qmlprofiler/qmlprofilernotesmodel.cpp @@ -110,14 +110,16 @@ void QmlProfilerNotesModel::stash() continue; int index = timelineIndex(i); - QmlNote save = { - model->typeId(index), - model->collapsedRow(index), - model->startTime(index), - model->duration(index), - text(i) - }; - m_notes.append(save); + if (index < model->count()) { + QmlNote save = { + model->typeId(index), + model->collapsedRow(index), + model->startTime(index), + model->duration(index), + text(i) + }; + m_notes.append(save); + } } resetModified(); } diff --git a/src/plugins/qmlprofiler/tests/flamegraphmodel_test.cpp b/src/plugins/qmlprofiler/tests/flamegraphmodel_test.cpp index f393e3ea0a5..4f5139ceece 100644 --- a/src/plugins/qmlprofiler/tests/flamegraphmodel_test.cpp +++ b/src/plugins/qmlprofiler/tests/flamegraphmodel_test.cpp @@ -71,15 +71,18 @@ int FlameGraphModelTest::generateData(QmlProfilerModelManager *manager, } event.setRangeStage(RangeEnd); + event.setTypeIndex(typeIndices.pop()); event.setTimestamp(++i); manager->addEvent(event); event.setRangeStage(RangeStart); + event.setTypeIndex(0); // Have it accepted by the JavaScript range model above + typeIndices.push(0); event.setTimestamp(++i); manager->addEvent(event); - for (int j = 0; !typeIndices.isEmpty(); ++j) { - event.setTimestamp(i + j); + while (!typeIndices.isEmpty()) { + event.setTimestamp(++i); event.setRangeStage(RangeEnd); event.setTypeIndex(typeIndices.pop()); manager->addEvent(event); @@ -88,7 +91,11 @@ int FlameGraphModelTest::generateData(QmlProfilerModelManager *manager, manager->finalize(); static_cast(manager->notesModel()) - ->setNotes(QVector({QmlNote(0, 2, 1, 20, "dings")})); + ->setNotes(QVector({ + // row 2 on purpose to test the range heuristic + QmlNote(0, 2, 1, 21, "dings"), + QmlNote(0, 3, 12, 1, "weg") + })); manager->notesModel()->restore(); return rangeModelId; @@ -138,8 +145,8 @@ void FlameGraphModelTest::testData() FlameGraphModel::tr("JavaScript")); QCOMPARE(model.data(index2, FlameGraphModel::TypeRole).toString(), FlameGraphModel::tr("Compile")); - QCOMPARE(model.data(index, FlameGraphModel::DurationRole).toLongLong(), 20); - QCOMPARE(model.data(index2, FlameGraphModel::DurationRole).toLongLong(), 12); + QCOMPARE(model.data(index, FlameGraphModel::DurationRole).toLongLong(), 21); + QCOMPARE(model.data(index2, FlameGraphModel::DurationRole).toLongLong(), 13); QCOMPARE(model.data(index, FlameGraphModel::CallCountRole).toInt(), 1); QCOMPARE(model.data(index2, FlameGraphModel::CallCountRole).toInt(), 1); QCOMPARE(model.data(index, FlameGraphModel::DetailsRole).toString(), @@ -154,10 +161,14 @@ void FlameGraphModelTest::testData() QCOMPARE(model.data(index2, FlameGraphModel::LineRole).toInt(), 4); QCOMPARE(model.data(index, FlameGraphModel::ColumnRole).toInt(), 20); QCOMPARE(model.data(index2, FlameGraphModel::ColumnRole).toInt(), 16); - QCOMPARE(model.data(index, FlameGraphModel::NoteRole).toString(), QString("dings")); + + // The flame graph model does not know that one of the notes belongs to row 2, and the + // other one to row 3. It will show both of them for all indexes with that type ID. + QCOMPARE(model.data(index, FlameGraphModel::NoteRole).toString(), QString("dings\nweg")); + QCOMPARE(model.data(index2, FlameGraphModel::NoteRole).toString(), QString()); - QCOMPARE(model.data(index, FlameGraphModel::TimePerCallRole).toLongLong(), 20); - QCOMPARE(model.data(index2, FlameGraphModel::TimePerCallRole).toLongLong(), 12); + QCOMPARE(model.data(index, FlameGraphModel::TimePerCallRole).toLongLong(), 21); + QCOMPARE(model.data(index2, FlameGraphModel::TimePerCallRole).toLongLong(), 13); QCOMPARE(model.data(index, FlameGraphModel::RangeTypeRole).toInt(), static_cast(Javascript)); QCOMPARE(model.data(index2, FlameGraphModel::RangeTypeRole).toInt(), @@ -170,12 +181,12 @@ void FlameGraphModelTest::testData() QVERIFY(!model.data(index2, -10).isValid()); QVERIFY(!model.data(QModelIndex(), FlameGraphModel::LineRole).isValid()); - for (int i = 1; i < 8; ++i) { + for (int i = 1; i < 9; ++i) { index = model.index(0, 0, index); QCOMPARE(model.data(index, FlameGraphModel::TypeRole).toString(), typeRoles[i % typeRoles.length()]); QCOMPARE(model.data(index, FlameGraphModel::NoteRole).toString(), - (i % typeRoles.length() == 0) ? QString("dings") : QString()); + (i % typeRoles.length() == 0) ? QString("dings\nweg") : QString()); } QCOMPARE(model.data(index, FlameGraphModel::CallCountRole).toInt(), 1); @@ -183,7 +194,7 @@ void FlameGraphModelTest::testData() QCOMPARE(model.data(index2, FlameGraphModel::TypeRole).toString(), FlameGraphModel::tr("Compile")); QCOMPARE(model.data(index2, FlameGraphModel::NoteRole).toString(), QString()); - QCOMPARE(model.data(index2, FlameGraphModel::CallCountRole).toInt(), 2); + QCOMPARE(model.data(index2, FlameGraphModel::CallCountRole).toInt(), 1); } void FlameGraphModelTest::testRoleNames() @@ -206,7 +217,10 @@ void FlameGraphModelTest::testNotes() { manager.notesModel()->add(rangeModelId, 1, QString("blubb")); QCOMPARE(model.data(model.index(0, 0), FlameGraphModel::NoteRole).toString(), - QString("dings\nblubb")); + QString("dings\nweg\nblubb")); + manager.notesModel()->remove(0); + QCOMPARE(model.data(model.index(0, 0), FlameGraphModel::NoteRole).toString(), + QString("weg\nblubb")); manager.notesModel()->remove(0); QCOMPARE(model.data(model.index(0, 0), FlameGraphModel::NoteRole).toString(), QString("blubb"));