[Webkit-unassigned] [Bug 140722] Simplify CachedImage clients to avoid overriding both or either only imageChanged or notifyFinished

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jan 28 08:22:14 PST 2015


https://bugs.webkit.org/show_bug.cgi?id=140722

Julien Isorce <j.isorce at samsung.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #245530|0                           |1
        is obsolete|                            |
 Attachment #245530|review?                     |
              Flags|                            |
 Attachment #245545|                            |review?
              Flags|                            |

--- Comment #10 from Julien Isorce <j.isorce at samsung.com> ---
Created attachment 245545
  --> https://bugs.webkit.org/attachment.cgi?id=245545&action=review
CachedImage: ensure clients overrides imageChanged instead of notifyFinished

I addressed remarks (grammar, missing end of a sentense, file explnations, nullptr)

About the 2 other points concerning removal of RenderImage::notifyFinished:

1* invalidateBackgroundObscurationStatus:
It will not always endup with invalidateBackgroundObscurationStatus as previously.
Instead it will be done from RenderReplaced::layout() when imageDimensionsChanged calls setNeedsLayout().
The condition being: imageSizeChanged || hasOverrideSize || containingBlockNeedsToRecomputePreferredSize || layoutSizeDependsOnIntrinsicSize
(also RenderReplaced::layout() can be triggered this way:
Breakpoint 1, 0x00007fe784e5caf0 in WebCore::RenderReplaced::layout() ()
(gdb) bt
#0  0x00007fe784e5caf0 in WebCore::RenderReplaced::layout()
#1  0x00007fe784de92e5 in WebCore::RenderImage::layout() 
#2  0x00007fe784d8433b in WebCore::RenderBlockFlow::layoutLineBoxes(bool, WebCore::LayoutUnit&, WebCore::LayoutUnit&) 
#3  0x00007fe784d74805 in WebCore::RenderBlockFlow::layoutBlock(bool, WebCore::LayoutUnit)
#4  0x00007fe784d5a056 in WebCore::RenderBlock::layout() 
#5  0x00007fe784d6f479 in WebCore::RenderBlockFlow::layoutBlockChild(WebCore::RenderBox&, WebCore::RenderBlockFlow::MarginInfo&, WebCore::LayoutUnit&, WebCore::LayoutUnit&) 
#6  0x00007fe784d704b8 in WebCore::RenderBlockFlow::layoutBlockChildren(bool, WebCore::LayoutUnit&)
#7  0x00007fe784d74d82 in WebCore::RenderBlockFlow::layoutBlock(bool, WebCore::LayoutUnit)
#8  0x00007fe784d5a056 in WebCore::RenderBlock::layout()
#9  0x00007fe784d6f479 in WebCore::RenderBlockFlow::layoutBlockChild(WebCore::RenderBox&, WebCore::RenderBlockFlow::MarginInfo&, WebCore::LayoutUnit&, WebCore::LayoutUnit&) 
#10 0x00007fe784d704b8 in WebCore::RenderBlockFlow::layoutBlockChildren(bool, WebCore::LayoutUnit&) 
#11 0x00007fe784d74d82 in WebCore::RenderBlockFlow::layoutBlock(bool, WebCore::LayoutUnit)
#12 0x00007fe784d5a056 in WebCore::RenderBlock::layout()
#13 0x00007fe784eb62b9 in WebCore::RenderView::layoutContent(WebCore::LayoutState const&)
#14 0x00007fe784eb6719 in WebCore::RenderView::layout()
#15 0x00007fe784c20d2d in WebCore::FrameView::layout(bool)
#16 0x00007fe7848eee65 in WebCore::Document::implicitClose()
#17 0x00007fe784b57807 in WebCore::FrameLoader::checkCompleted()
#18 0x00007fe784bc09fe in WebCore::CachedResourceLoader::loadDone(WebCore::CachedResource*, bool)
#19 0x00007fe784b80362 in WebCore::SubresourceLoader::notifyDone()
#20 0x00007fe784b812a2 in WebCore::SubresourceLoader::didFinishLoading(double)
#21 0x00007fe78507f4e8 in WebCore::readCallback(_GObject*, _GAsyncResult*, void*) 

Also it is worth to mention that the call to invalidateBackgroundObscurationStatus has been added by commit: git show cae24e092 which also added test LayoutTests/fast/backgrounds/obscured-background-child-style-change.html. And the test still passes with my patch.

2*  contentChanged(ImageChanged):
It will not always endup with contentChanged(ImageChanged) as previously.
Indeed now it is always done except if intrinsicSizeChanged && !(imageSizeChanged || hasOverrideSize || containingBlockNeedsToRecomputePreferredSize || layoutSizeDependsOnIntrinsicSize). Which means when it should repaint.

Also I suspect that RenderImage::notifyFinished, see commit ce3a4e007, has been added because of this setLoading(false) not being set before notifying clients (= one of the point of my patch). You can see in that commit that layer()->rendererContentChanged() (old name contentChanged(ImageChanged)) has been  put both in imageChanged and notifyFinished.
Also this commit contains test LayoutTests/compositing/direct-image-compositing.html which still passes.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150128/2095e19c/attachment-0002.html>


More information about the webkit-unassigned mailing list