Diff
Modified: trunk/LayoutTests/ChangeLog (206634 => 206635)
--- trunk/LayoutTests/ChangeLog 2016-09-30 14:20:25 UTC (rev 206634)
+++ trunk/LayoutTests/ChangeLog 2016-09-30 16:01:36 UTC (rev 206635)
@@ -1,3 +1,13 @@
+2016-09-30 Said Abou-Hallawa <sabouhall...@apple.com>
+
+ Change the MemoryCache and CachedResource adjustSize functions to take a long argument
+ https://bugs.webkit.org/show_bug.cgi?id=162708
+ <rdar://problem/28555702>
+
+ Reviewed by Brent Fulgham.
+
+ * TestExpectations: Remove failed tests.
+
2016-09-30 Chris Dumez <cdu...@apple.com>
Add support for ImageData.data attribute
Modified: trunk/LayoutTests/TestExpectations (206634 => 206635)
--- trunk/LayoutTests/TestExpectations 2016-09-30 14:20:25 UTC (rev 206634)
+++ trunk/LayoutTests/TestExpectations 2016-09-30 16:01:36 UTC (rev 206635)
@@ -985,18 +985,3 @@
# Only iOS has implemented lettepress.
fast/text/letterpress-different.html [ ImageOnlyFailure ]
-
-webkit.org/b/162696 [ Release ] fast/images/paletted-png-with-color-profile.html [ Crash ]
-webkit.org/b/162696 [ Release ] fast/images/paint-subrect.html [ Crash ]
-webkit.org/b/162696 [ Release ] fast/images/paint-subrect-grid.html [ Crash ]
-webkit.org/b/162696 [ Release ] fast/images/pdf-as-image-crop-box.html [ Crash ]
-webkit.org/b/162696 [ Release ] fast/images/link-body-content-imageDimensionChanged-crash.html [ Crash ]
-webkit.org/b/162696 [ Release ] fast/images/object-data-url-case-insensitivity.html [ Crash ]
-webkit.org/b/162696 [ Release ] fast/images/move-image-to-new-document.html [ Crash ]
-webkit.org/b/162696 [ Release ] fast/images/pdf-as-background.html [ Crash ]
-webkit.org/b/162696 [ Release ] fast/images/pdf-as-image-landscape.html [ Crash ]
-webkit.org/b/162696 [ Release ] fast/images/pdf-as-image-with-annotations-expected.html [ Crash ]
-webkit.org/b/162696 [ Release ] fast/images/pdf-as-image-too-big.html [ Crash ]
-webkit.org/b/162696 [ Release ] fast/images/object-image.html [ Crash ]
-webkit.org/b/162696 [ Release ] fast/images/pdf-as-image-with-annotations.html [ Crash ]
-webkit.org/b/162696 [ Release ] fast/images/load-img-with-empty-src.html [ Crash ]
Modified: trunk/Source/WebCore/ChangeLog (206634 => 206635)
--- trunk/Source/WebCore/ChangeLog 2016-09-30 14:20:25 UTC (rev 206634)
+++ trunk/Source/WebCore/ChangeLog 2016-09-30 16:01:36 UTC (rev 206635)
@@ -1,3 +1,63 @@
+2016-09-30 Said Abou-Hallawa <sabouhall...@apple.com>
+
+ Change the MemoryCache and CachedResource adjustSize functions to take a long argument
+ https://bugs.webkit.org/show_bug.cgi?id=162708
+ <rdar://problem/28555702>
+
+ Reviewed by Brent Fulgham.
+
+ Because the MemoryCache stores the size of the cached memory in unsigned,
+ two problems my happen when reporting a change in the size of the memory:
+
+ 1. Signed integer overflow -- which can happen because MemoryCache::adjustSize()
+ takes a signed integer argument. If the allocated or the freed memory size is
+ larger than the maximum of a signed integer, an overflow will happen.
+ For the image caching code, this can be seen where the unsigned decodedSize
+ is casted to an integer before passing it to ImageObserver::decodedSizeChanged().
+
+ 2. Unsigned integer overflow -- which can happen if the new allocated memory
+ size plus the currentSize exceeds the maximum of unsigned.
+ This can be seen in MemoryCache::adjustSize() where we add delta to m_liveSize
+ or m_deadSize without checking whether this addition will overflow or not. We
+ do not assert for overflow although we assert for underflow.
+
+ The fix for these two problems can be the following:
+
+ 1. Make all the adjustSize functions all the way till MemoryCache::adjustSize()
+ take a signed long integer argument.
+
+ 2. Do not create a NativeImagePtr for an ImageFrame if its frameBytes plus the
+ ImageFrameCache::decodedSize() will exceed the maximum of an unsigned integer.
+
+ * loader/cache/CachedImage.cpp:
+ (WebCore::CachedImage::decodedSizeChanged): Change the argument to be long. No overflow will happen when casting the argument from unsigned to long.
+ * loader/cache/CachedImage.h:
+ * loader/cache/CachedResource.cpp:
+ (WebCore::CachedResource::setDecodedSize): Use long integer casting when calling MemoryCache::adjustSize().
+ (WebCore::CachedResource::setEncodedSize): Ditto.
+ * loader/cache/MemoryCache.cpp:
+ (WebCore::MemoryCache::MemoryCache): Add as static assert to ensure sizeof(long long) can hold any unsigned or its negation.
+ (WebCore::MemoryCache::revalidationSucceeded): Use long integer casting when calling MemoryCache::adjustSize().
+ (WebCore::MemoryCache::remove): Ditto.
+ (WebCore::MemoryCache::adjustSize): Change the function argument to long integer. No overflow will happen when casting the argument from unsigned to long.
+ * loader/cache/MemoryCache.h:
+ * platform/graphics/ImageFrameCache.cpp:
+ (WebCore::ImageFrameCache::destroyIncompleteDecodedData): Call a function with its new name.
+ (WebCore::ImageFrameCache::decodedSizeChanged): Change the function argument to long integer. No overflow will happen when casting the argument from unsigned to long.
+ (WebCore::ImageFrameCache::decodedSizeIncreased): Use long integer casting when calling decodedSizeChanged().
+ (WebCore::ImageFrameCache::decodedSizeDecreased): Ditto.
+ (WebCore::ImageFrameCache::decodedSizeReset): Ditto.
+ (WebCore::ImageFrameCache::didDecodeProperties): Ditto.
+ (WebCore::ImageFrameCache::frameAtIndex): Do not create the NativeImage if adding its frameByes to the MemoryCache will cause numerical overflow.
+ (WebCore::ImageFrameCache::decodedSizeIncremented): Deleted. This function is renamed decodedSizeIncreased().
+ (WebCore::ImageFrameCache::decodedSizeDecremented): Deleted. This function is renamed decodedSizeDecreased().
+ * platform/graphics/ImageFrameCache.h:
+ * platform/graphics/ImageObserver.h:
+ * platform/graphics/IntSize.h:
+ (WebCore::IntSize::unclampedArea): Returns the area of an IntSize in size_t.
+ * platform/graphics/cg/PDFDocumentImage.cpp:
+ (WebCore::PDFDocumentImage::decodedSizeChanged): Use long integer casting when calling ImageObserver::decodedSizeChanged().
+
2016-09-30 Chris Dumez <cdu...@apple.com>
Add support for ImageData.data attribute
Modified: trunk/Source/WebCore/loader/cache/CachedImage.cpp (206634 => 206635)
--- trunk/Source/WebCore/loader/cache/CachedImage.cpp 2016-09-30 14:20:25 UTC (rev 206634)
+++ trunk/Source/WebCore/loader/cache/CachedImage.cpp 2016-09-30 16:01:36 UTC (rev 206635)
@@ -450,11 +450,12 @@
m_image->destroyDecodedData();
}
-void CachedImage::decodedSizeChanged(const Image* image, int delta)
+void CachedImage::decodedSizeChanged(const Image* image, long long delta)
{
if (!image || image != m_image)
return;
-
+
+ ASSERT(delta >= 0 || decodedSize() + delta >= 0);
setDecodedSize(decodedSize() + delta);
}
Modified: trunk/Source/WebCore/loader/cache/CachedImage.h (206634 => 206635)
--- trunk/Source/WebCore/loader/cache/CachedImage.h 2016-09-30 14:20:25 UTC (rev 206634)
+++ trunk/Source/WebCore/loader/cache/CachedImage.h 2016-09-30 16:01:36 UTC (rev 206635)
@@ -118,7 +118,7 @@
bool stillNeedsLoad() const override { return !errorOccurred() && status() == Unknown && !isLoading(); }
// ImageObserver
- void decodedSizeChanged(const Image*, int delta) override;
+ void decodedSizeChanged(const Image*, long long delta) override;
void didDraw(const Image*) override;
void animationAdvanced(const Image*) override;
Modified: trunk/Source/WebCore/loader/cache/CachedResource.cpp (206634 => 206635)
--- trunk/Source/WebCore/loader/cache/CachedResource.cpp 2016-09-30 14:20:25 UTC (rev 206634)
+++ trunk/Source/WebCore/loader/cache/CachedResource.cpp 2016-09-30 16:01:36 UTC (rev 206635)
@@ -649,13 +649,13 @@
if (size == m_decodedSize)
return;
- int delta = size - m_decodedSize;
+ long long delta = static_cast<long long>(size) - m_decodedSize;
// The object must be moved to a different queue, since its size has been changed.
// Remove before updating m_decodedSize, so we find the resource in the correct LRU list.
if (allowsCaching() && inCache())
MemoryCache::singleton().removeFromLRUList(*this);
-
+
m_decodedSize = size;
if (allowsCaching() && inCache()) {
@@ -686,7 +686,7 @@
if (size == m_encodedSize)
return;
- int delta = size - m_encodedSize;
+ long long delta = static_cast<long long>(size) - m_encodedSize;
// The object must be moved to a different queue, since its size has been changed.
// Remove before updating m_encodedSize, so we find the resource in the correct LRU list.
Modified: trunk/Source/WebCore/loader/cache/MemoryCache.cpp (206634 => 206635)
--- trunk/Source/WebCore/loader/cache/MemoryCache.cpp 2016-09-30 14:20:25 UTC (rev 206634)
+++ trunk/Source/WebCore/loader/cache/MemoryCache.cpp 2016-09-30 16:01:36 UTC (rev 206635)
@@ -71,6 +71,7 @@
, m_deadSize(0)
, m_pruneTimer(*this, &MemoryCache::prune)
{
+ static_assert(sizeof(long long) > sizeof(unsigned), "Numerical overflow can happen when adjusting the size of the cached memory.");
}
auto MemoryCache::sessionResourceMap(SessionID sessionID) const -> CachedResourceMap*
@@ -148,7 +149,7 @@
resource.setInCache(true);
resource.updateResponseAfterRevalidation(response);
insertInLRUList(resource);
- int delta = resource.size();
+ long long delta = resource.size();
if (resource.decodedSize() && resource.hasClients())
insertInLiveDecodedResourcesList(resource);
if (delta)
@@ -448,7 +449,7 @@
// Remove from the appropriate LRU list.
removeFromLRUList(resource);
removeFromLiveDecodedResourcesList(resource);
- adjustSize(resource.hasClients(), -static_cast<int>(resource.size()));
+ adjustSize(resource.hasClients(), -static_cast<long long>(resource.size()));
} else
ASSERT(resources->get(key) != &resource);
}
@@ -641,13 +642,13 @@
m_deadSize += resource.size();
}
-void MemoryCache::adjustSize(bool live, int delta)
+void MemoryCache::adjustSize(bool live, long long delta)
{
if (live) {
- ASSERT(delta >= 0 || ((int)m_liveSize + delta >= 0));
+ ASSERT(delta >= 0 || (static_cast<long long>(m_liveSize) + delta >= 0));
m_liveSize += delta;
} else {
- ASSERT(delta >= 0 || ((int)m_deadSize + delta >= 0));
+ ASSERT(delta >= 0 || (static_cast<long long>(m_deadSize) + delta >= 0));
m_deadSize += delta;
}
}
Modified: trunk/Source/WebCore/loader/cache/MemoryCache.h (206634 => 206635)
--- trunk/Source/WebCore/loader/cache/MemoryCache.h 2016-09-30 14:20:25 UTC (rev 206634)
+++ trunk/Source/WebCore/loader/cache/MemoryCache.h 2016-09-30 16:01:36 UTC (rev 206635)
@@ -133,7 +133,7 @@
void removeFromLRUList(CachedResource&);
// Called to adjust the cache totals when a resource changes size.
- void adjustSize(bool live, int delta);
+ void adjustSize(bool live, long long delta);
// Track decoded resources that are in the cache and referenced by a Web page.
void insertInLiveDecodedResourcesList(CachedResource&);
Modified: trunk/Source/WebCore/platform/graphics/ImageFrameCache.cpp (206634 => 206635)
--- trunk/Source/WebCore/platform/graphics/ImageFrameCache.cpp 2016-09-30 14:20:25 UTC (rev 206634)
+++ trunk/Source/WebCore/platform/graphics/ImageFrameCache.cpp 2016-09-30 16:01:36 UTC (rev 206635)
@@ -35,6 +35,9 @@
#include "ImageDecoder.h"
#endif
+#include <wtf/CheckedArithmetic.h>
+
+
namespace WebCore {
ImageFrameCache::ImageFrameCache(Image* image)
@@ -66,7 +69,7 @@
unsigned decodedSize = 0;
for (size_t i = 0; i < count; ++i)
decodedSize += m_frames[i].clearImage();
-
+
decodedSizeReset(decodedSize);
}
@@ -93,12 +96,11 @@
decodedSize += frame.clear();
}
-
- decodedSizeDecremented(decodedSize);
+
+ decodedSizeDecreased(decodedSize);
}
-
-void ImageFrameCache::decodedSizeChanged(int decodedSize)
+void ImageFrameCache::decodedSizeChanged(long long decodedSize)
{
if (!decodedSize || !m_image || !m_image->imageObserver())
return;
@@ -106,7 +108,7 @@
m_image->imageObserver()->decodedSizeChanged(m_image, decodedSize);
}
-void ImageFrameCache::decodedSizeIncremented(unsigned decodedSize)
+void ImageFrameCache::decodedSizeIncreased(unsigned decodedSize)
{
if (!decodedSize)
return;
@@ -115,19 +117,19 @@
// The fully-decoded frame will subsume the partially decoded data used
// to determine image properties.
- int changeSize = decodedSize - m_decodedPropertiesSize;
+ long long changeSize = static_cast<long long>(decodedSize) - m_decodedPropertiesSize;
m_decodedPropertiesSize = 0;
decodedSizeChanged(changeSize);
}
-void ImageFrameCache::decodedSizeDecremented(unsigned decodedSize)
+void ImageFrameCache::decodedSizeDecreased(unsigned decodedSize)
{
if (!decodedSize)
return;
-
+
ASSERT(m_decodedSize >= decodedSize);
m_decodedSize -= decodedSize;
- decodedSizeChanged(-safeCast<int>(decodedSize));
+ decodedSizeChanged(-static_cast<long long>(decodedSize));
}
void ImageFrameCache::decodedSizeReset(unsigned decodedSize)
@@ -134,12 +136,12 @@
{
ASSERT(m_decodedSize >= decodedSize);
m_decodedSize -= decodedSize;
-
+
// Clearing the ImageSource destroys the extra decoded data used for
// determining image properties.
decodedSize += m_decodedPropertiesSize;
m_decodedPropertiesSize = 0;
- decodedSizeChanged(-safeCast<int>(decodedSize));
+ decodedSizeChanged(-static_cast<long long>(decodedSize));
}
void ImageFrameCache::didDecodeProperties(unsigned decodedPropertiesSize)
@@ -146,8 +148,8 @@
{
if (m_decodedSize)
return;
-
- int decodedSize = decodedPropertiesSize - m_decodedPropertiesSize;
+
+ long long decodedSize = static_cast<long long>(decodedPropertiesSize) - m_decodedPropertiesSize;
m_decodedPropertiesSize = decodedPropertiesSize;
decodedSizeChanged(decodedSize);
}
@@ -215,7 +217,7 @@
if (frame.hasInvalidNativeImage(subsamplingLevel)) {
unsigned decodedSize = frame.clear();
- decodedSizeDecremented(decodedSize);
+ decodedSizeDecreased(decodedSize);
}
if (!frame.isComplete() && caching == ImageFrame::Caching::Metadata)
@@ -222,8 +224,13 @@
setFrameMetadata(index, subsamplingLevel);
if (!frame.hasNativeImage() && caching == ImageFrame::Caching::MetadataAndImage) {
- setFrameNativeImage(m_decoder->createFrameImageAtIndex(index, subsamplingLevel), index, subsamplingLevel);
- decodedSizeIncremented(frame.frameBytes());
+ size_t frameBytes = size().unclampedArea() * sizeof(RGBA32);
+
+ // Do not create the NativeImage if adding its frameByes to the MemoryCache will cause numerical overflow.
+ if (WTF::isInBounds<unsigned>(frameBytes + decodedSize())) {
+ setFrameNativeImage(m_decoder->createFrameImageAtIndex(index, subsamplingLevel), index, subsamplingLevel);
+ decodedSizeIncreased(frame.frameBytes());
+ }
}
return frame;
Modified: trunk/Source/WebCore/platform/graphics/ImageFrameCache.h (206634 => 206635)
--- trunk/Source/WebCore/platform/graphics/ImageFrameCache.h 2016-09-30 14:20:25 UTC (rev 206634)
+++ trunk/Source/WebCore/platform/graphics/ImageFrameCache.h 2016-09-30 16:01:36 UTC (rev 206635)
@@ -87,10 +87,10 @@
T frameMetadataAtIndex(size_t index, SubsamplingLevel = SubsamplingLevel::Undefinded, ImageFrame::Caching = ImageFrame::Caching::Empty, Optional<T>* = nullptr);
bool isDecoderAvailable() const { return m_decoder; }
- void decodedSizeChanged(int decodedSize);
+ void decodedSizeChanged(long long decodedSize);
void didDecodeProperties(unsigned decodedPropertiesSize);
- void decodedSizeIncremented(unsigned decodedSize);
- void decodedSizeDecremented(unsigned decodedSize);
+ void decodedSizeIncreased(unsigned decodedSize);
+ void decodedSizeDecreased(unsigned decodedSize);
void decodedSizeReset(unsigned decodedSize);
void setNativeImage(NativeImagePtr&&);
Modified: trunk/Source/WebCore/platform/graphics/ImageObserver.h (206634 => 206635)
--- trunk/Source/WebCore/platform/graphics/ImageObserver.h 2016-09-30 14:20:25 UTC (rev 206634)
+++ trunk/Source/WebCore/platform/graphics/ImageObserver.h 2016-09-30 16:01:36 UTC (rev 206635)
@@ -37,7 +37,7 @@
protected:
virtual ~ImageObserver() {}
public:
- virtual void decodedSizeChanged(const Image*, int delta) = 0;
+ virtual void decodedSizeChanged(const Image*, long long delta) = 0;
virtual void didDraw(const Image*) = 0;
virtual void animationAdvanced(const Image*) = 0;
Modified: trunk/Source/WebCore/platform/graphics/IntSize.h (206634 => 206635)
--- trunk/Source/WebCore/platform/graphics/IntSize.h 2016-09-30 14:20:25 UTC (rev 206634)
+++ trunk/Source/WebCore/platform/graphics/IntSize.h 2016-09-30 16:01:36 UTC (rev 206635)
@@ -136,6 +136,11 @@
return abs(m_width) * abs(m_height);
}
+ size_t unclampedArea() const
+ {
+ return static_cast<size_t>(abs(m_width)) * abs(m_height);
+ }
+
int diagonalLengthSquared() const
{
return m_width * m_width + m_height * m_height;
Modified: trunk/Source/WebCore/platform/graphics/cg/PDFDocumentImage.cpp (206634 => 206635)
--- trunk/Source/WebCore/platform/graphics/cg/PDFDocumentImage.cpp 2016-09-30 14:20:25 UTC (rev 206634)
+++ trunk/Source/WebCore/platform/graphics/cg/PDFDocumentImage.cpp 2016-09-30 16:01:36 UTC (rev 206635)
@@ -182,7 +182,7 @@
return;
if (imageObserver())
- imageObserver()->decodedSizeChanged(this, -safeCast<int>(m_cachedBytes) + newCachedBytes);
+ imageObserver()->decodedSizeChanged(this, -static_cast<long long>(m_cachedBytes) + newCachedBytes);
ASSERT(s_allDecodedDataSize >= m_cachedBytes);
// Update with the difference in two steps to avoid unsigned underflow subtraction.
