forked from qt-creator/qt-creator
Fix crash when examples are in multiple categories
When items are added into multiple categories, we got into a double delete when cleaning up, e.g. when changing the shown examples to a different Qt version. Sort copies into categories and delete the originals as a quickfix. Cleaner would be if we didn't allocate them on the heap in the first place. Fixes: QTCREATORBUG-29197 Change-Id: I6490111aba87335b0f7189c4957ed1da8681806f Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org> Reviewed-by: Christian Stenger <christian.stenger@qt.io>
This commit is contained in:
@@ -319,16 +319,22 @@ QList<std::pair<QString, QList<ExampleItem *>>> getCategories(const QList<Exampl
|
||||
QList<ExampleItem *> other;
|
||||
QMap<QString, QList<ExampleItem *>> categoryMap;
|
||||
if (useCategories) {
|
||||
// Append copies of the items and delete the original ones,
|
||||
// because items might be added to multiple categories and that needs individual items
|
||||
for (ExampleItem *item : items) {
|
||||
const QStringList itemCategories = item->metaData.value("category");
|
||||
const QStringList itemCategories = Utils::filteredUnique(
|
||||
item->metaData.value("category"));
|
||||
for (const QString &category : itemCategories)
|
||||
categoryMap[category].append(item);
|
||||
categoryMap[category].append(new ExampleItem(*item));
|
||||
if (itemCategories.isEmpty())
|
||||
other.append(item);
|
||||
other.append(new ExampleItem(*item));
|
||||
}
|
||||
}
|
||||
QList<std::pair<QString, QList<ExampleItem *>>> categories;
|
||||
if (categoryMap.isEmpty()) {
|
||||
// If we tried sorting into categories, but none were defined, we copied the items
|
||||
// into "other", which we don't use here. Get rid of them again.
|
||||
qDeleteAll(other);
|
||||
// The example set doesn't define categories. Consider the "highlighted" ones as "featured"
|
||||
QList<ExampleItem *> featured;
|
||||
QList<ExampleItem *> allOther;
|
||||
@@ -340,6 +346,8 @@ QList<std::pair<QString, QList<ExampleItem *>>> getCategories(const QList<Exampl
|
||||
if (!allOther.isEmpty())
|
||||
categories.append({otherDisplayName, allOther});
|
||||
} else {
|
||||
// All original items have been copied into a category or other, delete.
|
||||
qDeleteAll(items);
|
||||
const auto end = categoryMap.constKeyValueEnd();
|
||||
for (auto it = categoryMap.constKeyValueBegin(); it != end; ++it)
|
||||
categories.append(*it);
|
||||
|
||||
@@ -88,6 +88,7 @@ void tst_Examples::parsing_data()
|
||||
QTest::addColumn<QString>("videoLength");
|
||||
QTest::addColumn<QStringList>("platforms");
|
||||
QTest::addColumn<MetaData>("metaData");
|
||||
QTest::addColumn<QStringList>("categories");
|
||||
|
||||
QTest::addRow("example")
|
||||
<< QByteArray(R"raw(
|
||||
@@ -102,6 +103,8 @@ void tst_Examples::parsing_data()
|
||||
<fileToOpen mainFile="true">widgets/widgets/analogclock/analogclock.cpp</fileToOpen>
|
||||
<meta>
|
||||
<entry name="category">Graphics</entry>
|
||||
<entry name="category">Graphics</entry>
|
||||
<entry name="category">Foobar</entry>
|
||||
<entry name="tags">widgets</entry>
|
||||
</meta>
|
||||
</example>
|
||||
@@ -120,13 +123,29 @@ void tst_Examples::parsing_data()
|
||||
"manifest/widgets/widgets/analogclock/analogclock.cpp")}
|
||||
<< FilePath::fromUserInput("manifest/widgets/widgets/analogclock/analogclock.cpp")
|
||||
<< FilePaths() << Example << true << false << false << ""
|
||||
<< "" << QStringList() << MetaData({{"category", {"Graphics"}}, {"tags", {"widgets"}}});
|
||||
<< "" << QStringList()
|
||||
<< MetaData({{"category", {"Graphics", "Graphics", "Foobar"}}, {"tags", {"widgets"}}})
|
||||
<< QStringList{"Foobar", "Graphics"};
|
||||
|
||||
QTest::addRow("no category, highlighted")
|
||||
<< QByteArray(R"raw(
|
||||
<examples>
|
||||
<example name="No Category, highlighted"
|
||||
isHighlighted="true">
|
||||
</example>
|
||||
</examples>
|
||||
)raw") << /*isExamples=*/true
|
||||
<< "No Category, highlighted" << QString() << QString() << QStringList()
|
||||
<< FilePath("manifest") << QString() << FilePaths() << FilePath() << FilePaths() << Example
|
||||
<< /*hasSourceCode=*/false << false << /*isHighlighted=*/true << ""
|
||||
<< "" << QStringList() << MetaData() << QStringList{"Featured"};
|
||||
}
|
||||
|
||||
void tst_Examples::parsing()
|
||||
{
|
||||
QFETCH(QByteArray, data);
|
||||
QFETCH(bool, isExamples);
|
||||
QFETCH(QStringList, categories);
|
||||
const ExampleItem expected = fetchItem();
|
||||
const expected_str<QList<ExampleItem *>> result
|
||||
= parseExamples(data,
|
||||
@@ -154,7 +173,17 @@ void tst_Examples::parsing()
|
||||
QCOMPARE(item.videoLength, expected.videoLength);
|
||||
QCOMPARE(item.platforms, expected.platforms);
|
||||
QCOMPARE(item.metaData, expected.metaData);
|
||||
qDeleteAll(*result);
|
||||
|
||||
const QList<std::pair<QString, QList<ExampleItem *>>> resultCategories = getCategories(*result,
|
||||
true);
|
||||
QCOMPARE(resultCategories.size(), categories.size());
|
||||
for (int i = 0; i < resultCategories.size(); ++i) {
|
||||
QCOMPARE(resultCategories.at(i).first, categories.at(i));
|
||||
QCOMPARE(resultCategories.at(i).second.size(), 1);
|
||||
}
|
||||
|
||||
for (const auto &category : resultCategories)
|
||||
qDeleteAll(category.second);
|
||||
}
|
||||
|
||||
QTEST_APPLESS_MAIN(tst_Examples)
|
||||
|
||||
Reference in New Issue
Block a user