- Revision
- 112215
- Author
- [email protected]
- Date
- 2012-03-27 00:21:37 -0700 (Tue, 27 Mar 2012)
Log Message
ImageLoader::m_firedLoadEvent is a confusing name
https://bugs.webkit.org/show_bug.cgi?id=82283
Reviewed by Kentaro Hara.
This patch renames m_firedLoadEvent (and friends) to
m_hasPendingLoadEvent (and negates the value). That name more
accurately reflects the semantics of this piece of state. For example,
we now initialize m_hasPendingLoadEvent to false, which makes sense as
there is no pending load event, whereas before we initialized
m_firedLoadEvent to true, which made less sense since we hadn't yet
actually fired the load event.
* bindings/v8/V8GCController.cpp:
(WebCore::calculateGroupId):
* html/HTMLImageElement.cpp:
(WebCore::HTMLImageElement::attach):
* html/HTMLImageElement.h:
(HTMLImageElement):
(WebCore::HTMLImageElement::hasPendingLoadEvent):
(WebCore::HTMLImageElement::hasPendingActivity):
* html/ImageInputType.cpp:
(WebCore::ImageInputType::attach):
* loader/ImageLoader.cpp:
(WebCore::ImageLoader::ImageLoader):
(WebCore::ImageLoader::~ImageLoader):
(WebCore::ImageLoader::setImage):
(WebCore::ImageLoader::updateFromElement):
(WebCore::ImageLoader::notifyFinished):
(WebCore::ImageLoader::dispatchPendingBeforeLoadEvent):
(WebCore::ImageLoader::dispatchPendingLoadEvent):
(WebCore::ImageLoader::dispatchPendingErrorEvent):
* loader/ImageLoader.h:
(WebCore::ImageLoader::hasPendingBeforeLoadEvent):
(WebCore::ImageLoader::hasPendingLoadEvent):
(ImageLoader):
* svg/SVGImageElement.cpp:
(WebCore::SVGImageElement::haveLoadedRequiredResources):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (112214 => 112215)
--- trunk/Source/WebCore/ChangeLog 2012-03-27 07:17:36 UTC (rev 112214)
+++ trunk/Source/WebCore/ChangeLog 2012-03-27 07:21:37 UTC (rev 112215)
@@ -1,3 +1,44 @@
+2012-03-27 Adam Barth <[email protected]>
+
+ ImageLoader::m_firedLoadEvent is a confusing name
+ https://bugs.webkit.org/show_bug.cgi?id=82283
+
+ Reviewed by Kentaro Hara.
+
+ This patch renames m_firedLoadEvent (and friends) to
+ m_hasPendingLoadEvent (and negates the value). That name more
+ accurately reflects the semantics of this piece of state. For example,
+ we now initialize m_hasPendingLoadEvent to false, which makes sense as
+ there is no pending load event, whereas before we initialized
+ m_firedLoadEvent to true, which made less sense since we hadn't yet
+ actually fired the load event.
+
+ * bindings/v8/V8GCController.cpp:
+ (WebCore::calculateGroupId):
+ * html/HTMLImageElement.cpp:
+ (WebCore::HTMLImageElement::attach):
+ * html/HTMLImageElement.h:
+ (HTMLImageElement):
+ (WebCore::HTMLImageElement::hasPendingLoadEvent):
+ (WebCore::HTMLImageElement::hasPendingActivity):
+ * html/ImageInputType.cpp:
+ (WebCore::ImageInputType::attach):
+ * loader/ImageLoader.cpp:
+ (WebCore::ImageLoader::ImageLoader):
+ (WebCore::ImageLoader::~ImageLoader):
+ (WebCore::ImageLoader::setImage):
+ (WebCore::ImageLoader::updateFromElement):
+ (WebCore::ImageLoader::notifyFinished):
+ (WebCore::ImageLoader::dispatchPendingBeforeLoadEvent):
+ (WebCore::ImageLoader::dispatchPendingLoadEvent):
+ (WebCore::ImageLoader::dispatchPendingErrorEvent):
+ * loader/ImageLoader.h:
+ (WebCore::ImageLoader::hasPendingBeforeLoadEvent):
+ (WebCore::ImageLoader::hasPendingLoadEvent):
+ (ImageLoader):
+ * svg/SVGImageElement.cpp:
+ (WebCore::SVGImageElement::haveLoadedRequiredResources):
+
2012-03-27 Dan Bernstein <[email protected]>
Tried to fix 32-bit builds.
Modified: trunk/Source/WebCore/bindings/v8/V8GCController.cpp (112214 => 112215)
--- trunk/Source/WebCore/bindings/v8/V8GCController.cpp 2012-03-27 07:17:36 UTC (rev 112214)
+++ trunk/Source/WebCore/bindings/v8/V8GCController.cpp 2012-03-27 07:21:37 UTC (rev 112215)
@@ -291,7 +291,7 @@
// element of the tree to which it belongs.
static GroupId calculateGroupId(Node* node)
{
- if (node->inDocument() || (node->hasTagName(HTMLNames::imgTag) && !static_cast<HTMLImageElement*>(node)->haveFiredLoadEvent()))
+ if (node->inDocument() || (node->hasTagName(HTMLNames::imgTag) && static_cast<HTMLImageElement*>(node)->hasPendingLoadEvent()))
return GroupId(node->document());
Node* root = node;
Modified: trunk/Source/WebCore/html/HTMLImageElement.cpp (112214 => 112215)
--- trunk/Source/WebCore/html/HTMLImageElement.cpp 2012-03-27 07:17:36 UTC (rev 112214)
+++ trunk/Source/WebCore/html/HTMLImageElement.cpp 2012-03-27 07:21:37 UTC (rev 112215)
@@ -154,7 +154,7 @@
{
HTMLElement::attach();
- if (renderer() && renderer()->isImage() && m_imageLoader.haveFiredBeforeLoadEvent()) {
+ if (renderer() && renderer()->isImage() && !m_imageLoader.hasPendingBeforeLoadEvent()) {
RenderImage* renderImage = toRenderImage(renderer());
RenderImageResource* renderImageResource = renderImage->imageResource();
if (renderImageResource->hasImage())
Modified: trunk/Source/WebCore/html/HTMLImageElement.h (112214 => 112215)
--- trunk/Source/WebCore/html/HTMLImageElement.h 2012-03-27 07:17:36 UTC (rev 112214)
+++ trunk/Source/WebCore/html/HTMLImageElement.h 2012-03-27 07:21:37 UTC (rev 112215)
@@ -72,8 +72,9 @@
bool complete() const;
- bool haveFiredLoadEvent() const { return m_imageLoader.haveFiredLoadEvent(); }
- bool hasPendingActivity() const { return !m_imageLoader.haveFiredLoadEvent(); }
+ // FIXME: Why do we have two names for the same thing?
+ bool hasPendingLoadEvent() const { return m_imageLoader.hasPendingLoadEvent(); }
+ bool hasPendingActivity() const { return m_imageLoader.hasPendingLoadEvent(); }
virtual bool canContainRangeEndPoint() const { return false; }
Modified: trunk/Source/WebCore/html/ImageInputType.cpp (112214 => 112215)
--- trunk/Source/WebCore/html/ImageInputType.cpp 2012-03-27 07:17:36 UTC (rev 112214)
+++ trunk/Source/WebCore/html/ImageInputType.cpp 2012-03-27 07:21:37 UTC (rev 112215)
@@ -130,7 +130,7 @@
if (!renderer)
return;
- if (!m_imageLoader->haveFiredBeforeLoadEvent())
+ if (m_imageLoader->hasPendingBeforeLoadEvent())
return;
RenderImageResource* imageResource = renderer->imageResource();
Modified: trunk/Source/WebCore/loader/ImageLoader.cpp (112214 => 112215)
--- trunk/Source/WebCore/loader/ImageLoader.cpp 2012-03-27 07:17:36 UTC (rev 112214)
+++ trunk/Source/WebCore/loader/ImageLoader.cpp 2012-03-27 07:21:37 UTC (rev 112215)
@@ -84,9 +84,9 @@
ImageLoader::ImageLoader(Element* element)
: m_element(element)
, m_image(0)
- , m_firedBeforeLoad(true)
- , m_firedLoad(true)
- , m_firedError(true)
+ , m_hasPendingBeforeLoadEvent(false)
+ , m_hasPendingLoadEvent(false)
+ , m_hasPendingErrorEvent(false)
, m_imageComplete(true)
, m_loadManually(false)
{
@@ -97,16 +97,16 @@
if (m_image)
m_image->removeClient(this);
- ASSERT(!m_firedBeforeLoad || !beforeLoadEventSender().hasPendingEvents(this));
- if (!m_firedBeforeLoad)
+ ASSERT(m_hasPendingBeforeLoadEvent || !beforeLoadEventSender().hasPendingEvents(this));
+ if (m_hasPendingBeforeLoadEvent)
beforeLoadEventSender().cancelEvent(this);
- ASSERT(!m_firedLoad || !loadEventSender().hasPendingEvents(this));
- if (!m_firedLoad)
+ ASSERT(m_hasPendingLoadEvent || !loadEventSender().hasPendingEvents(this));
+ if (m_hasPendingLoadEvent)
loadEventSender().cancelEvent(this);
- ASSERT(!m_firedError || !errorEventSender().hasPendingEvents(this));
- if (!m_firedError)
+ ASSERT(m_hasPendingErrorEvent || !errorEventSender().hasPendingEvents(this));
+ if (m_hasPendingErrorEvent)
errorEventSender().cancelEvent(this);
}
@@ -116,17 +116,17 @@
CachedImage* oldImage = m_image.get();
if (newImage != oldImage) {
m_image = newImage;
- if (!m_firedBeforeLoad) {
+ if (m_hasPendingBeforeLoadEvent) {
beforeLoadEventSender().cancelEvent(this);
- m_firedBeforeLoad = true;
+ m_hasPendingBeforeLoadEvent = false;
}
- if (!m_firedLoad) {
+ if (m_hasPendingLoadEvent) {
loadEventSender().cancelEvent(this);
- m_firedLoad = true;
+ m_hasPendingLoadEvent = false;
}
- if (!m_firedError) {
+ if (m_hasPendingErrorEvent) {
errorEventSender().cancelEvent(this);
- m_firedError = true;
+ m_hasPendingErrorEvent = false;
}
m_imageComplete = true;
if (newImage)
@@ -186,16 +186,16 @@
CachedImage* oldImage = m_image.get();
if (newImage != oldImage) {
- if (!m_firedBeforeLoad)
+ if (m_hasPendingBeforeLoadEvent)
beforeLoadEventSender().cancelEvent(this);
- if (!m_firedLoad)
+ if (m_hasPendingLoadEvent)
loadEventSender().cancelEvent(this);
- if (!m_firedError)
+ if (m_hasPendingErrorEvent)
errorEventSender().cancelEvent(this);
m_image = newImage;
- m_firedBeforeLoad = !newImage;
- m_firedLoad = !newImage;
+ m_hasPendingBeforeLoadEvent = newImage;
+ m_hasPendingLoadEvent = newImage;
m_imageComplete = !newImage;
if (newImage) {
@@ -230,10 +230,10 @@
ASSERT(resource == m_image.get());
m_imageComplete = true;
- if (haveFiredBeforeLoadEvent())
+ if (!hasPendingBeforeLoadEvent())
updateRenderer();
- if (m_firedLoad)
+ if (!m_hasPendingLoadEvent)
return;
if (m_element->fastHasAttribute(HTMLNames::crossoriginAttr)
@@ -242,18 +242,18 @@
setImage(0);
- m_firedError = false;
+ m_hasPendingErrorEvent = true;
errorEventSender().dispatchEventSoon(this);
DEFINE_STATIC_LOCAL(String, consoleMessage, ("Cross-origin image load denied by Cross-Origin Resource Sharing policy."));
m_element->document()->addConsoleMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, consoleMessage);
- ASSERT(m_firedLoad);
+ ASSERT(!m_hasPendingLoadEvent);
return;
}
if (resource->wasCanceled()) {
- m_firedLoad = true;
+ m_hasPendingLoadEvent = false;
return;
}
@@ -314,13 +314,13 @@
void ImageLoader::dispatchPendingBeforeLoadEvent()
{
- if (m_firedBeforeLoad)
+ if (!m_hasPendingBeforeLoadEvent)
return;
if (!m_image)
return;
if (!m_element->document()->attached())
return;
- m_firedBeforeLoad = true;
+ m_hasPendingBeforeLoadEvent = false;
if (m_element->dispatchBeforeLoadEvent(m_image->url())) {
updateRenderer();
return;
@@ -331,7 +331,7 @@
}
loadEventSender().cancelEvent(this);
- m_firedLoad = true;
+ m_hasPendingLoadEvent = false;
if (m_element->hasTagName(HTMLNames::objectTag))
static_cast<HTMLObjectElement*>(m_element)->renderFallbackContent();
@@ -339,23 +339,23 @@
void ImageLoader::dispatchPendingLoadEvent()
{
- if (m_firedLoad)
+ if (!m_hasPendingLoadEvent)
return;
if (!m_image)
return;
if (!m_element->document()->attached())
return;
- m_firedLoad = true;
+ m_hasPendingLoadEvent = false;
dispatchLoadEvent();
}
void ImageLoader::dispatchPendingErrorEvent()
{
- if (m_firedError)
+ if (!m_hasPendingErrorEvent)
return;
if (!m_element->document()->attached())
return;
- m_firedError = true;
+ m_hasPendingErrorEvent = false;
m_element->dispatchEvent(Event::create(eventNames().errorEvent, false, false));
}
Modified: trunk/Source/WebCore/loader/ImageLoader.h (112214 => 112215)
--- trunk/Source/WebCore/loader/ImageLoader.h 2012-03-27 07:17:36 UTC (rev 112214)
+++ trunk/Source/WebCore/loader/ImageLoader.h 2012-03-27 07:21:37 UTC (rev 112215)
@@ -59,8 +59,8 @@
void setLoadManually(bool loadManually) { m_loadManually = loadManually; }
- bool haveFiredBeforeLoadEvent() const { return m_firedBeforeLoad; }
- bool haveFiredLoadEvent() const { return m_firedLoad; }
+ bool hasPendingBeforeLoadEvent() const { return m_hasPendingBeforeLoadEvent; }
+ bool hasPendingLoadEvent() const { return m_hasPendingLoadEvent; }
void dispatchPendingEvent(ImageEventSender*);
@@ -85,9 +85,9 @@
Element* m_element;
CachedResourceHandle<CachedImage> m_image;
AtomicString m_failedLoadURL;
- bool m_firedBeforeLoad : 1;
- bool m_firedLoad : 1;
- bool m_firedError : 1;
+ bool m_hasPendingBeforeLoadEvent : 1;
+ bool m_hasPendingLoadEvent : 1;
+ bool m_hasPendingErrorEvent : 1;
bool m_imageComplete : 1;
bool m_loadManually : 1;
};
Modified: trunk/Source/WebCore/svg/SVGImageElement.cpp (112214 => 112215)
--- trunk/Source/WebCore/svg/SVGImageElement.cpp 2012-03-27 07:17:36 UTC (rev 112214)
+++ trunk/Source/WebCore/svg/SVGImageElement.cpp 2012-03-27 07:21:37 UTC (rev 112215)
@@ -194,7 +194,7 @@
bool SVGImageElement::haveLoadedRequiredResources()
{
- return !externalResourcesRequiredBaseValue() || m_imageLoader.haveFiredLoadEvent();
+ return !externalResourcesRequiredBaseValue() || !m_imageLoader.hasPendingLoadEvent();
}
void SVGImageElement::attach()
