Index: third_party/WebKit/Source/core/fetch/ImageResource.cpp |
diff --git a/third_party/WebKit/Source/core/fetch/ImageResource.cpp b/third_party/WebKit/Source/core/fetch/ImageResource.cpp |
index 1214c2ca111d30ddcbf52936e152a59ed2f5f6fa..0aaab6ee31f5eb01df4e3f58227731ec7eefd540 100644 |
--- a/third_party/WebKit/Source/core/fetch/ImageResource.cpp |
+++ b/third_party/WebKit/Source/core/fetch/ImageResource.cpp |
@@ -81,7 +81,6 @@ ImageResource::ImageResource(blink::Image* image, const ResourceLoaderOptions& o |
ImageResource::~ImageResource() |
{ |
WTF_LOG(Timers, "~ImageResource %p", this); |
- clearImage(); |
} |
DEFINE_TRACE(ImageResource) |
@@ -124,10 +123,7 @@ void ImageResource::addObserver(ImageResourceObserver* observer) |
if (isCacheValidator()) |
return; |
- if (m_data && !m_image && !errorOccurred()) { |
- createImage(); |
- m_image->setData(m_data, true); |
- } |
+ DCHECK(!m_data || m_image); |
if (m_image && !m_image->isNull()) { |
observer->imageChanged(this); |
@@ -190,18 +186,15 @@ bool ImageResource::isSafeToUnlock() const |
void ImageResource::destroyDecodedDataForFailedRevalidation() |
{ |
- m_image = nullptr; |
- setDecodedSize(0); |
+ m_image->destroyDecodedData(); |
hiroshige
2016/06/29 07:04:06
I think we have to clear |m_image| here, because w
hiroshige
2016/06/29 08:20:52
Created the tests. https://codereview.chromium.org
hajimehoshi
2016/07/04 10:46:24
Done.
|
} |
void ImageResource::destroyDecodedDataIfPossible() |
{ |
- if (!hasClientsOrObservers() && !isLoading() && (!m_image || (m_image->hasOneRef() && m_image->isBitmapImage()))) { |
- m_image = nullptr; |
- setDecodedSize(0); |
- } else if (m_image && !errorOccurred()) { |
+ if (!m_image) |
+ return; |
+ if ((!hasClientsOrObservers() && !isLoading() && m_image->hasOneRef() && m_image->isBitmapImage()) || !errorOccurred()) |
hiroshige
2016/06/29 07:04:06
I'm not sure, but can we simplify this condition t
hajimehoshi
2016/07/04 10:46:24
As we talked offline, let's keep this condition as
|
m_image->destroyDecodedData(); |
- } |
} |
void ImageResource::doResetAnimation() |
@@ -226,6 +219,16 @@ void ImageResource::allClientsAndObserversRemoved() |
Resource::allClientsAndObserversRemoved(); |
} |
+PassRefPtr<SharedBuffer> ImageResource::resourceBuffer() const |
+{ |
+ RefPtr<SharedBuffer> data = Resource::resourceBuffer(); |
+ if (data) |
+ return data; |
f(malita)
2016/06/30 18:21:46
Nit: return data.release();
hajimehoshi
2016/07/04 10:46:24
Done.
|
+ if (m_image) |
+ return m_image->data(); |
+ return nullptr; |
+} |
+ |
void ImageResource::appendData(const char* data, size_t length) |
{ |
if (m_multipartParser) { |
@@ -329,16 +332,16 @@ void ImageResource::notifyObservers(const IntRect* changeRect) |
void ImageResource::clear() |
{ |
prune(); |
- clearImage(); |
hiroshige
2016/06/29 07:04:06
Perhaps, we need to call clearImage() here, becaus
hajimehoshi
2016/07/04 10:46:24
Done.
|
m_data.clear(); |
setEncodedSize(0); |
} |
inline void ImageResource::createImage() |
{ |
- // Create the image if it doesn't yet exist. |
+ // When |m_image| exists here, this is a previous part of a multipart |
+ // response. Recreate it. |
if (m_image) |
- return; |
+ m_image = nullptr; |
scroggo_chromium
2016/06/28 15:33:44
It's not obvious to me how this relates to the res
hiroshige
2016/06/29 07:04:06
I think it's better to call clearImage() explicitl
hajimehoshi
2016/07/04 10:46:24
Done.
|
if (m_response.mimeType() == "image/svg+xml") { |
m_image = SVGImage::create(this); |
@@ -347,17 +350,6 @@ inline void ImageResource::createImage() |
} |
} |
-inline void ImageResource::clearImage() |
-{ |
- if (!m_image) |
- return; |
- |
- // If our Image has an observer, it's always us so we need to clear the back pointer |
- // before dropping our reference. |
- m_image->clearImageObserver(); |
scroggo_chromium
2016/06/28 15:33:44
Why is it okay to drop this line? My first thought
hajimehoshi
2016/07/04 10:46:24
Ah, you're right. I'll revert this function and ca
|
- m_image.clear(); |
-} |
- |
void ImageResource::updateImage(bool allDataReceived) |
{ |
TRACE_EVENT0("blink", "ImageResource::updateImage"); |
@@ -370,7 +362,7 @@ void ImageResource::updateImage(bool allDataReceived) |
// 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). |
- if (m_image) |
+ if (m_image && m_data) |
scroggo_chromium
2016/06/28 15:33:44
If we have m_data, aren't we guaranteed to also ha
hajimehoshi
2016/07/04 10:46:24
Right, now m_image's lifetime should include m_dat
|
sizeAvailable = m_image->setData(m_data, allDataReceived); |
// Go ahead and tell our observers to try to draw if we have either |
@@ -390,13 +382,9 @@ void ImageResource::updateImage(bool allDataReceived) |
// (decoding delayed until painting) that seems hard. |
notifyObservers(); |
} |
-} |
-void ImageResource::updateImageAndClearBuffer() |
-{ |
- clearImage(); |
- updateImage(true); |
- m_data.clear(); |
+ if (allDataReceived) |
+ m_data.clear(); |
} |
void ImageResource::finish(double loadFinishTime) |
@@ -404,7 +392,7 @@ void ImageResource::finish(double loadFinishTime) |
if (m_multipartParser) { |
m_multipartParser->finish(); |
if (m_data) |
- updateImageAndClearBuffer(); |
+ updateImage(true); |
} else { |
updateImage(true); |
} |
@@ -518,7 +506,7 @@ void ImageResource::reloadIfLoFi(ResourceFetcher* fetcher) |
if (isLoading()) |
m_loader->cancel(); |
else |
- updateImageAndClearBuffer(); |
+ updateImage(true); |
setStatus(NotStarted); |
fetcher->startLoad(this); |
} |
@@ -540,7 +528,7 @@ void ImageResource::onePartInMultipartReceived(const ResourceResponse& response) |
m_multipartParsingState = MultipartParsingState::ParsingFirstPart; |
return; |
} |
- updateImageAndClearBuffer(); |
+ updateImage(true); |
if (m_multipartParsingState == MultipartParsingState::ParsingFirstPart) { |
m_multipartParsingState = MultipartParsingState::FinishedParsingFirstPart; |