- Revision
- 145423
- Author
- schen...@chromium.org
- Date
- 2013-03-11 16:18:44 -0700 (Mon, 11 Mar 2013)
Log Message
HTMLInputElement can delete an ImageLoader while it's still needed
https://bugs.webkit.org/show_bug.cgi?id=110621
Reviewed by Darin Adler.
Source/WebCore:
ImageLoader objects may fire events for HTMLInputElements that are of
type ImageInputType that own the loader. These events may cause script
to run that changes the type of the input element and hence causes the
ImageLoader to be deleted, while the image loader is still processing
the event dispatch. Bad things ensue.
This change moves ownership of the ImageLoader from the ImageInputType
onto the HTMLImageElement which is already protected from deletion during
event processing.
Test: fast/forms/image/image-error-event-modifies-type-crash.html
* html/HTMLInputElement.cpp:
(WebCore::HTMLInputElement::imageLoader): Method to return the
ImageLoader, creating it if not already created.
* html/HTMLInputElement.h:
(WebCore::HTMLInputElement::hasImageLoader): Return true if the
ImageLoader has been created.
(HTMLInputElement): Define ImageLoader access methods and the OwnPtr
for the HTMLImageLoader.
* html/ImageInputType.cpp:
(WebCore::ImageInputType::srcAttributeChanged): Use the element's ImageLoader.
(WebCore::ImageInputType::attach): Use the element's ImageLoader.
(WebCore::ImageInputType::willMoveToNewOwnerDocument): Use the element's ImageLoader.
(WebCore::ImageInputType::height): Use the element's ImageLoader.
(WebCore::ImageInputType::width): Use the element's ImageLoader.
* html/ImageInputType.h:
(ImageInputType): Remove the declaration of the ImageLoader.
LayoutTests:
* fast/forms/image/image-error-event-modifies-type-crash-expected.txt: Added.
* fast/forms/image/image-error-event-modifies-type-crash.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (145422 => 145423)
--- trunk/LayoutTests/ChangeLog 2013-03-11 22:59:58 UTC (rev 145422)
+++ trunk/LayoutTests/ChangeLog 2013-03-11 23:18:44 UTC (rev 145423)
@@ -1,3 +1,13 @@
+2013-03-11 Stephen Chenney <schen...@chromium.org>
+
+ HTMLInputElement can delete an ImageLoader while it's still needed
+ https://bugs.webkit.org/show_bug.cgi?id=110621
+
+ Reviewed by Darin Adler.
+
+ * fast/forms/image/image-error-event-modifies-type-crash-expected.txt: Added.
+ * fast/forms/image/image-error-event-modifies-type-crash.html: Added.
+
2013-03-11 Alok Priyadarshi <al...@chromium.org>
Revert "Mark GraphicsLayers as opaque when possible"
Added: trunk/LayoutTests/fast/forms/image/image-error-event-modifies-type-crash-expected.txt (0 => 145423)
--- trunk/LayoutTests/fast/forms/image/image-error-event-modifies-type-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/forms/image/image-error-event-modifies-type-crash-expected.txt 2013-03-11 23:18:44 UTC (rev 145423)
@@ -0,0 +1 @@
+ Test Passes if it does not crash.
Added: trunk/LayoutTests/fast/forms/image/image-error-event-modifies-type-crash.html (0 => 145423)
--- trunk/LayoutTests/fast/forms/image/image-error-event-modifies-type-crash.html (rev 0)
+++ trunk/LayoutTests/fast/forms/image/image-error-event-modifies-type-crash.html 2013-03-11 23:18:44 UTC (rev 145423)
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html>
+ <body>
+ <script>
+ if (window.testRunner) {
+ testRunner.waitUntilDone();
+ testRunner.dumpAsText();
+ }
+ function eventhandler() {
+ inputNode = document.getElementById("imageInput");
+ inputNode.type = "";
+ setTimeout("cleanup()", 10);
+ }
+
+ function cleanup() {
+ if (window.testRunner)
+ {
+ testRunner.notifyDone();
+ }
+ }
+ </script>
+ <input>
+ <input id="imageInput" type="image" _onerror_="eventhandler()" src="" />
+ </input>
+ Test Passes if it does not crash.
+ </body>
+</html>
\ No newline at end of file
Modified: trunk/Source/WebCore/ChangeLog (145422 => 145423)
--- trunk/Source/WebCore/ChangeLog 2013-03-11 22:59:58 UTC (rev 145422)
+++ trunk/Source/WebCore/ChangeLog 2013-03-11 23:18:44 UTC (rev 145423)
@@ -1,3 +1,39 @@
+2013-03-11 Stephen Chenney <schen...@chromium.org>
+
+ HTMLInputElement can delete an ImageLoader while it's still needed
+ https://bugs.webkit.org/show_bug.cgi?id=110621
+
+ Reviewed by Darin Adler.
+
+ ImageLoader objects may fire events for HTMLInputElements that are of
+ type ImageInputType that own the loader. These events may cause script
+ to run that changes the type of the input element and hence causes the
+ ImageLoader to be deleted, while the image loader is still processing
+ the event dispatch. Bad things ensue.
+
+ This change moves ownership of the ImageLoader from the ImageInputType
+ onto the HTMLImageElement which is already protected from deletion during
+ event processing.
+
+ Test: fast/forms/image/image-error-event-modifies-type-crash.html
+
+ * html/HTMLInputElement.cpp:
+ (WebCore::HTMLInputElement::imageLoader): Method to return the
+ ImageLoader, creating it if not already created.
+ * html/HTMLInputElement.h:
+ (WebCore::HTMLInputElement::hasImageLoader): Return true if the
+ ImageLoader has been created.
+ (HTMLInputElement): Define ImageLoader access methods and the OwnPtr
+ for the HTMLImageLoader.
+ * html/ImageInputType.cpp:
+ (WebCore::ImageInputType::srcAttributeChanged): Use the element's ImageLoader.
+ (WebCore::ImageInputType::attach): Use the element's ImageLoader.
+ (WebCore::ImageInputType::willMoveToNewOwnerDocument): Use the element's ImageLoader.
+ (WebCore::ImageInputType::height): Use the element's ImageLoader.
+ (WebCore::ImageInputType::width): Use the element's ImageLoader.
+ * html/ImageInputType.h:
+ (ImageInputType): Remove the declaration of the ImageLoader.
+
2013-03-11 Alok Priyadarshi <al...@chromium.org>
Revert "Mark GraphicsLayers as opaque when possible"
Modified: trunk/Source/WebCore/html/HTMLInputElement.cpp (145422 => 145423)
--- trunk/Source/WebCore/html/HTMLInputElement.cpp 2013-03-11 22:59:58 UTC (rev 145422)
+++ trunk/Source/WebCore/html/HTMLInputElement.cpp 2013-03-11 23:18:44 UTC (rev 145423)
@@ -47,6 +47,7 @@
#include "HTMLCollection.h"
#include "HTMLDataListElement.h"
#include "HTMLFormElement.h"
+#include "HTMLImageLoader.h"
#include "HTMLNames.h"
#include "HTMLOptionElement.h"
#include "HTMLParserIdioms.h"
@@ -147,6 +148,13 @@
return inputElement.release();
}
+HTMLImageLoader* HTMLInputElement::imageLoader()
+{
+ if (!m_imageLoader)
+ m_imageLoader = adoptPtr(new HTMLImageLoader(this));
+ return m_imageLoader.get();
+}
+
void HTMLInputElement::didAddUserAgentShadowRoot(ShadowRoot*)
{
m_inputType->createShadowSubtree();
@@ -1513,7 +1521,9 @@
void HTMLInputElement::didMoveToNewDocument(Document* oldDocument)
{
- m_inputType->willMoveToNewOwnerDocument();
+ if (hasImageLoader())
+ imageLoader()->elementDidMoveToNewDocument();
+
bool needsSuspensionCallback = this->needsSuspensionCallback();
if (oldDocument) {
// Always unregister for cache callbacks when leaving a document, even if we would otherwise like to be registered
Modified: trunk/Source/WebCore/html/HTMLInputElement.h (145422 => 145423)
--- trunk/Source/WebCore/html/HTMLInputElement.h 2013-03-11 22:59:58 UTC (rev 145422)
+++ trunk/Source/WebCore/html/HTMLInputElement.h 2013-03-11 23:18:44 UTC (rev 145423)
@@ -35,6 +35,7 @@
class DragData;
class FileList;
class HTMLDataListElement;
+class HTMLImageLoader;
class HTMLOptionElement;
class Icon;
class InputType;
@@ -294,6 +295,9 @@
virtual void setRangeText(const String& replacement, ExceptionCode&) OVERRIDE;
virtual void setRangeText(const String& replacement, unsigned start, unsigned end, const String& selectionMode, ExceptionCode&) OVERRIDE;
+ bool hasImageLoader() const { return m_imageLoader; }
+ HTMLImageLoader* imageLoader();
+
#if ENABLE(DATE_AND_TIME_INPUT_TYPES)
bool setupDateTimeChooserParameters(DateTimeChooserParameters&);
#endif
@@ -430,6 +434,10 @@
bool m_hasTouchEventHandler : 1;
#endif
OwnPtr<InputType> m_inputType;
+ // The ImageLoader must be owned by this element because the loader code assumes
+ // that it lives as long as its owning element lives. If we move the loader into
+ // the ImageInput object we may delete the loader while this element lives on.
+ OwnPtr<HTMLImageLoader> m_imageLoader;
#if ENABLE(DATALIST_ELEMENT)
OwnPtr<ListAttributeTargetObserver> m_listAttributeTargetObserver;
#endif
Modified: trunk/Source/WebCore/html/ImageInputType.cpp (145422 => 145423)
--- trunk/Source/WebCore/html/ImageInputType.cpp 2013-03-11 22:59:58 UTC (rev 145422)
+++ trunk/Source/WebCore/html/ImageInputType.cpp 2013-03-11 23:18:44 UTC (rev 145423)
@@ -120,42 +120,32 @@
{
if (!element()->renderer())
return;
- if (!m_imageLoader)
- m_imageLoader = adoptPtr(new HTMLImageLoader(element()));
- m_imageLoader->updateFromElementIgnoringPreviousError();
+ element()->imageLoader()->updateFromElementIgnoringPreviousError();
}
void ImageInputType::attach()
{
BaseButtonInputType::attach();
- if (!m_imageLoader)
- m_imageLoader = adoptPtr(new HTMLImageLoader(element()));
- m_imageLoader->updateFromElement();
+ HTMLImageLoader* imageLoader = element()->imageLoader();
+ imageLoader->updateFromElement();
RenderImage* renderer = toRenderImage(element()->renderer());
if (!renderer)
return;
- if (m_imageLoader->hasPendingBeforeLoadEvent())
+ if (imageLoader->hasPendingBeforeLoadEvent())
return;
RenderImageResource* imageResource = renderer->imageResource();
- imageResource->setCachedImage(m_imageLoader->image());
+ imageResource->setCachedImage(imageLoader->image());
// If we have no image at all because we have no src attribute, set
// image height and width for the alt text instead.
- if (!m_imageLoader->image() && !imageResource->cachedImage())
+ if (!imageLoader->image() && !imageResource->cachedImage())
renderer->setImageSizeForAltText();
}
-void ImageInputType::willMoveToNewOwnerDocument()
-{
- BaseButtonInputType::willMoveToNewOwnerDocument();
- if (m_imageLoader)
- m_imageLoader->elementDidMoveToNewDocument();
-}
-
bool ImageInputType::shouldRespectAlignAttribute()
{
return true;
@@ -192,8 +182,11 @@
return height;
// If the image is available, use its height.
- if (m_imageLoader && m_imageLoader->image())
- return m_imageLoader->image()->imageSizeForRenderer(element->renderer(), 1).height();
+ if (element->hasImageLoader()) {
+ HTMLImageLoader* imageLoader = element->imageLoader();
+ if (imageLoader->image())
+ return imageLoader->image()->imageSizeForRenderer(element->renderer(), 1).height();
+ }
}
element->document()->updateLayout();
@@ -213,8 +206,11 @@
return width;
// If the image is available, use its width.
- if (m_imageLoader && m_imageLoader->image())
- return m_imageLoader->image()->imageSizeForRenderer(element->renderer(), 1).width();
+ if (element->hasImageLoader()) {
+ HTMLImageLoader* imageLoader = element->imageLoader();
+ if (imageLoader->image())
+ return imageLoader->image()->imageSizeForRenderer(element->renderer(), 1).width();
+ }
}
element->document()->updateLayout();
Modified: trunk/Source/WebCore/html/ImageInputType.h (145422 => 145423)
--- trunk/Source/WebCore/html/ImageInputType.h 2013-03-11 22:59:58 UTC (rev 145422)
+++ trunk/Source/WebCore/html/ImageInputType.h 2013-03-11 23:18:44 UTC (rev 145423)
@@ -39,8 +39,6 @@
namespace WebCore {
-class HTMLImageLoader;
-
class ImageInputType : public BaseButtonInputType {
public:
static PassOwnPtr<InputType> create(HTMLInputElement*);
@@ -56,7 +54,6 @@
virtual void altAttributeChanged() OVERRIDE;
virtual void srcAttributeChanged() OVERRIDE;
virtual void attach() OVERRIDE;
- virtual void willMoveToNewOwnerDocument() OVERRIDE;
virtual bool shouldRespectAlignAttribute() OVERRIDE;
virtual bool canBeSuccessfulSubmitButton() OVERRIDE;
virtual bool isImageButton() const OVERRIDE;
@@ -65,7 +62,6 @@
virtual unsigned height() const OVERRIDE;
virtual unsigned width() const OVERRIDE;
- OwnPtr<HTMLImageLoader> m_imageLoader;
IntPoint m_clickLocation; // Valid only during HTMLFormElement::prepareForSubmission().
};
Modified: trunk/Source/WebCore/html/InputType.cpp (145422 => 145423)
--- trunk/Source/WebCore/html/InputType.cpp 2013-03-11 22:59:58 UTC (rev 145422)
+++ trunk/Source/WebCore/html/InputType.cpp 2013-03-11 23:18:44 UTC (rev 145423)
@@ -596,10 +596,6 @@
{
}
-void InputType::willMoveToNewOwnerDocument()
-{
-}
-
bool InputType::shouldRespectAlignAttribute()
{
return false;
Modified: trunk/Source/WebCore/html/InputType.h (145422 => 145423)
--- trunk/Source/WebCore/html/InputType.h 2013-03-11 22:59:58 UTC (rev 145422)
+++ trunk/Source/WebCore/html/InputType.h 2013-03-11 23:18:44 UTC (rev 145423)
@@ -245,7 +245,6 @@
virtual void stepAttributeChanged();
virtual void altAttributeChanged();
virtual void srcAttributeChanged();
- virtual void willMoveToNewOwnerDocument();
virtual bool shouldRespectAlignAttribute();
virtual FileList* files();
virtual void setFiles(PassRefPtr<FileList>);
