diff options
13 files changed, 114 insertions, 48 deletions
diff --git a/third_party/WebKit/Source/core/fetch/ImageResource.cpp b/third_party/WebKit/Source/core/fetch/ImageResource.cpp index 60911b1..f48ff7d 100644 --- a/third_party/WebKit/Source/core/fetch/ImageResource.cpp +++ b/third_party/WebKit/Source/core/fetch/ImageResource.cpp @@ -101,6 +101,19 @@ DEFINE_TRACE(ImageResource) MultipartImageResourceParser::Client::trace(visitor); } +void ImageResource::notifyObserver(ImageResourceObserver* observer, bool isNotifyingFinish, const IntRect* changeRect) +{ + if (isNotifyingFinish) { + observer->imageNotifyFinished(this); + if (m_observers.contains(observer)) { + m_finishedObservers.add(observer); + m_observers.remove(observer); + } + } + + observer->imageChanged(this, changeRect); +} + void ImageResource::addObserver(ImageResourceObserver* observer) { willAddClientOrObserver(); @@ -115,29 +128,49 @@ void ImageResource::addObserver(ImageResourceObserver* observer) m_image->setData(m_data, true); } - if (m_image && !m_image->isNull()) - observer->imageChanged(this); + if (m_image && !m_image->isNull()) { + bool isNotifyingFinish = !isLoading() && !stillNeedsLoad(); + notifyObserver(observer, isNotifyingFinish); + } } void ImageResource::removeObserver(ImageResourceObserver* observer) { ASSERT(observer); - ASSERT(m_observers.contains(observer)); - m_observers.remove(observer); + + if (m_observers.contains(observer)) + m_observers.remove(observer); + else if (m_finishedObservers.contains(observer)) + m_finishedObservers.remove(observer); + else + ASSERT_NOT_REACHED(); + didRemoveClientOrObserver(); } +static void priorityFromObserver(const ImageResourceObserver* observer, ResourcePriority& priority) +{ + ResourcePriority nextPriority = observer->computeResourcePriority(); + if (nextPriority.visibility == ResourcePriority::NotVisible) + return; + priority.visibility = ResourcePriority::Visible; + priority.intraPriorityValue += nextPriority.intraPriorityValue; +} + ResourcePriority ImageResource::priorityFromObservers() { ResourcePriority priority; - ImageResourceObserverWalker w(m_observers); - while (const auto* observer = w.next()) { - ResourcePriority nextPriority = observer->computeResourcePriority(); - if (nextPriority.visibility == ResourcePriority::NotVisible) - continue; - priority.visibility = ResourcePriority::Visible; - priority.intraPriorityValue += nextPriority.intraPriorityValue; + + ImageResourceObserverWalker finishedWalker(m_finishedObservers); + while (const auto* observer = finishedWalker.next()) { + priorityFromObserver(observer, priority); } + + ImageResourceObserverWalker walker(m_observers); + while (const auto* observer = walker.next()) { + priorityFromObserver(observer, priority); + } + return priority; } @@ -259,11 +292,16 @@ LayoutSize ImageResource::imageSize(RespectImageOrientationEnum shouldRespectIma return size; } -void ImageResource::notifyObservers(const IntRect* changeRect) +void ImageResource::notifyObservers(bool isNotifyingFinish, const IntRect* changeRect) { - ImageResourceObserverWalker w(m_observers); - while (auto* observer = w.next()) { - observer->imageChanged(this, changeRect); + ImageResourceObserverWalker finishedWalker(m_finishedObservers); + while (auto* observer = finishedWalker.next()) { + notifyObserver(observer, false, changeRect); + } + + ImageResourceObserverWalker walker(m_observers); + while (auto* observer = walker.next()) { + notifyObserver(observer, isNotifyingFinish, changeRect); } } @@ -333,7 +371,7 @@ void ImageResource::updateImage(bool allDataReceived) // 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(); + notifyObservers(allDataReceived); } } @@ -358,7 +396,7 @@ void ImageResource::error(Resource::Status status) m_multipartParser->cancel(); clear(); Resource::error(status); - notifyObservers(); + notifyObservers(true); } void ImageResource::responseReceived(const ResourceResponse& response, PassOwnPtr<WebDataConsumerHandle> handle) @@ -405,11 +443,18 @@ bool ImageResource::shouldPauseAnimation(const blink::Image* image) if (!image || image != m_image) return false; - ImageResourceObserverWalker w(m_observers); - while (auto* observer = w.next()) { + ImageResourceObserverWalker finishedWalker(m_finishedObservers); + while (auto* observer = finishedWalker.next()) { if (observer->willRenderImage()) return false; } + + ImageResourceObserverWalker walker(m_observers); + while (auto* observer = walker.next()) { + if (observer->willRenderImage()) + return false; + } + return true; } @@ -417,7 +462,7 @@ void ImageResource::animationAdvanced(const blink::Image* image) { if (!image || image != m_image) return; - notifyObservers(); + notifyObservers(false); } void ImageResource::updateImageAnimationPolicy() @@ -426,8 +471,15 @@ void ImageResource::updateImageAnimationPolicy() return; ImageAnimationPolicy newPolicy = ImageAnimationPolicyAllowed; - ImageResourceObserverWalker w(m_observers); - while (auto* observer = w.next()) { + + ImageResourceObserverWalker finishedWalker(m_finishedObservers); + while (auto* observer = finishedWalker.next()) { + if (observer->getImageAnimationPolicy(newPolicy)) + break; + } + + ImageResourceObserverWalker walker(m_observers); + while (auto* observer = walker.next()) { if (observer->getImageAnimationPolicy(newPolicy)) break; } @@ -452,7 +504,7 @@ void ImageResource::changedInRect(const blink::Image* image, const IntRect& rect { if (!image || image != m_image) return; - notifyObservers(&rect); + notifyObservers(false, &rect); } void ImageResource::onePartInMultipartReceived(const ResourceResponse& response) diff --git a/third_party/WebKit/Source/core/fetch/ImageResource.h b/third_party/WebKit/Source/core/fetch/ImageResource.h index 74849d1..4e79c38 100644 --- a/third_party/WebKit/Source/core/fetch/ImageResource.h +++ b/third_party/WebKit/Source/core/fetch/ImageResource.h @@ -38,7 +38,6 @@ namespace blink { class FetchRequest; class FloatSize; class ImageResourceObserver; -class Length; class MemoryCache; class ResourceClient; class ResourceFetcher; @@ -94,7 +93,7 @@ public: void addObserver(ImageResourceObserver*); void removeObserver(ImageResourceObserver*); - bool hasClientsOrObservers() const override { return Resource::hasClientsOrObservers() || !m_observers.isEmpty(); } + bool hasClientsOrObservers() const override { return Resource::hasClientsOrObservers() || !m_observers.isEmpty() || !m_finishedObservers.isEmpty(); } ResourcePriority priorityFromObservers() override; @@ -158,7 +157,8 @@ private: void updateImage(bool allDataReceived); void clearImage(); // If not null, changeRect is the changed part of the image. - void notifyObservers(const IntRect* changeRect = nullptr); + void notifyObservers(bool isNotifyingFinish, const IntRect* changeRect = nullptr); + void notifyObserver(ImageResourceObserver*, bool isNotifyingFinish, const IntRect* changeRect = nullptr); float m_devicePixelRatioHeaderValue; @@ -167,6 +167,7 @@ private: MultipartParsingState m_multipartParsingState = MultipartParsingState::WaitingForFirstPart; bool m_hasDevicePixelRatioHeaderValue; HashCountedSet<ImageResourceObserver*> m_observers; + HashCountedSet<ImageResourceObserver*> m_finishedObservers; }; DEFINE_RESOURCE_TYPE_CASTS(Image); diff --git a/third_party/WebKit/Source/core/fetch/ImageResourceObserver.h b/third_party/WebKit/Source/core/fetch/ImageResourceObserver.h index 9a002c4..f9a31a5 100644 --- a/third_party/WebKit/Source/core/fetch/ImageResourceObserver.h +++ b/third_party/WebKit/Source/core/fetch/ImageResourceObserver.h @@ -41,6 +41,11 @@ public: // because we are animating. If not null, the IntRect is the changed rect of the image. virtual void imageChanged(ImageResource*, const IntRect* = 0) { } + // Similar to ResourceClient::notifyFinished(), except for a slight call + // order difference. This is to avoid an ImageResourceObserver from being + // also a ResourceClient just to be notified for load finish. + virtual void imageNotifyFinished(ImageResource*) { } + // Called to find out if this client wants to actually display the image. Used to tell when we // can halt animation. Content nodes that hold image refs for example would not render the image, // but LayoutImages would (assuming they have visibility: visible and their layout tree isn't hidden diff --git a/third_party/WebKit/Source/core/fetch/MockResourceClients.cpp b/third_party/WebKit/Source/core/fetch/MockResourceClients.cpp index 1894dab..f735e58 100644 --- a/third_party/WebKit/Source/core/fetch/MockResourceClients.cpp +++ b/third_party/WebKit/Source/core/fetch/MockResourceClients.cpp @@ -36,6 +36,7 @@ void MockResourceClient::removeAsClient() MockImageResourceClient::MockImageResourceClient(PassRefPtrWillBeRawPtr<ImageResource> resource) : MockResourceClient(resource) , m_imageChangedCount(0) + , m_imageNotifyFinishedCount(0) { toImageResource(m_resource.get())->addObserver(this); } @@ -57,4 +58,17 @@ void MockImageResourceClient::imageChanged(ImageResource*, const IntRect*) m_imageChangedCount++; } +void MockImageResourceClient::imageNotifyFinished(ImageResource*) +{ + ASSERT_EQ(0, m_imageNotifyFinishedCount); + m_imageNotifyFinishedCount++; +} + +bool MockImageResourceClient::notifyFinishedCalled() const +{ + EXPECT_EQ(m_notifyFinishedCalled ? 1 : 0, m_imageNotifyFinishedCount); + + return m_notifyFinishedCalled; +} + } // namespace blink diff --git a/third_party/WebKit/Source/core/fetch/MockResourceClients.h b/third_party/WebKit/Source/core/fetch/MockResourceClients.h index 6fa5351..d71b21f 100644 --- a/third_party/WebKit/Source/core/fetch/MockResourceClients.h +++ b/third_party/WebKit/Source/core/fetch/MockResourceClients.h @@ -45,7 +45,7 @@ public: void notifyFinished(Resource*) override; String debugName() const override { return "MockResourceClient"; } - bool notifyFinishedCalled() const { return m_notifyFinishedCalled; } + virtual bool notifyFinishedCalled() const { return m_notifyFinishedCalled; } virtual void removeAsClient(); @@ -60,16 +60,20 @@ public: explicit MockImageResourceClient(const PassRefPtrWillBeRawPtr<ImageResource>); ~MockImageResourceClient() override; + void imageNotifyFinished(ImageResource*) override; void imageChanged(ImageResource*, const IntRect*) override; String debugName() const override { return "MockImageResourceClient"; } + bool notifyFinishedCalled() const override; + void removeAsClient() override; int imageChangedCount() const { return m_imageChangedCount; } private: int m_imageChangedCount; + int m_imageNotifyFinishedCount; }; } // namespace blink diff --git a/third_party/WebKit/Source/core/layout/LayoutImage.cpp b/third_party/WebKit/Source/core/layout/LayoutImage.cpp index bb23422..5d07c7f 100644 --- a/third_party/WebKit/Source/core/layout/LayoutImage.cpp +++ b/third_party/WebKit/Source/core/layout/LayoutImage.cpp @@ -89,7 +89,7 @@ void LayoutImage::setImageResource(PassOwnPtrWillBeRawPtr<LayoutImageResource> i { ASSERT(!m_imageResource); m_imageResource = imageResource; - m_imageResource->initialize(this, this); + m_imageResource->initialize(this); } void LayoutImage::imageChanged(WrappedImagePtr newImage, const IntRect* rect) @@ -175,7 +175,7 @@ void LayoutImage::invalidatePaintAndMarkForLayoutIfNeeded() contentChanged(ImageChanged); } -void LayoutImage::notifyFinished(Resource* newImage) +void LayoutImage::imageNotifyFinished(ImageResource* newImage) { if (!m_imageResource) return; diff --git a/third_party/WebKit/Source/core/layout/LayoutImage.h b/third_party/WebKit/Source/core/layout/LayoutImage.h index c81ed4e..2824bcf 100644 --- a/third_party/WebKit/Source/core/layout/LayoutImage.h +++ b/third_party/WebKit/Source/core/layout/LayoutImage.h @@ -44,7 +44,7 @@ class HTMLMapElement; // // The class is image type agnostic as it only manipulates decoded images. // See LayoutImageResource that holds this image. -class CORE_EXPORT LayoutImage : public LayoutReplaced, public ResourceClient { +class CORE_EXPORT LayoutImage : public LayoutReplaced { public: // These are the paddings to use when displaying either alt text or an image. static const unsigned short paddingWidth = 4; @@ -78,7 +78,6 @@ public: } const char* name() const override { return "LayoutImage"; } - String debugName() const final { return LayoutObject::debugName(); } protected: bool needsPreferredWidthsRecalculation() const final; @@ -107,7 +106,7 @@ private: LayoutUnit minimumReplacedHeight() const override; - void notifyFinished(Resource*) final; + void imageNotifyFinished(ImageResource*) final; bool nodeAtPoint(HitTestResult&, const HitTestLocation& locationInContainer, const LayoutPoint& accumulatedOffset, HitTestAction) final; bool boxShadowShouldBeAppliedToBackground(BackgroundBleedAvoidance, const InlineFlowBox*) const final; diff --git a/third_party/WebKit/Source/core/layout/LayoutImageResource.cpp b/third_party/WebKit/Source/core/layout/LayoutImageResource.cpp index ed4aab3..9ce3365 100644 --- a/third_party/WebKit/Source/core/layout/LayoutImageResource.cpp +++ b/third_party/WebKit/Source/core/layout/LayoutImageResource.cpp @@ -36,7 +36,6 @@ namespace blink { LayoutImageResource::LayoutImageResource() : m_layoutObject(nullptr) , m_cachedImage(nullptr) - , m_client(nullptr) { } @@ -44,12 +43,11 @@ LayoutImageResource::~LayoutImageResource() { } -void LayoutImageResource::initialize(LayoutObject* layoutObject, ResourceClient* client) +void LayoutImageResource::initialize(LayoutObject* layoutObject) { ASSERT(!m_layoutObject); ASSERT(layoutObject); m_layoutObject = layoutObject; - m_client = client; } void LayoutImageResource::shutdown() @@ -58,8 +56,6 @@ void LayoutImageResource::shutdown() if (!m_cachedImage) return; - if (m_client) - m_cachedImage->removeClient(m_client); m_cachedImage->removeObserver(m_layoutObject); } @@ -71,14 +67,10 @@ void LayoutImageResource::setImageResource(ImageResource* newImage) return; if (m_cachedImage) { - if (m_client) - m_cachedImage->removeClient(m_client); m_cachedImage->removeObserver(m_layoutObject); } m_cachedImage = newImage; if (m_cachedImage) { - if (m_client) - m_cachedImage->addClient(m_client); m_cachedImage->addObserver(m_layoutObject); if (m_cachedImage->errorOccurred()) m_layoutObject->imageChanged(m_cachedImage.get()); diff --git a/third_party/WebKit/Source/core/layout/LayoutImageResource.h b/third_party/WebKit/Source/core/layout/LayoutImageResource.h index 9b87daf..9680c5a 100644 --- a/third_party/WebKit/Source/core/layout/LayoutImageResource.h +++ b/third_party/WebKit/Source/core/layout/LayoutImageResource.h @@ -44,7 +44,7 @@ public: return adoptPtrWillBeNoop(new LayoutImageResource); } - virtual void initialize(LayoutObject*, ResourceClient*); + virtual void initialize(LayoutObject*); virtual void shutdown(); void setImageResource(ImageResource*); @@ -69,7 +69,6 @@ protected: LayoutImageResource(); LayoutObject* m_layoutObject; RefPtrWillBeMember<ImageResource> m_cachedImage; - ResourceClient* m_client; }; } // namespace blink diff --git a/third_party/WebKit/Source/core/layout/LayoutImageResourceStyleImage.cpp b/third_party/WebKit/Source/core/layout/LayoutImageResourceStyleImage.cpp index 017015c..390a214 100644 --- a/third_party/WebKit/Source/core/layout/LayoutImageResourceStyleImage.cpp +++ b/third_party/WebKit/Source/core/layout/LayoutImageResourceStyleImage.cpp @@ -44,9 +44,9 @@ LayoutImageResourceStyleImage::~LayoutImageResourceStyleImage() ASSERT(!m_cachedImage); } -void LayoutImageResourceStyleImage::initialize(LayoutObject* layoutObject, ResourceClient* client) +void LayoutImageResourceStyleImage::initialize(LayoutObject* layoutObject) { - LayoutImageResource::initialize(layoutObject, client); + LayoutImageResource::initialize(layoutObject); if (m_styleImage->isImageResource()) m_cachedImage = toStyleFetchedImage(m_styleImage)->cachedImage(); diff --git a/third_party/WebKit/Source/core/layout/LayoutImageResourceStyleImage.h b/third_party/WebKit/Source/core/layout/LayoutImageResourceStyleImage.h index 9ef29b1..0cc4051 100644 --- a/third_party/WebKit/Source/core/layout/LayoutImageResourceStyleImage.h +++ b/third_party/WebKit/Source/core/layout/LayoutImageResourceStyleImage.h @@ -42,7 +42,7 @@ public: { return adoptPtrWillBeNoop(new LayoutImageResourceStyleImage(styleImage)); } - void initialize(LayoutObject*, ResourceClient*) override; + void initialize(LayoutObject*) override; void shutdown() override; bool hasImage() const override { return true; } diff --git a/third_party/WebKit/Source/core/layout/LayoutObject.h b/third_party/WebKit/Source/core/layout/LayoutObject.h index 0e7df46..e331b41 100644 --- a/third_party/WebKit/Source/core/layout/LayoutObject.h +++ b/third_party/WebKit/Source/core/layout/LayoutObject.h @@ -233,7 +233,7 @@ public: // DisplayItemClient methods. LayoutRect visualRect() const override; - String debugName() const override; + String debugName() const final; LayoutObject* parent() const { return m_parent; } bool isDescendantOf(const LayoutObject*) const; diff --git a/third_party/WebKit/Source/core/layout/svg/LayoutSVGImage.cpp b/third_party/WebKit/Source/core/layout/svg/LayoutSVGImage.cpp index 5a8bf3a..ccb9c17 100644 --- a/third_party/WebKit/Source/core/layout/svg/LayoutSVGImage.cpp +++ b/third_party/WebKit/Source/core/layout/svg/LayoutSVGImage.cpp @@ -47,7 +47,7 @@ LayoutSVGImage::LayoutSVGImage(SVGImageElement* impl) , m_needsTransformUpdate(true) , m_imageResource(LayoutImageResource::create()) { - m_imageResource->initialize(this, nullptr); + m_imageResource->initialize(this); } LayoutSVGImage::~LayoutSVGImage() |