Bug 2144200

Created attachment 1925754 [details]
Full backtrace from GDB

Here's the full backtrace from GDB

Created attachment 1925755 [details]
plasmashell core dump

systemd core dump for plasmashell

Pull request to fix this: https://src.fedoraproject.org/rpms/qt5-qtwebengine/pull-request/13

Copr build to test fix: https://copr.fedorainfracloud.org/coprs/build/5048782

Have you already tested this? Does it work? I am asking because I doubt it does, as is, because the patch does not modify pdfium at all, whereas the crash was in pdfium::base::SetSystemPagesAccess. My guess would be that the changes in the patch are needed, too, but you are missing some change to pdfium that fixes the crash you ran into.

I have not tested it yet. I'm building in COPR right now so I can test it.

Another question is, why does Plasma Desktop load libQt5Pdf.so.5 to show a wallpaper? Is that using a PDF as the wallpaper? Surely that is not the common case, is it?

But it looks like pdfium::base is actually the same code as chromium::base, just in a different namespace for some reason. The code seems to be from the Chromium code base (base/allocator/partition_allocator/page_allocator.cc), not the Pdfium code base. So the patch could be sufficient after all. So I am eagerly waiting for your test results.

So I did a local build (which built *really quickly*) and I have verified that the Plasma Desktop now works with this fix.

Created attachment 1925970 [details]
Screenshot showing a working KDE Plasma Desktop

With the fixed QtWebEngine, I was able to load Plasma Desktop and take a screenshot.

Cool!

The one question that is still open though is the one I asked in comment 6: Why does Plasma Desktop load libQt5Pdf.so.5 to show a wallpaper? Are you using a PDF as the wallpaper? Or why does it think the wallpaper is a PDF?

(In reply to Kevin Kofler from comment #10)
> Cool!
> 
> The one question that is still open though is the one I asked in comment 6:
> Why does Plasma Desktop load libQt5Pdf.so.5 to show a wallpaper? Are you
> using a PDF as the wallpaper? Or why does it think the wallpaper is a PDF?

I'm not using a PDF as a wallpaper. so I don't know.

Looks like something that should be addressed separately upstream. Though QtWebEngine would have needed to be fixed anyway. So thanks for the fix!

(In reply to Kevin Kofler from comment #12)
> Looks like something that should be addressed separately upstream. Though
> QtWebEngine would have needed to be fixed anyway. So thanks for the fix!

I'm building it now for F37 and Rawhide...

Rawhide: https://koji.fedoraproject.org/koji/buildinfo?buildID=2090854
Fedora 37: https://koji.fedoraproject.org/koji/buildinfo?buildID=2090853

FEDORA-2022-af3ef0a1b6 has been submitted as an update to Fedora 37. https://bodhi.fedoraproject.org/updates/FEDORA-2022-af3ef0a1b6

FEDORA-2022-af3ef0a1b6 has been pushed to the Fedora 37 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf upgrade --enablerepo=updates-testing --refresh --advisory=FEDORA-2022-af3ef0a1b6`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2022-af3ef0a1b6

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

FEDORA-2022-af3ef0a1b6 has been pushed to the Fedora 37 stable repository.
If problem still persists, please make note of it in this bug report.

FYI, upstream Gerrit review for the fix: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/445075

So what I think is happening here is that the code to autodetect the file type of the wallpaper (MediaProxy::determineBackgroundType) works by enumerating all types supported by QMovie (QMovie::supportedFormats) and then checking whether they match. But QMovie decides that it supports all formats supported by QImage that support QImageIOHandler::Animation, and to check that property, it needs to initialize the QImage decoder. As a result, this ends up initializing any and all installed QImage decoder plugins. There is also one for PDF, because somebody thought it would be a great idea to treat a PDF like an image rather than a document. (I think PDF wallpapers were actually one of the intended use cases.) So then we end up calling into QPdfPlugin::create, which initializes pdfium, and if that fails to initialize, the whole Plasma crashes and we lose.

I am not sure where the best place to fix this mess would be, but initializing all image decoding libraries in existence (some of which can have quite poor quality) in a process (all the more in the desktop shell's main process) strikes me as a very bad idea.

I wonder whether using QImageReader::supportedImageFormats() instead of QMovie::supportedFormats() would be sufficient to avoid having to initialize all the plugins or whether that would also initialize them all.

But animated backgrounds and video backgrounds are also supported in Plasma, so switching to QImageReader::supportedImageFormats() would remove that ability, wouldn't it?

Not necessarily, since QImageReader::supportedImageFormats() is (or should be, at least) a superset of QMovie::supportedFormats(). By the way, I wonder through what code path image formats that do not support animations are going, because they are not (or at least should not be) returned by QMovie::supportedFormats(). I guess I need to look at the MediaProxy::determineBackgroundType code.

So this is the code:
https://invent.kde.org/plasma/plasma-workspace/-/blob/42a88bfdd4a5a71cc608230240f1877f7840de44/wallpapers/image/plugin/utils/mediaproxy.cpp#L250

What this actually does is, it checks whether the image format that corresponds to the file extension supports animations. It does so by enumerating all formats that support animations, which is a very inefficient way to do the check. In addition, it also does not check anywhere whether the image is actually animated. It just assumes that, if the image format supports animations, we have a BackgroundType::Type::AnimatedImage.

The better way would be to do what QMovie::supportedFormats() does internally, but only on the type we actually care about, without enumerating them:
    QBuffer dummyBuffer;
    dummyBuffer.open(QIODevice::ReadOnly);

    if (QImageReader(&buffer, QFileInfo(filePath).suffix().toLower().toLatin1()).supportsOption(QImageIOHandler::Animation)) {
        // Derived from the suffix
        m_backgroundType = BackgroundType::Type::AnimatedImage;
…

(That still only checks the file type, not the actual image, but it does so while instantiating only the one image reader that we care about, not every single one.)

Sorry, typo fix (I renamed buffer to dummyBuffer to be clear, but forgot to rename one usage instance of it):
    QBuffer dummyBuffer;
    dummyBuffer.open(QIODevice::ReadOnly);

    if (QImageReader(&dummyBuffer, QFileInfo(filePath).suffix().toLower().toLatin1()).supportsOption(QImageIOHandler::Animation)) {
        // Derived from the suffix
        m_backgroundType = BackgroundType::Type::AnimatedImage;

(Editing messages is a feature sorely missing in Bugzilla.)

I have filed https://bugs.kde.org/show_bug.cgi?id=462308 for that issue (MediaProxy::determineBackgroundType needlessly instantiates all available QImage reader plugins).