REGRESSION (AsyncImageDecoding): A tab with the WWDC keynote paused is killed for...
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 27 Jun 2017 03:28:00 +0000 (03:28 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 27 Jun 2017 03:28:00 +0000 (03:28 +0000)
https://bugs.webkit.org/show_bug.cgi?id=173804
<rdar://problem/32623745>

Reviewed by Simon Fraser.

Source/WebCore:

When under memory pressure MemoryCache::singleton().pruneLiveResources(true) is called inFrameView::didPaintContents()
after top level paint. We end up decoding and pruning bitmaps repeatedly for each tile, which is not great.

Situation gets worse with async decoding. Painting now doesn’t actually decode the image, it just starts the decoding.
When it completes we trigger another paint to get the bits to the tiles. The paint for the first tile then calls
pruneLiveResources and loses the bitmap and the second tile triggers another round of async decoding. We have code
that prevents pruning of visible images but non-visible images in tiling area can hit this bug easily.

Test: fast/images/low-memory-decode.html

* page/FrameView.cpp:
(WebCore::FrameView::willPaintContents):
(WebCore::FrameView::didPaintContents):

    Eliminate synchronous pruning during painting. This is an obsolete mechanism from early iOS times.

* platform/graphics/BitmapImage.cpp:
(WebCore::BitmapImage::imageFrameAvailableAtIndex):
(WebCore::BitmapImage::decodeCountForTesting):

    Testing support.

* platform/graphics/BitmapImage.h:
* testing/Internals.cpp:
(WebCore::Internals::imageDecodeCount):
* testing/Internals.h:
* testing/Internals.idl:

LayoutTests:

* fast/images/low-memory-decode-expected.txt: Added.
* fast/images/low-memory-decode.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@218826 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/images/low-memory-decode-expected.txt [new file with mode: 0644]
LayoutTests/fast/images/low-memory-decode.html [new file with mode: 0644]
LayoutTests/platform/mac-wk1/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/page/FrameView.cpp
Source/WebCore/platform/graphics/BitmapImage.cpp
Source/WebCore/platform/graphics/BitmapImage.h
Source/WebCore/testing/Internals.cpp
Source/WebCore/testing/Internals.h
Source/WebCore/testing/Internals.idl

index 1e6f3a6..950bd41 100644 (file)
@@ -1,3 +1,14 @@
+2017-06-26  Antti Koivisto  <antti@apple.com>
+
+        REGRESSION (AsyncImageDecoding): A tab with the WWDC keynote paused is killed for using excessive power (Image thrashing)
+        https://bugs.webkit.org/show_bug.cgi?id=173804
+        <rdar://problem/32623745>
+
+        Reviewed by Simon Fraser.
+
+        * fast/images/low-memory-decode-expected.txt: Added.
+        * fast/images/low-memory-decode.html: Added.
+
 2017-06-26  Matt Lewis  <jlewis3@apple.com>
 
         Marked media/media-source/media-source-paint-to-canvas.html as flaky.
diff --git a/LayoutTests/fast/images/low-memory-decode-expected.txt b/LayoutTests/fast/images/low-memory-decode-expected.txt
new file mode 100644 (file)
index 0000000..d5b8475
--- /dev/null
@@ -0,0 +1,2 @@
+Image decode count: 1
+
diff --git a/LayoutTests/fast/images/low-memory-decode.html b/LayoutTests/fast/images/low-memory-decode.html
new file mode 100644 (file)
index 0000000..edf0728
--- /dev/null
@@ -0,0 +1,24 @@
+<script>
+if (window.testRunner) {
+    testRunner.waitUntilDone();
+    testRunner.dumpAsText();
+    internals.settings.setLargeImageAsyncDecodingEnabled(true);
+       internals.beginSimulatedMemoryPressure();
+}
+function tryFinish() {
+    const decodeCount = internals.imageDecodeCount(image);
+    if (decodeCount < 1)
+        return;
+    log.innerHTML = `Image decode count: ${internals.imageDecodeCount(image)}`;
+    internals.endSimulatedMemoryPressure();
+    testRunner.notifyDone()
+}
+
+function test() {
+    if (!window.testRunner)
+        return;
+    setInterval(tryFinish, 100);
+}
+</script>
+<div id=log style="height:1000px"></div>
+<img id=image src="resources/flowchart.jpg" width="1000" height="1000" onload="test()">
index 942ad36..8526343 100644 (file)
@@ -376,3 +376,5 @@ webkit.org/b/172922 [ Debug ] webrtc/datachannel/basic.html [ Pass Timeout ]
 
 webkit.org/b/173432 [ Debug ] imported/w3c/web-platform-tests/fetch/nosniff/importscripts.html [ Pass Crash ]
 
+# requires wk2 speculative tiling
+fast/images/low-memory-decode.html [ Skip ]
index eadfce8..cf28c35 100644 (file)
@@ -1,3 +1,39 @@
+2017-06-26  Antti Koivisto  <antti@apple.com>
+
+        REGRESSION (AsyncImageDecoding): A tab with the WWDC keynote paused is killed for using excessive power (Image thrashing)
+        https://bugs.webkit.org/show_bug.cgi?id=173804
+        <rdar://problem/32623745>
+
+        Reviewed by Simon Fraser.
+
+        When under memory pressure MemoryCache::singleton().pruneLiveResources(true) is called inFrameView::didPaintContents()
+        after top level paint. We end up decoding and pruning bitmaps repeatedly for each tile, which is not great.
+
+        Situation gets worse with async decoding. Painting now doesn’t actually decode the image, it just starts the decoding.
+        When it completes we trigger another paint to get the bits to the tiles. The paint for the first tile then calls
+        pruneLiveResources and loses the bitmap and the second tile triggers another round of async decoding. We have code
+        that prevents pruning of visible images but non-visible images in tiling area can hit this bug easily.
+
+        Test: fast/images/low-memory-decode.html
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::willPaintContents):
+        (WebCore::FrameView::didPaintContents):
+
+            Eliminate synchronous pruning during painting. This is an obsolete mechanism from early iOS times.
+
+        * platform/graphics/BitmapImage.cpp:
+        (WebCore::BitmapImage::imageFrameAvailableAtIndex):
+        (WebCore::BitmapImage::decodeCountForTesting):
+
+            Testing support.
+
+        * platform/graphics/BitmapImage.h:
+        * testing/Internals.cpp:
+        (WebCore::Internals::imageDecodeCount):
+        * testing/Internals.h:
+        * testing/Internals.idl:
+
 2017-06-26  Chris Dumez  <cdumez@apple.com>
 
         ImageFrameCache::startAsyncDecodingQueue() unsafely passes Strings across threads
index 61e2695..2740204 100644 (file)
@@ -4371,14 +4371,6 @@ void FrameView::willPaintContents(GraphicsContext& context, const IntRect&, Pain
 
     paintingState.isTopLevelPainter = !sCurrentPaintTimeStamp;
 
-    if (paintingState.isTopLevelPainter && MemoryPressureHandler::singleton().isUnderMemoryPressure()) {
-        LOG(MemoryPressure, "Under memory pressure: %s", WTF_PRETTY_FUNCTION);
-
-        // To avoid unnecessary image decoding, we don't prune recently-decoded live resources here since
-        // we might need some live bitmaps on painting.
-        MemoryCache::singleton().prune();
-    }
-
     if (paintingState.isTopLevelPainter)
         sCurrentPaintTimeStamp = monotonicallyIncreasingTime();
 
@@ -4413,11 +4405,6 @@ void FrameView::didPaintContents(GraphicsContext& context, const IntRect& dirtyR
     m_paintBehavior = paintingState.paintBehavior;
     m_lastPaintTime = monotonicallyIncreasingTime();
 
-    // Painting can lead to decoding of large amounts of bitmaps
-    // If we are low on memory, wipe them out after the paint.
-    if (paintingState.isTopLevelPainter && MemoryPressureHandler::singleton().isUnderMemoryPressure())
-        MemoryCache::singleton().pruneLiveResources(true);
-
     // Regions may have changed as a result of the visibility/z-index of element changing.
 #if ENABLE(DASHBOARD_SUPPORT)
     if (frame().document()->annotatedRegionsDirty())
index 03c58be..7103f89 100644 (file)
@@ -230,6 +230,9 @@ void BitmapImage::draw(GraphicsContext& context, const FloatRect& destRect, cons
         image = frameImageAtIndexCacheIfNeeded(m_currentFrame, m_currentSubsamplingLevel, &context);
         if (!image) // If it's too early we won't have an image yet.
             return;
+
+        if (m_currentFrameDecodingStatus != ImageFrame::DecodingStatus::Complete)
+            ++m_decodeCountForTesting;
     }
 
     ASSERT(image);
@@ -494,10 +497,19 @@ void BitmapImage::imageFrameAvailableAtIndex(size_t index)
         m_source.stopAsyncDecodingQueue();
     if (m_currentFrameDecodingStatus == ImageFrame::DecodingStatus::Decoding)
         m_currentFrameDecodingStatus = frameDecodingStatusAtIndex(m_currentFrame);
+
+    if (m_currentFrameDecodingStatus == ImageFrame::DecodingStatus::Complete)
+        ++m_decodeCountForTesting;
+
     if (imageObserver())
         imageObserver()->imageFrameAvailable(*this, ImageAnimatingState::No);
 }
 
+unsigned BitmapImage::decodeCountForTesting() const
+{
+    return m_decodeCountForTesting;
+}
+
 void BitmapImage::dump(TextStream& ts) const
 {
     Image::dump(ts);
index da04995..33fc29e 100644 (file)
@@ -107,6 +107,8 @@ public:
     bool shouldUseAsyncDecodingForAnimatedImages();
     void setClearDecoderAfterAsyncFrameRequestForTesting(bool value) { m_clearDecoderAfterAsyncFrameRequestForTesting = value; }
 
+    WEBCORE_EXPORT unsigned decodeCountForTesting() const;
+
     // Accessors for native image formats.
 #if USE(APPKIT)
     NSImage *nsImage() override;
@@ -229,6 +231,8 @@ private:
     size_t m_cachedFrameCount { 0 };
 #endif
 
+    unsigned m_decodeCountForTesting { 0 };
+
 #if USE(APPKIT)
     mutable RetainPtr<NSImage> m_nsImage; // A cached NSImage of all the frames. Only built lazily if someone actually queries for one.
 #endif
index 5ae3c0e..2e39abf 100644 (file)
@@ -826,6 +826,19 @@ void Internals::setClearDecoderAfterAsyncFrameRequestForTesting(HTMLImageElement
     downcast<BitmapImage>(*image).setClearDecoderAfterAsyncFrameRequestForTesting(value);
 }
 
+unsigned Internals::imageDecodeCount(HTMLImageElement& element)
+{
+    auto* cachedImage = element.cachedImage();
+    if (!cachedImage)
+        return 0;
+
+    auto* image = cachedImage->image();
+    if (!is<BitmapImage>(image))
+        return 0;
+
+    return downcast<BitmapImage>(*image).decodeCountForTesting();
+}
+
 void Internals::setGridMaxTracksLimit(unsigned maxTrackLimit)
 {
     GridPosition::setMaxPositionForTesting(maxTrackLimit);
index 20895af..95ca882 100644 (file)
@@ -125,6 +125,7 @@ public:
     void resetImageAnimation(HTMLImageElement&);
     bool isImageAnimating(HTMLImageElement&);
     void setClearDecoderAfterAsyncFrameRequestForTesting(HTMLImageElement&, bool);
+    unsigned imageDecodeCount(HTMLImageElement&);
 
     void setGridMaxTracksLimit(unsigned);
 
index cb83b7d..648a8a2 100644 (file)
@@ -252,6 +252,7 @@ enum EventThrottlingBehavior {
     void resetImageAnimation(HTMLImageElement element);
     boolean isImageAnimating(HTMLImageElement element);
     void setClearDecoderAfterAsyncFrameRequestForTesting(HTMLImageElement element, boolean value);
+    unsigned long imageDecodeCount(HTMLImageElement element);
 
     void setGridMaxTracksLimit(unsigned long maxTracksLimit);