Bug 141637 – ASSERTION FAILED: observer in WebCore::BitmapImage::drawPattern

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Renata Hodovan]

Created attachment 246643 [details]
Test case

Load this with debug WK:

<style>
* {
    -webkit-mask-image: -webkit-canvas(viewport-mask);
    -webkit-mask-source-type: luminance;
}
</style>


Backtrace:

ASSERTION FAILED: observer
../../Source/WebCore/platform/graphics/BitmapImage.cpp(623) : virtual void WebCore::BitmapImage::drawPattern(WebCore::GraphicsContext*, const WebCore::FloatRect&, const WebCore::AffineTransform&, const WebCore::FloatPoint&, WebCore::ColorSpace, WebCore::CompositeOperator, const WebCore::FloatRect&, WebCore::BlendMode)

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fff8affd700 (LWP 27905)]
0x00007fffed73b5ef in WTFCrash () at ../../Source/WTF/wtf/Assertions.cpp:321
321	    *(int *)(uintptr_t)0xbbadbeef = 0;
#0  0x00007fffed73b5ef in WTFCrash () at ../../Source/WTF/wtf/Assertions.cpp:321
#1  0x00007ffff3642031 in WebCore::BitmapImage::drawPattern (this=0x7ffff7f1b9c0, ctxt=0x7ffff7f36c60, tileRect=..., transform=..., phase=..., styleColorSpace=WebCore::ColorSpaceDeviceRGB, op=WebCore::CompositeSourceOver, destRect=..., blendMode=WebCore::BlendModeNormal) at ../../Source/WebCore/platform/graphics/BitmapImage.cpp:623
#2  0x00007ffff36a0604 in WebCore::Image::drawTiled (this=0x7ffff7f1b9c0, ctxt=0x7ffff7f36c60, destRect=..., srcPoint=..., scaledTileSize=..., styleColorSpace=WebCore::ColorSpaceDeviceRGB, op=WebCore::CompositeSourceOver, blendMode=WebCore::BlendModeNormal) at ../../Source/WebCore/platform/graphics/Image.cpp:198
#3  0x00007ffff368df2a in WebCore::GraphicsContext::drawTiledImage (this=0x7ffff7f36c60, image=0x7ffff7f1b9c0, colorSpace=WebCore::ColorSpaceDeviceRGB, destination=..., source=..., tileSize=..., imagePaintingOptions=...) at ../../Source/WebCore/platform/graphics/GraphicsContext.cpp:525
#4  0x00007ffff3842953 in WebCore::RenderBoxModelObject::paintFillLayerExtended (this=0x7ffff7f15cc0, paintInfo=..., color=..., bgLayer=0x7ffff7f2a430, rect=..., bleedAvoidance=WebCore::BackgroundBleedNone, box=0x0, boxSize=..., op=WebCore::CompositeSourceOver, backgroundObject=0x0, baseBgColorUsage=WebCore::BaseBackgroundColorUse) at ../../Source/WebCore/rendering/RenderBoxModelObject.cpp:863
#5  0x00007ffff3823c2e in WebCore::RenderBox::paintFillLayer (this=0x7ffff7f15cc0, paintInfo=..., c=..., fillLayer=0x7ffff7f2a430, rect=..., bleedAvoidance=WebCore::BackgroundBleedNone, op=WebCore::CompositeSourceOver, backgroundObject=0x0, baseBgColorUsage=WebCore::BaseBackgroundColorUse) at ../../Source/WebCore/rendering/RenderBox.cpp:1603
#6  0x00007ffff3823b2e in WebCore::RenderBox::paintFillLayers (this=0x7ffff7f15cc0, paintInfo=..., c=..., fillLayer=0x7ffff7f2a430, rect=..., bleedAvoidance=WebCore::BackgroundBleedNone, op=WebCore::CompositeSourceOver, backgroundObject=0x0) at ../../Source/WebCore/rendering/RenderBox.cpp:1594
#7  0x00007ffff3823615 in WebCore::RenderBox::paintMaskImages (this=0x7ffff7f15cc0, paintInfo=..., paintRect=...) at ../../Source/WebCore/rendering/RenderBox.cpp:1523
#8  0x00007ffff3823327 in WebCore::RenderBox::paintMask (this=0x7ffff7f15cc0, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/RenderBox.cpp:1483
#9  0x00007ffff37c63a3 in WebCore::RenderBlock::paintObject (this=0x7ffff7f15cc0, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/RenderBlock.cpp:1558
#10 0x00007ffff37c5682 in WebCore::RenderBlock::paint (this=0x7ffff7f15cc0, paintInfo=..., paintOffset=...) at ../../Source/WebCore/rendering/RenderBlock.cpp:1421
#11 0x00007ffff38dd18b in WebCore::RenderLayer::paintMaskForFragments (this=0x7ffff7f33360, layerFragments=..., context=0x7ffff7f36c60, localPaintingInfo=..., subtreePaintRootForRenderer=0x0) at ../../Source/WebCore/rendering/RenderLayer.cpp:4732
#12 0x00007ffff38daf9f in WebCore::RenderLayer::paintLayerContents (this=0x7ffff7f33360, context=0x7ffff7f36c60, paintingInfo=..., paintFlags=225) at ../../Source/WebCore/rendering/RenderLayer.cpp:4320
#13 0x00007ffff38d97d6 in WebCore::RenderLayer::paintLayerContentsAndReflection (this=0x7ffff7f33360, context=0x7ffff7f36c60, paintingInfo=..., paintFlags=225) at ../../Source/WebCore/rendering/RenderLayer.cpp:3961
#14 0x00007ffff38d96b5 in WebCore::RenderLayer::paintLayer (this=0x7ffff7f33360, context=0x7ffff7f36c60, paintingInfo=..., paintFlags=225) at ../../Source/WebCore/rendering/RenderLayer.cpp:3943
#15 0x00007ffff38db53f in WebCore::RenderLayer::paintList (this=0x7ffff7f33480, list=0x7ffff7f0f3d0, context=0x7ffff7f36c60, paintingInfo=..., paintFlags=225) at ../../Source/WebCore/rendering/RenderLayer.cpp:4385
#16 0x00007ffff38dadff in WebCore::RenderLayer::paintLayerContents (this=0x7ffff7f33480, context=0x7ffff7f36c60, paintingInfo=..., paintFlags=225) at ../../Source/WebCore/rendering/RenderLayer.cpp:4297
#17 0x00007ffff38d97d6 in WebCore::RenderLayer::paintLayerContentsAndReflection (this=0x7ffff7f33480, context=0x7ffff7f36c60, paintingInfo=..., paintFlags=225) at ../../Source/WebCore/rendering/RenderLayer.cpp:3961
#18 0x00007ffff38d96b5 in WebCore::RenderLayer::paintLayer (this=0x7ffff7f33480, context=0x7ffff7f36c60, paintingInfo=..., paintFlags=225) at ../../Source/WebCore/rendering/RenderLayer.cpp:3943
#19 0x00007ffff38db53f in WebCore::RenderLayer::paintList (this=0x7ffff7f336c0, list=0x7ffff7f0f3b0, context=0x7ffff7f36c60, paintingInfo=..., paintFlags=224) at ../../Source/WebCore/rendering/RenderLayer.cpp:4385
#20 0x00007ffff38dadff in WebCore::RenderLayer::paintLayerContents (this=0x7ffff7f336c0, context=0x7ffff7f36c60, paintingInfo=..., paintFlags=224) at ../../Source/WebCore/rendering/RenderLayer.cpp:4297
#21 0x00007ffff38d97d6 in WebCore::RenderLayer::paintLayerContentsAndReflection (this=0x7ffff7f336c0, context=0x7ffff7f36c60, paintingInfo=..., paintFlags=0) at ../../Source/WebCore/rendering/RenderLayer.cpp:3961
#22 0x00007ffff38d96b5 in WebCore::RenderLayer::paintLayer (this=0x7ffff7f336c0, context=0x7ffff7f36c60, paintingInfo=..., paintFlags=0) at ../../Source/WebCore/rendering/RenderLayer.cpp:3943
#23 0x00007ffff38d872b in WebCore::RenderLayer::paint (this=0x7ffff7f336c0, context=0x7ffff7f36c60, damageRect=..., subpixelAccumulation=..., paintBehavior=0, subtreePaintRoot=0x0, paintFlags=0) at ../../Source/WebCore/rendering/RenderLayer.cpp:3746
#24 0x00007ffff35301ce in WebCore::FrameView::paintContents (this=0x7ffff7ec6b00, context=0x7ffff7f36c60, dirtyRect=...) at ../../Source/WebCore/page/FrameView.cpp:3874
#25 0x00007ffff35fc239 in WebCore::ScrollView::paint (this=0x7ffff7ec6b00, context=0x7ffff7f36c60, rect=...) at ../../Source/WebCore/platform/ScrollView.cpp:1247
#26 0x00007ffff288f9e3 in WebKit::WebPage::drawRect (this=0x7ffff7f2d800, graphicsContext=..., rect=...) at ../../Source/WebKit2/WebProcess/WebPage/WebPage.cpp:1368
#27 0x00007ffff2997d4b in WebKit::DrawingAreaImpl::display (this=0x78f420, updateInfo=...) at ../../Source/WebKit2/WebProcess/WebPage/DrawingAreaImpl.cpp:654
#28 0x00007ffff29975bc in WebKit::DrawingAreaImpl::display (this=0x78f420) at ../../Source/WebKit2/WebProcess/WebPage/DrawingAreaImpl.cpp:570
#29 0x00007ffff299747a in WebKit::DrawingAreaImpl::displayTimerFired (this=0x78f420) at ../../Source/WebKit2/WebProcess/WebPage/DrawingAreaImpl.cpp:549
#30 0x00007ffff2999caf in WTF::RunLoop::Timer<WebKit::DrawingAreaImpl>::fired (this=0x78f620) at ../../Source/WTF/wtf/RunLoop.h:121
#31 0x00007ffff44b36f3 in WTF::RunLoop::TimerBase::__lambda3::operator() (__closure=0x478a20) at ../../Source/WTF/wtf/gtk/RunLoopGtk.cpp:121
#32 0x00007ffff44b3b99 in std::_Function_handler<bool(), WTF::RunLoop::TimerBase::start(double, bool)::__lambda3>::_M_invoke(const std::_Any_data &) (__functor=...) at /usr/include/c++/4.8/functional:2057
#33 0x00007ffff44b2d74 in std::function<bool ()>::operator()() const (this=0x7fffffffd5a8) at /usr/include/c++/4.8/functional:2464
#34 0x00007fffed78e1c8 in WTF::GMainLoopSource::boolCallback (this=0x78f630) at ../../Source/WTF/wtf/gobject/GMainLoopSource.cpp:405
#35 0x00007fffed78e63d in WTF::GMainLoopSource::boolSourceCallback (source=0x78f630, source@entry=<error reading variable: value has been optimized out>) at ../../Source/WTF/wtf/gobject/GMainLoopSource.cpp:462
#36 0x00007fffea556443 in g_timeout_dispatch (source=0x7924f0, callback=<optimized out>, user_data=<optimized out>) at gmain.c:4472
#37 0x00007fffea555a1d in g_main_dispatch (context=0x478b00) at gmain.c:3064
#38 g_main_context_dispatch (context=context@entry=0x478b00) at gmain.c:3663
#39 0x00007fffea555d88 in g_main_context_iterate (context=0x478b00, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at gmain.c:3734
#40 0x00007fffea55604a in g_main_loop_run (loop=0x901d10) at gmain.c:3928
#41 0x00007ffff44b31e6 in WTF::RunLoop::run () at ../../Source/WTF/wtf/gtk/RunLoopGtk.cpp:59
#42 0x00007ffff29a1cfc in WebKit::ChildProcessMain<WebKit::WebProcess, WebKit::WebProcessMain> (argc=2, argv=0x7fffffffd948) at ../../Source/WebKit2/Shared/unix/ChildProcessMain.h:61
#43 0x00007ffff29a1b61 in WebKit::WebProcessMainUnix (argc=2, argv=0x7fffffffd948) at ../../Source/WebKit2/WebProcess/gtk/WebProcessMainGtk.cpp:77
#44 0x00000000004008d1 in main (argc=2, argv=0x7fffffffd948) at ../../Source/WebKit2/WebProcess/EntryPoint/unix/WebProcessMain.cpp:44

[Alexey Proskuryakov]

See also: rdar://problem/19725522

[Brent Fulgham]

This reproduces in r204037.

[Radar WebKit Bug Importer]

<rdar://problem/27709864>

[Said Abou-Hallawa]

If the BitmapImage is initialized from a native image, there is no need for an ImageObserver. The ImageObserver is only needed when the BitmapImage is initialized from a SubresourceLoader via a CachedImage. In this case BitmapImage will receive encoded data which will be decoded by an ImageDecoder. In this case the ImageObserver is the CachedImage itself.  The ImageDecoder needs to notify the ImageObserver (the CachedImage) how much data has been decoded to track how much memory the image has allocated.

So this assertion is not needed especially in this place, the drawing. It does not matter how the BitmapImage was created, either from a native image or from encoded data. When we draw it we do not need to check for the ImageObserver.

[Brent Fulgham]

(In reply to comment #4)
> So this assertion is not needed especially in this place, the drawing. It
> does not matter how the BitmapImage was created, either from a native image
> or from encoded data. When we draw it we do not need to check for the
> ImageObserver.

Would it make sense to assert something like the following?

ASSERT(observer || m_source.initialized());

I think that would assert that we either have an image observer, or were created by a NativeImage?

[Brent Fulgham]

(In reply to comment #5)
> (In reply to comment #4)
> > So this assertion is not needed especially in this place, the drawing. It
> > does not matter how the BitmapImage was created, either from a native image
> > or from encoded data. When we draw it we do not need to check for the
> > ImageObserver.
> 
> Would it make sense to assert something like the following?
> 
> ASSERT(observer || m_source.initialized());

Oh, I see. We were only getting it so we could temporarily set it to nullptr, then put it back. We don't actually care if it exists or not.

I agree: We should get rid of this assert.

[Brent Fulgham]

Created attachment 285447 [details]
Patch

[Said Abou-Hallawa]

Unofficial r=me.

[Tim Horton]

Comment on attachment 285447 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=285447&action=review

> Source/WebCore/platform/graphics/BitmapImage.cpp:-614
> -        ASSERT(observer);

Possible this code should be using TemporaryChange?

[Brent Fulgham]

Comment on attachment 285447 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=285447&action=review

>> Source/WebCore/platform/graphics/BitmapImage.cpp:-614
>> -        ASSERT(observer);
> 
> Possible this code should be using TemporaryChange?

That would be cool! But I don't think TemporaryChange is able to call C++ methods to set values the way we can with properties in Obj C++.

[Brent Fulgham]

Committed r204198: <http://trac.webkit.org/changeset/204198>