Bug 186767

Summary: [GTK] Do not depend on resources provided by the GNOME icon theme
Product: WebKit Reporter: Adrian Perez <aperez>
Component: WebKitGTKAssignee: Diego Pino <dpino>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, bugs-noreply, calvaris, clopez, dpino, ews-watchlist, gyuyoung.kim, mcatanzaro, ryuan.choi, sergio
Priority: P2    
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch ews-feeder: commit-queue-

Description Adrian Perez 2018-06-18 08:33:45 PDT
As per the discussion in bug #186638 (comments 15 to 17), we should NOT
depend on gnome-icon-theme being installed, and therefore the resources
built as part of WebKitGTK+ should include a copy of the needed icons.
Comment 1 Michael Catanzaro 2018-06-18 11:17:02 PDT
This entails fixing the following crash:

Thread 1 (Thread 0x7fb014ea73c0 (LWP 575)):
#0  _g_log_abort () at /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.54.2/glib/gmessages.c:554
#1  0x00007fb01cc9d415 in g_logv () at /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.54.2/glib/gmessages.c:1362
#2  0x00007fb01cc9d562 in g_log () at /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.54.2/glib/gmessages.c:1403
#3  0x00007fb01cc725de in g_bytes_get_size () at /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.54.2/glib/gbytes.c:267
#4  0x00007fb025074516 in WebCore::loadImageFromGResource(char const*) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#5  0x00007fb0250747eb in WebCore::Image::loadPlatformResource(char const*) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#6  0x00007fb0247688d4 in WebCore::CachedImage::brokenImage(float) const () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#7  0x00007fb024b14396 in WebCore::RenderImage::paintReplaced(WebCore::PaintInfo&, WebCore::LayoutPoint const&) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#8  0x00007fb024ba3d1c in WebCore::RenderReplaced::paint(WebCore::PaintInfo&, WebCore::LayoutPoint const&) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#9  0x00007fb024b13612 in WebCore::RenderImage::paint(WebCore::PaintInfo&, WebCore::LayoutPoint const&) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#10 0x00007fb024ae3a14 in WebCore::RenderElement::paintAsInlineBlock(WebCore::PaintInfo&, WebCore::LayoutPoint const&) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#11 0x00007fb024a4bb02 in WebCore::InlineElementBox::paint(WebCore::PaintInfo&, WebCore::LayoutPoint const&, WebCore::LayoutUnit, WebCore::LayoutUnit) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#12 0x00007fb024a5a21a in WebCore::InlineFlowBox::paint(WebCore::PaintInfo&, WebCore::LayoutPoint const&, WebCore::LayoutUnit, WebCore::LayoutUnit) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#13 0x00007fb024c057fc in WebCore::RootInlineBox::paint(WebCore::PaintInfo&, WebCore::LayoutPoint const&, WebCore::LayoutUnit, WebCore::LayoutUnit) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#14 0x00007fb024b74d9f in WebCore::RenderLineBoxList::paint(WebCore::RenderBoxModelObject*, WebCore::PaintInfo&, WebCore::LayoutPoint const&) const () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#15 0x00007fb024a6bc09 in WebCore::RenderBlock::paintContents(WebCore::PaintInfo&, WebCore::LayoutPoint const&) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#16 0x00007fb024a89043 in WebCore::RenderBlock::paintObject(WebCore::PaintInfo&, WebCore::LayoutPoint const&) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#17 0x00007fb024a6772b in WebCore::RenderBlock::paint(WebCore::PaintInfo&, WebCore::LayoutPoint const&) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#18 0x00007fb024a6be87 in WebCore::RenderBlock::paintChild(WebCore::RenderBox&, WebCore::PaintInfo&, WebCore::LayoutPoint const&, WebCore::PaintInfo&, bool, WebCore::RenderBlock::PaintBlockType) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#19 0x00007fb024a6c09d in WebCore::RenderBlock::paintChildren(WebCore::PaintInfo&, WebCore::LayoutPoint const&, WebCore::PaintInfo&, bool) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#20 0x00007fb024a6bbf8 in WebCore::RenderBlock::paintContents(WebCore::PaintInfo&, WebCore::LayoutPoint const&) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#21 0x00007fb024a89043 in WebCore::RenderBlock::paintObject(WebCore::PaintInfo&, WebCore::LayoutPoint const&) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#22 0x00007fb024a6772b in WebCore::RenderBlock::paint(WebCore::PaintInfo&, WebCore::LayoutPoint const&) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#23 0x00007fb024a6be87 in WebCore::RenderBlock::paintChild(WebCore::RenderBox&, WebCore::PaintInfo&, WebCore::LayoutPoint const&, WebCore::PaintInfo&, bool, WebCore::RenderBlock::PaintBlockType) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#24 0x00007fb024a6c09d in WebCore::RenderBlock::paintChildren(WebCore::PaintInfo&, WebCore::LayoutPoint const&, WebCore::PaintInfo&, bool) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#25 0x00007fb024a6bbf8 in WebCore::RenderBlock::paintContents(WebCore::PaintInfo&, WebCore::LayoutPoint const&) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#26 0x00007fb024a89043 in WebCore::RenderBlock::paintObject(WebCore::PaintInfo&, WebCore::LayoutPoint const&) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#27 0x00007fb024a6772b in WebCore::RenderBlock::paint(WebCore::PaintInfo&, WebCore::LayoutPoint const&) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#28 0x00007fb024b33279 in WebCore::RenderLayer::paintForegroundForFragmentsWithPhase(WebCore::PaintPhase, WTF::Vector<WebCore::LayerFragment, 1ul, WTF::CrashOnOverflow, 16ul> const&, WebCore::GraphicsContext&, WebCore::RenderLayer::LayerPaintingInfo const&, unsigned int, WebCore::RenderObject*) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#29 0x00007fb024b4176a in WebCore::RenderLayer::paintForegroundForFragments(WTF::Vector<WebCore::LayerFragment, 1ul, WTF::CrashOnOverflow, 16ul> const&, WebCore::GraphicsContext&, WebCore::GraphicsContext&, WebCore::LayoutRect const&, bool, WebCore::RenderLayer::LayerPaintingInfo const&, unsigned int, WebCore::RenderObject*) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#30 0x00007fb024b5a394 in WebCore::RenderLayer::paintLayerContents(WebCore::GraphicsContext&, WebCore::RenderLayer::LayerPaintingInfo const&, unsigned int) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#31 0x00007fb024b5b230 in WebCore::RenderLayer::paintLayer(WebCore::GraphicsContext&, WebCore::RenderLayer::LayerPaintingInfo const&, unsigned int) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#32 0x00007fb024b5b555 in WebCore::RenderLayer::paintList(WTF::Vector<WebCore::RenderLayer*, 0ul, WTF::CrashOnOverflow, 16ul>*, WebCore::GraphicsContext&, WebCore::RenderLayer::LayerPaintingInfo const&, unsigned int) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#33 0x00007fb024b5a02e in WebCore::RenderLayer::paintLayerContents(WebCore::GraphicsContext&, WebCore::RenderLayer::LayerPaintingInfo const&, unsigned int) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#34 0x00007fb024b5b230 in WebCore::RenderLayer::paintLayer(WebCore::GraphicsContext&, WebCore::RenderLayer::LayerPaintingInfo const&, unsigned int) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#35 0x00007fb024b5b39d in WebCore::RenderLayer::paint(WebCore::GraphicsContext&, WebCore::LayoutRect const&, WebCore::LayoutSize const&, unsigned int, WebCore::RenderObject*, unsigned int, WebCore::RenderLayer::SecurityOriginPaintPolicy) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#36 0x00007fb0247e498e in WebCore::FrameView::paintContents(WebCore::GraphicsContext&, WebCore::IntRect const&, WebCore::Widget::SecurityOriginPaintPolicy) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#37 0x00007fb02489c0da in WebCore::ScrollView::paint(WebCore::GraphicsContext&, WebCore::IntRect const&, WebCore::Widget::SecurityOriginPaintPolicy) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#38 0x00007fb0239008fd in WebKit::WebPage::drawRect(WebCore::GraphicsContext&, WebCore::IntRect const&) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#39 0x00007fb023b44f31 in WebKit::DrawingAreaImpl::display(WebKit::UpdateInfo&) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#40 0x00007fb023b4572b in WebKit::DrawingAreaImpl::sendDidUpdateBackingStoreState() () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#41 0x00007fb023b43248 in WebKit::AcceleratedDrawingArea::updateBackingStoreState(unsigned long, bool, float, WebCore::IntSize const&, WebCore::IntSize const&) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#42 0x00007fb023b466eb in WebKit::DrawingAreaImpl::updateBackingStoreState(unsigned long, bool, float, WebCore::IntSize const&, WebCore::IntSize const&) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#43 0x00007fb0239bd29b in void IPC::handleMessage<Messages::DrawingArea::UpdateBackingStoreState, WebKit::DrawingArea, void (WebKit::DrawingArea::*)(unsigned long, bool, float, WebCore::IntSize const&, WebCore::IntSize const&)>(IPC::Decoder&, WebKit::DrawingArea*, void (WebKit::DrawingArea::*)(unsigned long, bool, float, WebCore::IntSize const&, WebCore::IntSize const&)) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#44 0x00007fb0239bd182 in WebKit::DrawingArea::didReceiveMessage(IPC::Connection&, IPC::Decoder&) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#45 0x00007fb0236794f9 in IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::Decoder&) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#46 0x00007fb023829846 in WebKit::WebProcess::didReceiveMessage(IPC::Connection&, IPC::Decoder&) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#47 0x00007fb02367525b in IPC::Connection::dispatchMessage(std::unique_ptr<IPC::Decoder, std::default_delete<IPC::Decoder> >) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#48 0x00007fb023675e3c in IPC::Connection::dispatchOneMessage() () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#49 0x00007fb021663c2d in WTF::RunLoop::performWork() () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#50 0x00007fb021699e19 in WTF::RunLoop::RunLoop()::{lambda(void*)#1}::_FUN(void*) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#51 0x00007fb01cc9681a in g_main_dispatch () at /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.54.2/glib/gmain.c:3148
#52 g_main_context_dispatch () at /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.54.2/glib/gmain.c:3813
#53 0x00007fb01cc96ba8 in g_main_context_iterate () at /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.54.2/glib/gmain.c:3886
#54 0x00007fb01cc96ec2 in g_main_loop_run () at /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/DependenciesGTK/Source/glib-2.54.2/glib/gmain.c:4082
#55 0x00007fb02169a810 in WTF::RunLoop::run() () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#56 0x00007fb023b51a62 in int WebKit::ChildProcessMain<WebKit::WebProcess, WebKit::WebProcessMain>(int, char**) () from /home/ews/ltilve-gtk-wk2-ews/WebKit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#57 0x00007fb01acd62e1 in __libc_start_main (main=0x555d01913ca0 <main>, argc=3, argv=0x7ffe827f5748, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7ffe827f5738) at ../csu/libc-start.c:291
#58 0x0000555d01913d2a in _start ()

STDERR: 
STDERR: warning: core file may not match specified executable file.
STDERR: g_bytes_get_size: assertion 'bytes != NULL' failed

We currently have the following code:

static Ref<Image> loadMissingImageIconFromTheme(const char* name)
{
    // ...
    if (iconInfo) {
        // Is the icon installed?
        return WTFMove(icon);
    }

    return loadImageFromGResource(name);
}

That's currently incorrect. But we won't need any code changes if fixing this bug. (It is surely preferable to lookup the icon at runtime, and only fall back to an embedded resource if missing.)

Note I've added a test expectation against this bug, for fast/hidpi/broken-image-icon-very-hidpi.html.
Comment 2 Xabier Rodríguez Calvar 2018-06-19 00:53:10 PDT
Please, do not forget the media control icons if you bundle them
Comment 3 Diego Pino 2021-06-08 14:53:21 PDT
Created attachment 430895 [details]
Patch
Comment 4 Diego Pino 2021-06-08 15:01:19 PDT
The only test filed under this bug at this moment is:

  fast/hidpi/broken-image-icon-very-hidpi.html

The test has been consistently crashing for the last 4000 revisions with the following stacktrace (debug crash-log):

Thread 1 (Thread 0x7f0d1bed4ec0 (LWP 13121)):
#0  WTFCrash() () at ../../Source/WTF/wtf/Assertions.cpp:321
#1  0x00007f0d260f3cb7 in CRASH_WITH_INFO(...) () at WTF/Headers/wtf/Assertions.h:744
#2  0x00007f0d2a10d1fe in WebCore::loadImageFromGResource(char const*) (iconName=0x7f0d2e62a258 "missingImage@3x") at ../../Source/WebCore/platform/graphics/gtk/ImageGtk.cpp:43
#3  0x00007f0d2a10d328 in WebCore::Image::loadPlatformResource(char const*) (name=0x7f0d2e62a258 "missingImage@3x") at ../../Source/WebCore/platform/graphics/gtk/ImageGtk.cpp:54
#4  0x00007f0d29b7aabf in WebCore::CachedImage::brokenImage(float) const (this=0x7f0d1b55e800, deviceScaleFactor=3) at ../../Source/WebCore/loader/cache/CachedImage.cpp:219
#5  0x00007f0d2a4733c7 in WebCore::RenderImage::paintReplaced(WebCore::PaintInfo&, WebCore::LayoutPoint const&) (this=0x7f0cdb22f0f0, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/RenderImage.cpp:506
...

So the issue seems to be that resource "missingImage@3x.png" is missing.
Comment 5 Diego Pino 2021-06-08 15:06:47 PDT
Created attachment 430900 [details]
Patch
Comment 6 Adrian Perez 2021-06-09 01:18:10 PDT
Comment on attachment 430900 [details]
Patch

The style-checker issue seems a bit like a false positive. It is possible to
workaround it by appending to the list one item at a time, for example:

  set(WebKitResources "")
  list(APPEND WebKitResources "<file alias=\"css/gtk-theme.css\">gtk-theme.css</file>\n")
  list(APPEND WebKitResources "<file alias=\"images/missingImage\">missingImage.png</file>\n")
  list(APPEND WebKitResources "<file alias=\"images/missingImage@2x\">missingImage@2x.png</file>\n")
  ...

We can either land manually to go around the false positive, or if you feel like
reworking this a bit as suggested to avoid the style checker complaints, then it's
fine to use the commit-queue :)
Comment 7 EWS 2021-06-09 01:38:32 PDT
/Volumes/Data/worker/Commit-Queue/build/Source/WebKit/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).
Comment 8 Diego Pino 2021-06-09 03:17:09 PDT
Created attachment 430951 [details]
Patch
Comment 9 EWS 2021-06-09 06:40:50 PDT
Committed r278658 (238640@main): <https://commits.webkit.org/238640@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 430951 [details].