Differential D27505

Do not crash when icon's width or height is 0
ClosedPublic
Actions

Authored by ahiemstra on Feb 19 2020, 2:46 PM.

Details

Reviewers
davidedmundson
Group Reviewers
Kirigami
Commits
R169:fcae16b30b06: Do not crash when icon's width or height is 0
Summary

When Icon's width or height is 0, I get a crash with the following backtrace:

#0  0x00007ffff7b9cd30 in QSGTexture::setFiltering(QSGTexture::Filtering) () from /usr/lib/libQt5Quick.so.5
#1  0x00007ffff7bd439c in QSGOpaqueTextureMaterialShader::updateState(QSGMaterialShader::RenderState const&, QSGMaterial*, QSGMaterial*) () from /usr/lib/libQt5Quick.so.5
#2  0x00007ffff7bb7857 in QSGBatchRenderer::Renderer::renderMergedBatch(QSGBatchRenderer::Batch const*) () from /usr/lib/libQt5Quick.so.5
#3  0x00007ffff7bbd5b6 in QSGBatchRenderer::Renderer::renderBatches() () from /usr/lib/libQt5Quick.so.5
#4  0x00007ffff7bbdcc5 in QSGBatchRenderer::Renderer::render() () from /usr/lib/libQt5Quick.so.5
#5  0x00007ffff7ba1832 in QSGRenderer::renderScene(QSGBindable const&) () from /usr/lib/libQt5Quick.so.5
#6  0x00007ffff7ba1d14 in QSGRenderer::renderScene(unsigned int) () from /usr/lib/libQt5Quick.so.5
#7  0x00007ffff7c10713 in QSGDefaultRenderContext::renderNextFrame(QSGRenderer*, unsigned int) () from /usr/lib/libQt5Quick.so.5
#8  0x00007ffff7c7b3a1 in QQuickWindowPrivate::renderSceneGraph(QSize const&, QSize const&) () from /usr/lib/libQt5Quick.so.5

This patch prevents Icon from doing anything when width or height is 0, avoiding the crash.

BUG: 417844
FIXED-IN: 5.68

Test Plan

The following QML code, when ran through qmlscene, no longer crashes:

import QtQuick 2.12
import org.kde.kirigami 2.11 as Kirigami
Item { 
    Kirigami.Icon { source: "document-new" }
}

Diff Detail

Repository
R169 Kirigami
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ahiemstra created this revision.Feb 19 2020, 2:46 PM
Restricted Application added a project: Kirigami. · View Herald TranscriptFeb 19 2020, 2:46 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
ahiemstra requested review of this revision.Feb 19 2020, 2:46 PM
davidedmundson accepted this revision.Feb 19 2020, 3:00 PM
davidedmundson added a subscriber: davidedmundson.

Weird. plasma-framework's version of icon (which this is copied from) has had that check for years.

Would be good to dump this test qml somewhere in the repo. A folder full of tests which just have one component in a bunch of configurations.
I've been pushing for that in p-f, and it catches so many things.

This revision is now accepted and ready to land.Feb 19 2020, 3:00 PM
ahiemstra added a comment.Feb 19 2020, 5:06 PM

It most likely does, since Icon also has no implicit width/height it will default to 0 and crash when there's no special layout properties specified.

ahiemstra added a comment.Feb 19 2020, 5:38 PM
In D27505#614240, @davidedmundson wrote:

Would be good to dump this test qml somewhere in the repo. A folder full of tests which just have one component in a bunch of configurations.
I've been pushing for that in p-f, and it catches so many things.

Actually, it should be possible to test this as part of the autotests. However, I just found out that the Kirigami autotests are broken and have not run for a pretty long time. So I will make a separate patch for that.

ngraham edited the summary of this revision. (Show Details)Feb 19 2020, 6:23 PM
Closed by commit R169:fcae16b30b06: Do not crash when icon's width or height is 0 (authored by ahiemstra). · Explain WhyFeb 20 2020, 11:42 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Msrc/icon.cpp (2 lines)

Diff 76043

View Options

src/icon.cpp

Loading...