Diff
Modified: trunk/Source/WebCore/ChangeLog (223090 => 223091)
--- trunk/Source/WebCore/ChangeLog 2017-10-10 00:07:45 UTC (rev 223090)
+++ trunk/Source/WebCore/ChangeLog 2017-10-10 00:07:48 UTC (rev 223091)
@@ -1,3 +1,83 @@
+2017-10-09 Said Abou-Hallawa <[email protected]>
+
+ Image data should be coalesced if it comes in small chunks before updating the ImageSource
+ https://bugs.webkit.org/show_bug.cgi?id=175890
+
+ Reviewed by Simon Fraser.
+
+ Coalesce the updates, which an Image makes when receiving encoded data in
+ small chunks, for all platforms. Ensure the clients of the CachedImage
+ won't be notified unless an update in the ImageSource happens.
+
+ I need to change some functions' names to better implement this patch.
+ The names of these functions have been confusing:
+ CachedImage::addData(SharedBuffer&)
+ CachedImage::addDataBuffer(const char* data, unsigned)
+ CachedImage::addIncrementalDataBuffer(SharedBuffer&)
+
+ The image data is not buffered incrementally into the CachedImage. When
+ new data is received, SubresourceLoader calls CachedImage to "update" its
+ m_data with either a SharedBuffer or a data pointer. In either case the
+ SharedBuffer or the pointer contains all the loaded data. SubresourceLoader
+ calls CachedImage to update its m_data, to ensure its m_image is created
+ and to notify its clients with the new data.
+
+ The verb "add" in the functions' name is misleading. I am suggesting the
+ following names instead:
+ CachedImage::updateBuffer(SharedBuffer&)
+ CachedImage::updateData(const char*, unsigned)
+ CachedImage::doUpdateBuffer(SharedBuffer&)
+
+ The first two are the virtual ones. They are called form SubresourceLoader.
+ The third one is the internal implementation to update the m_data member.
+ The same names will be used in the following classes:
+ CachedResource which is the base class of CachedImage
+ CachedRawResource which is derived from CachedResource
+ CachedTextTrack which is derived from CachedResource
+
+ * html/ImageDocument.cpp:
+ (WebCore::ImageDocument::updateDuringParsing):
+ * loader/SubresourceLoader.cpp:
+ (WebCore::SubresourceLoader::didReceiveDataOrBuffer):
+ * loader/cache/CachedImage.cpp:
+ (WebCore::CachedImage::clearImage): Reset the update back off members.
+ (WebCore::CachedImage::doUpdateBuffer): Don't update CachedImage with
+ the new data if it comes in small chunks with fast rate.
+ (WebCore::CachedImage::shouldDeferUpdateImageData const): This code is moved
+ from ImageSource::dataChanged().
+ (WebCore::CachedImage::didUpdateImageData): Ditto.
+ (WebCore::CachedImage::updateImageData):
+ (WebCore::CachedImage::updateBuffer):
+ (WebCore::CachedImage::updateData):
+ (WebCore::CachedImage::finishLoading):
+ (WebCore::CachedImage::addIncrementalDataBuffer): Deleted.
+ (WebCore::CachedImage::setImageDataBuffer): Deleted.
+ (WebCore::CachedImage::addDataBuffer): Deleted.
+ (WebCore::CachedImage::addData): Deleted.
+ * loader/cache/CachedImage.h:
+ * loader/cache/CachedRawResource.cpp:
+ (WebCore::CachedRawResource::updateBuffer):
+ (WebCore::CachedRawResource::updateData):
+ (WebCore::CachedRawResource::addDataBuffer): Deleted.
+ (WebCore::CachedRawResource::addData): Deleted.
+ * loader/cache/CachedRawResource.h:
+ * loader/cache/CachedResource.cpp:
+ (WebCore::CachedResource::updateBuffer):
+ (WebCore::CachedResource::updateData):
+ (WebCore::CachedResource::addDataBuffer): Deleted.
+ (WebCore::CachedResource::addData): Deleted.
+ * loader/cache/CachedResource.h:
+ * loader/cache/CachedTextTrack.cpp:
+ (WebCore::CachedTextTrack::doUpdateBuffer): Rename updateData() to doUpdateBuffer().
+ (WebCore::CachedTextTrack::updateBuffer): Rename addDataBuffer() to updateBuffer().
+ (WebCore::CachedTextTrack::finishLoading): Call the internal function doUpdateBuffer().
+ (WebCore::CachedTextTrack::updateData): Deleted.
+ (WebCore::CachedTextTrack::addDataBuffer): Deleted.
+ * loader/cache/CachedTextTrack.h:
+ * platform/graphics/ImageSource.cpp:
+ (WebCore::ImageSource::dataChanged): Move the update back off code to CachedImage::updateData().
+ * platform/graphics/ImageSource.h:
+
2017-10-09 Michael Saboff <[email protected]>
Implement RegExp Unicode property escapes
Modified: trunk/Source/WebCore/html/ImageDocument.cpp (223090 => 223091)
--- trunk/Source/WebCore/html/ImageDocument.cpp 2017-10-10 00:07:45 UTC (rev 223090)
+++ trunk/Source/WebCore/html/ImageDocument.cpp 2017-10-10 00:07:48 UTC (rev 223091)
@@ -135,7 +135,7 @@
createDocumentStructure();
if (RefPtr<SharedBuffer> buffer = loader()->mainResourceData())
- m_imageElement->cachedImage()->addDataBuffer(*buffer);
+ m_imageElement->cachedImage()->updateBuffer(*buffer);
imageUpdated();
}
Modified: trunk/Source/WebCore/loader/SubresourceLoader.cpp (223090 => 223091)
--- trunk/Source/WebCore/loader/SubresourceLoader.cpp 2017-10-10 00:07:45 UTC (rev 223090)
+++ trunk/Source/WebCore/loader/SubresourceLoader.cpp 2017-10-10 00:07:48 UTC (rev 223091)
@@ -403,9 +403,9 @@
if (!m_loadingMultipartContent) {
if (auto* resourceData = this->resourceData())
- m_resource->addDataBuffer(*resourceData);
+ m_resource->updateBuffer(*resourceData);
else
- m_resource->addData(buffer ? buffer->data() : data, buffer ? buffer->size() : length);
+ m_resource->updateData(buffer ? buffer->data() : data, buffer ? buffer->size() : length);
}
}
Modified: trunk/Source/WebCore/loader/cache/CachedImage.cpp (223090 => 223091)
--- trunk/Source/WebCore/loader/cache/CachedImage.cpp 2017-10-10 00:07:45 UTC (rev 223090)
+++ trunk/Source/WebCore/loader/cache/CachedImage.cpp 2017-10-10 00:07:48 UTC (rev 223091)
@@ -404,18 +404,25 @@
}
m_image = nullptr;
+ m_lastUpdateImageDataTime = { };
+ m_updateImageDataCount = 0;
}
-void CachedImage::addIncrementalDataBuffer(SharedBuffer& data)
+void CachedImage::updateBufferInternal(SharedBuffer& data)
{
m_data = &data;
-
createImage();
+ setEncodedSize(m_image->data() ? m_image->data()->size() : 0);
- // Have the image update its data from its internal buffer.
- // It will not do anything now, but will delay decoding until
- // queried for info (like size or specific image frames).
- EncodedDataStatus encodedDataStatus = setImageDataBuffer(&data, false);
+ // Don't update the image with the new buffer very often. Changing the decoder
+ // internal data and repainting the observers sometimes are very expensive operations.
+ if (shouldDeferUpdateImageData())
+ return;
+
+ // Have the image update its data from its internal buffer. Decoding the image data
+ // will be delayed until info (like size or specific image frames) are queried which
+ // usually happens when the observers are repainted.
+ EncodedDataStatus encodedDataStatus = updateImageData(&data, false);
if (encodedDataStatus > EncodedDataStatus::Error && encodedDataStatus < EncodedDataStatus::SizeAvailable)
return;
@@ -430,31 +437,46 @@
}
// Tell our observers to try to draw.
- // Each chunk from the network causes observers to repaint, which will force that chunk to decode.
- // It would be nice to only redraw the decoded band of the image, but with the current design
- // (decoding delayed until painting) that seems hard.
notifyObservers();
+}
- setEncodedSize(m_image->data() ? m_image->data()->size() : 0);
+bool CachedImage::shouldDeferUpdateImageData() const
+{
+ static const double updateImageDataBackoffIntervals[] = { 0, 1, 3, 6, 15 };
+ unsigned interval = std::min<unsigned>(m_updateImageDataCount, 4);
+
+ // The first time through, the chunk time will be 0 and the image will get an update.
+ return (MonotonicTime::now() - m_lastUpdateImageDataTime).seconds() < updateImageDataBackoffIntervals[interval];
}
-EncodedDataStatus CachedImage::setImageDataBuffer(SharedBuffer* data, bool allDataReceived)
+void CachedImage::didUpdateImageData()
{
- return m_image ? m_image->setData(data, allDataReceived) : EncodedDataStatus::Error;
+ m_lastUpdateImageDataTime = MonotonicTime::now();
+ ASSERT(m_updateImageDataCount < std::numeric_limits<unsigned>::max());
+ ++m_updateImageDataCount;
}
-void CachedImage::addDataBuffer(SharedBuffer& data)
+EncodedDataStatus CachedImage::updateImageData(SharedBuffer* data, bool allDataReceived)
{
+ if (!m_image)
+ return EncodedDataStatus::Error;
+ EncodedDataStatus result = m_image->setData(data, allDataReceived);
+ didUpdateImageData();
+ return result;
+}
+
+void CachedImage::updateBuffer(SharedBuffer& data)
+{
ASSERT(dataBufferingPolicy() == BufferData);
- addIncrementalDataBuffer(data);
- CachedResource::addDataBuffer(data);
+ updateBufferInternal(data);
+ CachedResource::updateBuffer(data);
}
-void CachedImage::addData(const char* data, unsigned length)
+void CachedImage::updateData(const char* data, unsigned length)
{
ASSERT(dataBufferingPolicy() == DoNotBufferData);
- addIncrementalDataBuffer(SharedBuffer::create(data, length));
- CachedResource::addData(data, length);
+ updateBufferInternal(SharedBuffer::create(data, length));
+ CachedResource::updateData(data, length);
}
void CachedImage::finishLoading(SharedBuffer* data)
@@ -463,7 +485,7 @@
if (!m_image && data)
createImage();
- EncodedDataStatus encodedDataStatus = setImageDataBuffer(data, true);
+ EncodedDataStatus encodedDataStatus = updateImageData(data, true);
if (encodedDataStatus == EncodedDataStatus::Error || m_image->isNull()) {
// Image decoding failed; the image data is malformed.
Modified: trunk/Source/WebCore/loader/cache/CachedImage.h (223090 => 223091)
--- trunk/Source/WebCore/loader/cache/CachedImage.h 2017-10-10 00:07:45 UTC (rev 223090)
+++ trunk/Source/WebCore/loader/cache/CachedImage.h 2017-10-10 00:07:48 UTC (rev 223091)
@@ -68,7 +68,7 @@
bool imageHasRelativeWidth() const { return m_image && m_image->hasRelativeWidth(); }
bool imageHasRelativeHeight() const { return m_image && m_image->hasRelativeHeight(); }
- void addDataBuffer(SharedBuffer&) override;
+ void updateBuffer(SharedBuffer&) override;
void finishLoading(SharedBuffer*) override;
enum SizeType {
@@ -109,8 +109,10 @@
void allClientsRemoved() override;
void destroyDecodedData() override;
- EncodedDataStatus setImageDataBuffer(SharedBuffer*, bool allDataReceived);
- void addData(const char* data, unsigned length) override;
+ bool shouldDeferUpdateImageData() const;
+ void didUpdateImageData();
+ EncodedDataStatus updateImageData(SharedBuffer*, bool allDataReceived);
+ void updateData(const char* data, unsigned length) override;
void error(CachedResource::Status) override;
void responseReceived(const ResourceResponse&) override;
@@ -149,7 +151,7 @@
void imageFrameAvailable(const Image&, ImageAnimatingState, const IntRect* changeRect = nullptr, DecodingStatus = DecodingStatus::Invalid);
void changedInRect(const Image&, const IntRect*);
- void addIncrementalDataBuffer(SharedBuffer&);
+ void updateBufferInternal(SharedBuffer&);
void didReplaceSharedBufferContents() override;
@@ -166,6 +168,9 @@
RefPtr<CachedImageObserver> m_imageObserver;
RefPtr<Image> m_image;
+ MonotonicTime m_lastUpdateImageDataTime;
+ unsigned m_updateImageDataCount { 0 };
+
std::unique_ptr<SVGImageCache> m_svgImageCache;
bool m_isManuallyCached { false };
bool m_shouldPaintBrokenImage { true };
Modified: trunk/Source/WebCore/loader/cache/CachedRawResource.cpp (223090 => 223091)
--- trunk/Source/WebCore/loader/cache/CachedRawResource.cpp 2017-10-10 00:07:45 UTC (rev 223090)
+++ trunk/Source/WebCore/loader/cache/CachedRawResource.cpp 2017-10-10 00:07:48 UTC (rev 223091)
@@ -52,7 +52,7 @@
return data->getSomeData(previousDataLength);
}
-void CachedRawResource::addDataBuffer(SharedBuffer& data)
+void CachedRawResource::updateBuffer(SharedBuffer& data)
{
CachedResourceHandle<CachedRawResource> protectedThis(this);
ASSERT(dataBufferingPolicy() == BufferData);
@@ -69,14 +69,14 @@
return;
}
- CachedResource::addDataBuffer(data);
+ CachedResource::updateBuffer(data);
}
-void CachedRawResource::addData(const char* data, unsigned length)
+void CachedRawResource::updateData(const char* data, unsigned length)
{
ASSERT(dataBufferingPolicy() == DoNotBufferData);
notifyClientsDataWasReceived(data, length);
- CachedResource::addData(data, length);
+ CachedResource::updateData(data, length);
}
void CachedRawResource::finishLoading(SharedBuffer* data)
Modified: trunk/Source/WebCore/loader/cache/CachedRawResource.h (223090 => 223091)
--- trunk/Source/WebCore/loader/cache/CachedRawResource.h 2017-10-10 00:07:45 UTC (rev 223090)
+++ trunk/Source/WebCore/loader/cache/CachedRawResource.h 2017-10-10 00:07:48 UTC (rev 223091)
@@ -55,8 +55,8 @@
private:
void didAddClient(CachedResourceClient&) final;
- void addDataBuffer(SharedBuffer&) final;
- void addData(const char* data, unsigned length) final;
+ void updateBuffer(SharedBuffer&) final;
+ void updateData(const char* data, unsigned length) final;
void finishLoading(SharedBuffer*) final;
bool shouldIgnoreHTTPStatusCodeErrors() const override { return true; }
Modified: trunk/Source/WebCore/loader/cache/CachedResource.cpp (223090 => 223091)
--- trunk/Source/WebCore/loader/cache/CachedResource.cpp 2017-10-10 00:07:45 UTC (rev 223090)
+++ trunk/Source/WebCore/loader/cache/CachedResource.cpp 2017-10-10 00:07:48 UTC (rev 223091)
@@ -342,12 +342,12 @@
client->notifyFinished(*this);
}
-void CachedResource::addDataBuffer(SharedBuffer&)
+void CachedResource::updateBuffer(SharedBuffer&)
{
ASSERT(dataBufferingPolicy() == BufferData);
}
-void CachedResource::addData(const char*, unsigned)
+void CachedResource::updateData(const char*, unsigned)
{
ASSERT(dataBufferingPolicy() == DoNotBufferData);
}
Modified: trunk/Source/WebCore/loader/cache/CachedResource.h (223090 => 223091)
--- trunk/Source/WebCore/loader/cache/CachedResource.h 2017-10-10 00:07:45 UTC (rev 223090)
+++ trunk/Source/WebCore/loader/cache/CachedResource.h 2017-10-10 00:07:48 UTC (rev 223091)
@@ -102,8 +102,8 @@
virtual void setEncoding(const String&) { }
virtual String encoding() const { return String(); }
virtual const TextResourceDecoder* textResourceDecoder() const { return nullptr; }
- virtual void addDataBuffer(SharedBuffer&);
- virtual void addData(const char* data, unsigned length);
+ virtual void updateBuffer(SharedBuffer&);
+ virtual void updateData(const char* data, unsigned length);
virtual void finishLoading(SharedBuffer*);
virtual void error(CachedResource::Status);
Modified: trunk/Source/WebCore/loader/cache/CachedTextTrack.cpp (223090 => 223091)
--- trunk/Source/WebCore/loader/cache/CachedTextTrack.cpp 2017-10-10 00:07:45 UTC (rev 223090)
+++ trunk/Source/WebCore/loader/cache/CachedTextTrack.cpp 2017-10-10 00:07:48 UTC (rev 223091)
@@ -42,7 +42,7 @@
{
}
-void CachedTextTrack::updateData(SharedBuffer* data)
+void CachedTextTrack::doUpdateBuffer(SharedBuffer* data)
{
ASSERT(dataBufferingPolicy() == BufferData);
m_data = data;
@@ -53,15 +53,15 @@
client->deprecatedDidReceiveCachedResource(*this);
}
-void CachedTextTrack::addDataBuffer(SharedBuffer& data)
+void CachedTextTrack::updateBuffer(SharedBuffer& data)
{
- updateData(&data);
- CachedResource::addDataBuffer(data);
+ doUpdateBuffer(&data);
+ CachedResource::updateBuffer(data);
}
void CachedTextTrack::finishLoading(SharedBuffer* data)
{
- updateData(data);
+ doUpdateBuffer(data);
CachedResource::finishLoading(data);
}
Modified: trunk/Source/WebCore/loader/cache/CachedTextTrack.h (223090 => 223091)
--- trunk/Source/WebCore/loader/cache/CachedTextTrack.h 2017-10-10 00:07:45 UTC (rev 223090)
+++ trunk/Source/WebCore/loader/cache/CachedTextTrack.h 2017-10-10 00:07:48 UTC (rev 223091)
@@ -37,10 +37,10 @@
private:
bool mayTryReplaceEncodedData() const override { return true; }
- void addDataBuffer(SharedBuffer&) override;
+ void updateBuffer(SharedBuffer&) override;
void finishLoading(SharedBuffer*) override;
- void updateData(SharedBuffer*);
+ void doUpdateBuffer(SharedBuffer*);
};
} // namespace WebCore
Modified: trunk/Source/WebCore/platform/graphics/ImageSource.cpp (223090 => 223091)
--- trunk/Source/WebCore/platform/graphics/ImageSource.cpp 2017-10-10 00:07:45 UTC (rev 223090)
+++ trunk/Source/WebCore/platform/graphics/ImageSource.cpp 2017-10-10 00:07:48 UTC (rev 223091)
@@ -113,34 +113,13 @@
EncodedDataStatus ImageSource::dataChanged(SharedBuffer* data, bool allDataReceived)
{
-#if PLATFORM(IOS)
- // FIXME: We should expose a setting to enable/disable progressive loading and make this
- // code conditional on it. Then we can remove the PLATFORM(IOS)-guard.
- static const double chunkLoadIntervals[] = {0, 1, 3, 6, 15};
- double interval = chunkLoadIntervals[std::min(m_progressiveLoadChunkCount, static_cast<uint16_t>(4))];
-
- bool needsUpdate = false;
-
- // The first time through, the chunk time will be 0 and the image will get an update.
- if (currentTime() - m_progressiveLoadChunkTime > interval) {
- needsUpdate = true;
- m_progressiveLoadChunkTime = currentTime();
- ASSERT(m_progressiveLoadChunkCount <= std::numeric_limits<uint16_t>::max());
- ++m_progressiveLoadChunkCount;
- }
-
- if (needsUpdate || allDataReceived)
- setData(data, allDataReceived);
-#else
setData(data, allDataReceived);
-#endif
+ m_frameCache->clearMetadata();
- m_frameCache->clearMetadata();
EncodedDataStatus status = encodedDataStatus();
- if (status < EncodedDataStatus::SizeAvailable)
- return status;
+ if (status >= EncodedDataStatus::SizeAvailable)
+ m_frameCache->growFrames();
- m_frameCache->growFrames();
return status;
}
Modified: trunk/Source/WebCore/platform/graphics/ImageSource.h (223090 => 223091)
--- trunk/Source/WebCore/platform/graphics/ImageSource.h 2017-10-10 00:07:45 UTC (rev 223090)
+++ trunk/Source/WebCore/platform/graphics/ImageSource.h 2017-10-10 00:07:48 UTC (rev 223091)
@@ -121,12 +121,6 @@
std::optional<SubsamplingLevel> m_maximumSubsamplingLevel;
-#if PLATFORM(IOS)
- // FIXME: We should expose a setting to enable/disable progressive loading so that we can remove the PLATFORM(IOS)-guard.
- double m_progressiveLoadChunkTime { 0 };
- uint16_t m_progressiveLoadChunkCount { 0 };
-#endif
-
AlphaOption m_alphaOption { AlphaOption::Premultiplied };
GammaAndColorProfileOption m_gammaAndColorProfileOption { GammaAndColorProfileOption::Applied };
};
