summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorhiroshige <hiroshige@chromium.org>2016-03-25 20:55:01 -0700
committerCommit bot <commit-bot@chromium.org>2016-03-26 03:56:25 +0000
commitba00e4a1b03fa6ca3f2498397c1e6f1628027b5d (patch)
treef44fb2e190c97aafb4b19f3f63e5435f460b989b
parent66a21dd7fb6eba1be3b1cab587eb510eb5fd5f2a (diff)
downloadchromium_src-ba00e4a1b03fa6ca3f2498397c1e6f1628027b5d.zip
chromium_src-ba00e4a1b03fa6ca3f2498397c1e6f1628027b5d.tar.gz
chromium_src-ba00e4a1b03fa6ca3f2498397c1e6f1628027b5d.tar.bz2
Split ImageResourceClient into ResourceClient and ImageResourceObserver [2/2]
This CL makes LayoutImage to be no longer ResourceClient by adding imageNotifyFinished() to ImageResourceObserver, which is called around when ResourceClient::notifyFinished() would be called. This CL adds |ImageResource::m_finishedObservers| in order to avoid imageNotifyFinished() from being called multiple times. BUG=587663 Review URL: https://codereview.chromium.org/1728313003 Cr-Commit-Position: refs/heads/master@{#383441}
-rw-r--r--third_party/WebKit/Source/core/fetch/ImageResource.cpp98
-rw-r--r--third_party/WebKit/Source/core/fetch/ImageResource.h7
-rw-r--r--third_party/WebKit/Source/core/fetch/ImageResourceObserver.h5
-rw-r--r--third_party/WebKit/Source/core/fetch/MockResourceClients.cpp14
-rw-r--r--third_party/WebKit/Source/core/fetch/MockResourceClients.h6
-rw-r--r--third_party/WebKit/Source/core/layout/LayoutImage.cpp4
-rw-r--r--third_party/WebKit/Source/core/layout/LayoutImage.h5
-rw-r--r--third_party/WebKit/Source/core/layout/LayoutImageResource.cpp10
-rw-r--r--third_party/WebKit/Source/core/layout/LayoutImageResource.h3
-rw-r--r--third_party/WebKit/Source/core/layout/LayoutImageResourceStyleImage.cpp4
-rw-r--r--third_party/WebKit/Source/core/layout/LayoutImageResourceStyleImage.h2
-rw-r--r--third_party/WebKit/Source/core/layout/LayoutObject.h2
-rw-r--r--third_party/WebKit/Source/core/layout/svg/LayoutSVGImage.cpp2
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()