FlushImmediately paint should force all the contents of the page to be painted synchronously. But all the images which are pending for asynchronous image decoding will be waiting for the decoding to finish. So they are not going to be painted during this FlushImmediately pass. Instead the background of these images will still be painted. To fix this issue, the renderers of these pending images have to be repainted and their images have to be drawn synchronously.
<rdar://problem/41873022>
Created attachment 344742 [details] Patch
Comment on attachment 344742 [details] Patch Attachment 344742 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/8500741 Number of test failures exceeded the failure limit.
Created attachment 344747 [details] Archive of layout-test-results from ews122 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews122 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.4
Created attachment 344810 [details] Patch
Comment on attachment 344810 [details] Patch Attachment 344810 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/8511579 New failing tests: http/tests/security/video-poster-cross-origin-crash2.html
Created attachment 344821 [details] Archive of layout-test-results from ews204 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews204 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment on attachment 344810 [details] Patch Attachment 344810 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/8511538 New failing tests: fast/inline/out-of-flow-positioned-render-replaced-box-moves.html
Created attachment 344823 [details] Archive of layout-test-results from ews125 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.4
Created attachment 344848 [details] Patch
Comment on attachment 344848 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=344848&action=review > Source/WebCore/page/FrameView.h:643 > + WEBCORE_EXPORT void traverseRenderTreeForNonPainting(GraphicsContext::NonPaintingReasons); Something about the parts of speech feels really weird here. Or maybe it’s because “non-painting” is not a common phrase. > Source/WebCore/platform/ScrollView.cpp:1170 > + if (context.paintingDisabled() && !context.updatingNonPainting()) This is just clearly worded wrong, you’re not “updating a non-painting”.
Created attachment 344911 [details] Patch
Comment on attachment 344911 [details] Patch Attachment 344911 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/8522318 New failing tests: accessibility/smart-invert-reference.html
Created attachment 344917 [details] Archive of layout-test-results from ews106 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Created attachment 344947 [details] Patch
Comment on attachment 344848 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=344848&action=review >> Source/WebCore/page/FrameView.h:643 >> + WEBCORE_EXPORT void traverseRenderTreeForNonPainting(GraphicsContext::NonPaintingReasons); > > Something about the parts of speech feels really weird here. Or maybe it’s because “non-painting” is not a common phrase. I made these function have the following prototypes: void scanRenderTree(GraphicsContext::ScanningFunction); void paintControlTints() { scanRenderTree(GraphicsContext::ScanningFunction::PaintingControlTints); } void repaintImagesPendingDecoding() { scanRenderTree(GraphicsContext::ScanningFunction::RepaintingImagesPendingDecoding); } >> Source/WebCore/platform/ScrollView.cpp:1170 >> + if (context.paintingDisabled() && !context.updatingNonPainting()) > > This is just clearly worded wrong, you’re not “updating a non-painting”. I made the functions in GraphicsContext.h have the following name: bool isScanningForUpdating() const { return m_scanningFunction != ScanningFunction::None; } bool isPaintingControlTints() const { return m_scanningFunction == ScanningFunction::PaintingControlTints; } bool isRepaintingImagesPendingDecoding() const { return m_scanningFunction == ScanningFunction::RepaintingImagesPendingDecoding; }
Comment on attachment 344848 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=344848&action=review >>> Source/WebCore/page/FrameView.h:643 >>> + WEBCORE_EXPORT void traverseRenderTreeForNonPainting(GraphicsContext::NonPaintingReasons); >> >> Something about the parts of speech feels really weird here. Or maybe it’s because “non-painting” is not a common phrase. > > I made these function have the following prototypes: > > void scanRenderTree(GraphicsContext::ScanningFunction); > void paintControlTints() { scanRenderTree(GraphicsContext::ScanningFunction::PaintingControlTints); } > void repaintImagesPendingDecoding() { scanRenderTree(GraphicsContext::ScanningFunction::RepaintingImagesPendingDecoding); } I don't like "scan": it doesn't tell you that it's doing a pass in painting order, and it sounds like it's looking for something, rather than traversing the entire tree. Maybe "traverseForPaintInvalidation()" or something. > Source/WebCore/platform/graphics/GraphicsContext.h:262 > enum class NonPaintingReasons { I think we can make this less negative, and call them paintInvalidationReasons. > Source/WebCore/platform/graphics/GraphicsContext.h:273 > + bool updatingNonPainting() const { return m_nonPaintingReasons != NonPaintingReasons::NoReasons; } performingPaintInvalidation()? > Source/WebCore/platform/graphics/GraphicsContext.h:274 > bool updatingControlTints() const { return m_nonPaintingReasons == NonPaintingReasons::UpdatingControlTints; } invalidatingControlTints > Source/WebCore/platform/graphics/GraphicsContext.h:275 > + bool updatingPendingImageDecoding() const { return m_nonPaintingReasons == NonPaintingReasons::UpdatingPendingImageDecoding; } invalidatingImagesWithAsyncDecodes
Created attachment 344970 [details] Patch
Comment on attachment 344848 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=344848&action=review >>>> Source/WebCore/page/FrameView.h:643 >>>> + WEBCORE_EXPORT void traverseRenderTreeForNonPainting(GraphicsContext::NonPaintingReasons); >>> >>> Something about the parts of speech feels really weird here. Or maybe it’s because “non-painting” is not a common phrase. >> >> I made these function have the following prototypes: >> >> void scanRenderTree(GraphicsContext::ScanningFunction); >> void paintControlTints() { scanRenderTree(GraphicsContext::ScanningFunction::PaintingControlTints); } >> void repaintImagesPendingDecoding() { scanRenderTree(GraphicsContext::ScanningFunction::RepaintingImagesPendingDecoding); } > > I don't like "scan": it doesn't tell you that it's doing a pass in painting order, and it sounds like it's looking for something, rather than traversing the entire tree. > > Maybe "traverseForPaintInvalidation()" or something. Function was renamed to traverseForPaintInvalidation(). >> Source/WebCore/platform/graphics/GraphicsContext.h:262 >> enum class NonPaintingReasons { > > I think we can make this less negative, and call them paintInvalidationReasons. This enum was renamed to enum class PaintInvalidationReasons { None, InvalidatingControlTints, InvalidatingImagesWithAsyncDecodes }; >> Source/WebCore/platform/graphics/GraphicsContext.h:273 >> + bool updatingNonPainting() const { return m_nonPaintingReasons != NonPaintingReasons::NoReasons; } > > performingPaintInvalidation()? Done. >> Source/WebCore/platform/graphics/GraphicsContext.h:274 >> bool updatingControlTints() const { return m_nonPaintingReasons == NonPaintingReasons::UpdatingControlTints; } > > invalidatingControlTints Done. >> Source/WebCore/platform/graphics/GraphicsContext.h:275 >> + bool updatingPendingImageDecoding() const { return m_nonPaintingReasons == NonPaintingReasons::UpdatingPendingImageDecoding; } > > invalidatingImagesWithAsyncDecodes Done.
Comment on attachment 344970 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=344970&action=review > Source/WebCore/loader/cache/CachedImage.cpp:143 > +bool CachedImage::isPendingImageDrawingClient(CachedImageClient& client) const This function name is hard to parse. Does it mean "Is client of pending image drawing" or "has image drawing client which is pending (something)"? > Source/WebCore/loader/cache/CachedImage.cpp:167 > +void CachedImage::clearPendingImageDrawingClient() Also hard to parse. > Source/WebCore/platform/graphics/GraphicsContext.h:262 > + enum class PaintInvalidationReasons { : unint8_t
Created attachment 345126 [details] Patch
Created attachment 345129 [details] Patch
Comment on attachment 345129 [details] Patch Clearing flags on attachment: 345129 Committed r233872: <https://trac.webkit.org/changeset/233872>
All reviewed patches have been landed. Closing bug.