Diff
Modified: trunk/Source/WebCore/ChangeLog (225877 => 225878)
--- trunk/Source/WebCore/ChangeLog 2017-12-13 23:58:37 UTC (rev 225877)
+++ trunk/Source/WebCore/ChangeLog 2017-12-14 00:05:42 UTC (rev 225878)
@@ -1,3 +1,56 @@
+2017-12-13 Ryosuke Niwa <[email protected]>
+
+ Crash inside ImageLoader::updateFromElement()
+ https://bugs.webkit.org/show_bug.cgi?id=180769
+ <rdar://problem/35278782>
+
+ Reviewed by Antti Koivisto.
+
+ Fixed the crash by moving all call sites of ImageLoader::updateFromElement() to be post insertion callbacks
+ where it's safe to execute arbitrary scripts.
+
+ No new test since existing tests cover this with a newly added release assert in ImageLoader.
+
+ * html/HTMLImageElement.cpp:
+ (WebCore::HTMLImageElement::insertedIntoAncestor):
+ (WebCore::HTMLImageElement::didFinishInsertingNode): Extracted from insertedIntoAncestor to call
+ selectImageSource or updateFromElement.
+ * html/HTMLImageElement.h: Made many member functions final.
+ * html/HTMLInputElement.cpp:
+ (WebCore::HTMLInputElement::didAttachRenderers): Delay the call to ImageLoader::updateFromElement() in
+ ImageInputType using a post style resolution callback.
+ * html/HTMLMetaElement.h:
+ * html/HTMLPictureElement.cpp:
+ (WebCore::HTMLPictureElement::sourcesChanged): Store the list of child image elements into a vector before
+ calling selectImageSource since each call may execute arbitrary scripts.
+ * html/HTMLSourceElement.cpp:
+ (WebCore::HTMLSourceElement::insertedIntoAncestor): Delay the call to ImageLoader::updateFromElement()
+ using a post style resolution callback.
+ (WebCore::HTMLSourceElement::didFinishInsertingNode): Extracted from insertedIntoAncestor.
+ * html/HTMLSourceElement.h:
+ * html/HTMLVideoElement.cpp:
+ (WebCore::HTMLVideoElement::didAttachRenderers):
+ (WebCore::HTMLVideoElement::updateAfterStyleResolution): Extracted from didAttachRenderers.
+ * html/HTMLVideoElement.h:
+ * html/ImageInputType.cpp:
+ (WebCore::ImageInputType::needsPostStyleResolutionCallback): Added. Returns true so that HTMLInputElement's
+ didAttachRenderers would register a post style resolution callback.
+ (WebCore::ImageInputType::updateAfterStyleResolution): Extracted from attach.
+ (WebCore::ImageInputType::attach): Deleted.
+ * html/ImageInputType.h:
+ * html/InputType.cpp:
+ (WebCore::InputType::needsPostStyleResolutionCallback): Added. All but ImageInputType returns false.
+ (WebCore::InputType::updateAfterStyleResolution): Added.
+ (WebCore::InputType::attach): Deleted.
+ * html/InputType.h:
+ * loader/ImageLoader.cpp:
+ (WebCore::ImageLoader::updateFromElement): Added a release assertion. There is no direct security implication
+ so there is no need to use RELEASE_ASSERT_WITH_SECURITY_IMPLICATION here.
+ * svg/SVGImageElement.cpp:
+ (WebCore::SVGImageElement::insertedIntoAncestor):
+ (WebCore::SVGImageElement::didFinishInsertingNode):
+ * svg/SVGImageElement.h:
+
2017-12-13 Zalan Bujtas <[email protected]>
RenderImage can be destroyed even before setting the style on it.
Modified: trunk/Source/WebCore/html/HTMLImageElement.cpp (225877 => 225878)
--- trunk/Source/WebCore/html/HTMLImageElement.cpp 2017-12-13 23:58:37 UTC (rev 225877)
+++ trunk/Source/WebCore/html/HTMLImageElement.cpp 2017-12-14 00:05:42 UTC (rev 225878)
@@ -295,6 +295,8 @@
Node::InsertedIntoAncestorResult HTMLImageElement::insertedIntoAncestor(InsertionType insertionType, ContainerNode& parentOfInsertedTree)
{
+ HTMLElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
+
if (m_formSetByParser) {
m_form = m_formSetByParser;
m_formSetByParser = nullptr;
@@ -311,9 +313,6 @@
if (m_form)
m_form->registerImgElement(this);
}
- // Insert needs to complete first, before we start updating the loader. Loader dispatches events which could result
- // in callbacks back to this node.
- Node::InsertedIntoAncestorResult insertNotificationRequest = HTMLElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
if (insertionType.connectedToDocument && !m_parsedUsemap.isNull())
document().addImageElementByUsemap(*m_parsedUsemap.impl(), *this);
@@ -320,17 +319,25 @@
if (is<HTMLPictureElement>(parentNode())) {
setPictureElement(&downcast<HTMLPictureElement>(*parentNode()));
- selectImageSource();
+ return InsertedIntoAncestorResult::NeedsPostInsertionCallback;
}
// If we have been inserted from a renderer-less document,
// our loader may have not fetched the image, so do it now.
if (insertionType.connectedToDocument && !m_imageLoader.image())
- m_imageLoader.updateFromElement();
+ return InsertedIntoAncestorResult::NeedsPostInsertionCallback;
- return insertNotificationRequest;
+ return InsertedIntoAncestorResult::Done;
}
+void HTMLImageElement::didFinishInsertingNode()
+{
+ if (pictureElement())
+ selectImageSource();
+ else if (isConnected())
+ m_imageLoader.updateFromElement();
+}
+
void HTMLImageElement::removedFromAncestor(RemovalType removalType, ContainerNode& oldParentOfRemovedTree)
{
if (m_form)
Modified: trunk/Source/WebCore/html/HTMLImageElement.h (225877 => 225878)
--- trunk/Source/WebCore/html/HTMLImageElement.h 2017-12-13 23:58:37 UTC (rev 225877)
+++ trunk/Source/WebCore/html/HTMLImageElement.h 2017-12-14 00:05:42 UTC (rev 225878)
@@ -90,9 +90,9 @@
bool hasPendingActivity() const { return m_imageLoader.hasPendingActivity(); }
- bool canContainRangeEndPoint() const override { return false; }
+ bool canContainRangeEndPoint() const final { return false; }
- const AtomicString& imageSourceURL() const override;
+ const AtomicString& imageSourceURL() const final;
bool hasShadowControls() const { return m_experimentalImageMenuEnabled; }
@@ -105,26 +105,27 @@
void didMoveToNewDocument(Document& oldDocument, Document& newDocument) override;
private:
- void parseAttribute(const QualifiedName&, const AtomicString&) override;
- bool isPresentationAttribute(const QualifiedName&) const override;
- void collectStyleForPresentationAttribute(const QualifiedName&, const AtomicString&, MutableStyleProperties&) override;
+ void parseAttribute(const QualifiedName&, const AtomicString&) final;
+ bool isPresentationAttribute(const QualifiedName&) const final;
+ void collectStyleForPresentationAttribute(const QualifiedName&, const AtomicString&, MutableStyleProperties&) final;
- void didAttachRenderers() override;
- RenderPtr<RenderElement> createElementRenderer(RenderStyle&&, const RenderTreePosition&) override;
+ void didAttachRenderers() final;
+ RenderPtr<RenderElement> createElementRenderer(RenderStyle&&, const RenderTreePosition&) final;
void setBestFitURLAndDPRFromImageCandidate(const ImageCandidate&);
- bool canStartSelection() const override;
+ bool canStartSelection() const final;
- bool isURLAttribute(const Attribute&) const override;
- bool attributeContainsURL(const Attribute&) const override;
- String completeURLsInAttributeValue(const URL& base, const Attribute&) const override;
+ bool isURLAttribute(const Attribute&) const final;
+ bool attributeContainsURL(const Attribute&) const final;
+ String completeURLsInAttributeValue(const URL& base, const Attribute&) const final;
- bool draggable() const override;
+ bool draggable() const final;
- void addSubresourceAttributeURLs(ListHashSet<URL>&) const override;
+ void addSubresourceAttributeURLs(ListHashSet<URL>&) const final;
- InsertedIntoAncestorResult insertedIntoAncestor(InsertionType, ContainerNode&) override;
- void removedFromAncestor(RemovalType, ContainerNode&) override;
+ InsertedIntoAncestorResult insertedIntoAncestor(InsertionType, ContainerNode&) final;
+ void didFinishInsertingNode() final;
+ void removedFromAncestor(RemovalType, ContainerNode&) final;
bool isFormAssociatedElement() const final { return false; }
FormNamedItem* asFormNamedItem() final { return this; }
@@ -152,7 +153,7 @@
void tryCreateImageControls();
void destroyImageControls();
bool hasImageControls() const;
- bool childShouldCreateRenderer(const Node&) const override;
+ bool childShouldCreateRenderer(const Node&) const final;
#endif
friend class HTMLPictureElement;
Modified: trunk/Source/WebCore/html/HTMLInputElement.cpp (225877 => 225878)
--- trunk/Source/WebCore/html/HTMLInputElement.cpp 2017-12-13 23:58:37 UTC (rev 225877)
+++ trunk/Source/WebCore/html/HTMLInputElement.cpp 2017-12-14 00:05:42 UTC (rev 225878)
@@ -58,6 +58,7 @@
#include "SearchInputType.h"
#include "Settings.h"
#include "StyleResolver.h"
+#include "StyleTreeResolver.h"
#include "TextControlInnerElements.h"
#include <wtf/Language.h>
#include <wtf/MathExtras.h>
@@ -845,7 +846,11 @@
{
HTMLTextFormControlElement::didAttachRenderers();
- m_inputType->attach();
+ if (m_inputType->needsPostStyleResolutionCallback()) {
+ Style::queuePostResolutionCallback([protectedElement = makeRef(*this)] {
+ protectedElement->m_inputType->updateAfterStyleResolution();
+ });
+ }
if (document().focusedElement() == this) {
document().view()->queuePostLayoutCallback([protectedThis = makeRef(*this)] {
Modified: trunk/Source/WebCore/html/HTMLMetaElement.h (225877 => 225878)
--- trunk/Source/WebCore/html/HTMLMetaElement.h 2017-12-13 23:58:37 UTC (rev 225877)
+++ trunk/Source/WebCore/html/HTMLMetaElement.h 2017-12-14 00:05:42 UTC (rev 225878)
@@ -40,7 +40,7 @@
void parseAttribute(const QualifiedName&, const AtomicString&) final;
InsertedIntoAncestorResult insertedIntoAncestor(InsertionType, ContainerNode&) final;
- void didFinishInsertingNode();
+ void didFinishInsertingNode() final;
void process();
};
Modified: trunk/Source/WebCore/html/HTMLPictureElement.cpp (225877 => 225878)
--- trunk/Source/WebCore/html/HTMLPictureElement.cpp 2017-12-13 23:58:37 UTC (rev 225877)
+++ trunk/Source/WebCore/html/HTMLPictureElement.cpp 2017-12-14 00:05:42 UTC (rev 225878)
@@ -56,8 +56,11 @@
void HTMLPictureElement::sourcesChanged()
{
+ Vector<Ref<HTMLImageElement>, 4> imageElements;
for (auto& element : childrenOfType<HTMLImageElement>(*this))
- element.selectImageSource();
+ imageElements.append(element);
+ for (auto& element : imageElements)
+ element->selectImageSource();
}
bool HTMLPictureElement::viewportChangeAffectedPicture() const
Modified: trunk/Source/WebCore/html/HTMLSourceElement.cpp (225877 => 225878)
--- trunk/Source/WebCore/html/HTMLSourceElement.cpp 2017-12-13 23:58:37 UTC (rev 225877)
+++ trunk/Source/WebCore/html/HTMLSourceElement.cpp 2017-12-14 00:05:42 UTC (rev 225878)
@@ -74,11 +74,19 @@
else
#endif
if (is<HTMLPictureElement>(*parent))
- downcast<HTMLPictureElement>(*parent).sourcesChanged();
+ return InsertedIntoAncestorResult::NeedsPostInsertionCallback;
+
}
return InsertedIntoAncestorResult::Done;
}
+void HTMLSourceElement::didFinishInsertingNode()
+{
+ auto* parent = parentElement();
+ if (is<HTMLPictureElement>(*parent))
+ downcast<HTMLPictureElement>(*parent).sourcesChanged();
+}
+
void HTMLSourceElement::removedFromAncestor(RemovalType removalType, ContainerNode& oldParentOfRemovedTree)
{
HTMLElement::removedFromAncestor(removalType, oldParentOfRemovedTree);
Modified: trunk/Source/WebCore/html/HTMLSourceElement.h (225877 => 225878)
--- trunk/Source/WebCore/html/HTMLSourceElement.h 2017-12-13 23:58:37 UTC (rev 225877)
+++ trunk/Source/WebCore/html/HTMLSourceElement.h 2017-12-14 00:05:42 UTC (rev 225878)
@@ -46,6 +46,7 @@
HTMLSourceElement(const QualifiedName&, Document&);
InsertedIntoAncestorResult insertedIntoAncestor(InsertionType, ContainerNode&) final;
+ void didFinishInsertingNode() final;
void removedFromAncestor(RemovalType, ContainerNode&) final;
bool isURLAttribute(const Attribute&) const final;
Modified: trunk/Source/WebCore/html/HTMLVideoElement.cpp (225877 => 225878)
--- trunk/Source/WebCore/html/HTMLVideoElement.cpp 2017-12-13 23:58:37 UTC (rev 225877)
+++ trunk/Source/WebCore/html/HTMLVideoElement.cpp 2017-12-14 00:05:42 UTC (rev 225878)
@@ -43,6 +43,7 @@
#include "RenderVideo.h"
#include "ScriptController.h"
#include "Settings.h"
+#include "StyleTreeResolver.h"
#include <wtf/text/TextStream.h>
#if ENABLE(VIDEO_PRESENTATION_MODE)
@@ -90,14 +91,21 @@
updateDisplayState();
if (shouldDisplayPosterImage()) {
- if (!m_imageLoader)
- m_imageLoader = std::make_unique<HTMLImageLoader>(*this);
- m_imageLoader->updateFromElement();
- if (auto* renderer = this->renderer())
- renderer->imageResource().setCachedImage(m_imageLoader->image());
+ Style::queuePostResolutionCallback([protectedThis = makeRef(*this)] {
+ protectedThis->updateAfterStyleResolution();
+ });
}
}
+void HTMLVideoElement::updateAfterStyleResolution()
+{
+ if (!m_imageLoader)
+ m_imageLoader = std::make_unique<HTMLImageLoader>(*this);
+ m_imageLoader->updateFromElement();
+ if (auto* renderer = this->renderer())
+ renderer->imageResource().setCachedImage(m_imageLoader->image());
+}
+
void HTMLVideoElement::collectStyleForPresentationAttribute(const QualifiedName& name, const AtomicString& value, MutableStyleProperties& style)
{
if (name == widthAttr)
Modified: trunk/Source/WebCore/html/HTMLVideoElement.h (225877 => 225878)
--- trunk/Source/WebCore/html/HTMLVideoElement.h 2017-12-13 23:58:37 UTC (rev 225877)
+++ trunk/Source/WebCore/html/HTMLVideoElement.h 2017-12-14 00:05:42 UTC (rev 225878)
@@ -96,6 +96,7 @@
void scheduleResizeEventIfSizeChanged() final;
bool rendererIsNeeded(const RenderStyle&) final;
void didAttachRenderers() final;
+ void updateAfterStyleResolution();
void parseAttribute(const QualifiedName&, const AtomicString&) final;
bool isPresentationAttribute(const QualifiedName&) const final;
void collectStyleForPresentationAttribute(const QualifiedName&, const AtomicString&, MutableStyleProperties&) final;
Modified: trunk/Source/WebCore/html/ImageInputType.cpp (225877 => 225878)
--- trunk/Source/WebCore/html/ImageInputType.cpp 2017-12-13 23:58:37 UTC (rev 225877)
+++ trunk/Source/WebCore/html/ImageInputType.cpp 2017-12-14 00:05:42 UTC (rev 225878)
@@ -126,10 +126,13 @@
element().ensureImageLoader().updateFromElementIgnoringPreviousError();
}
-void ImageInputType::attach()
+bool ImageInputType::needsPostStyleResolutionCallback()
{
- BaseButtonInputType::attach();
+ return true;
+}
+void ImageInputType::updateAfterStyleResolution()
+{
HTMLImageLoader& imageLoader = element().ensureImageLoader();
imageLoader.updateFromElement();
Modified: trunk/Source/WebCore/html/ImageInputType.h (225877 => 225878)
--- trunk/Source/WebCore/html/ImageInputType.h 2017-12-13 23:58:37 UTC (rev 225877)
+++ trunk/Source/WebCore/html/ImageInputType.h 2017-12-14 00:05:42 UTC (rev 225878)
@@ -50,7 +50,8 @@
void handleDOMActivateEvent(Event&) override;
void altAttributeChanged() override;
void srcAttributeChanged() override;
- void attach() override;
+ bool needsPostStyleResolutionCallback() override;
+ void updateAfterStyleResolution() override;
bool shouldRespectAlignAttribute() override;
bool canBeSuccessfulSubmitButton() override;
bool isImageButton() const override;
Modified: trunk/Source/WebCore/html/InputType.cpp (225877 => 225878)
--- trunk/Source/WebCore/html/InputType.cpp 2017-12-13 23:58:37 UTC (rev 225877)
+++ trunk/Source/WebCore/html/InputType.cpp 2017-12-14 00:05:42 UTC (rev 225878)
@@ -587,10 +587,15 @@
{
}
-void InputType::attach()
+bool InputType::needsPostStyleResolutionCallback()
{
+ return false;
}
+void InputType::updateAfterStyleResolution()
+{
+}
+
void InputType::detach()
{
}
Modified: trunk/Source/WebCore/html/InputType.h (225877 => 225878)
--- trunk/Source/WebCore/html/InputType.h 2017-12-13 23:58:37 UTC (rev 225877)
+++ trunk/Source/WebCore/html/InputType.h 2017-12-14 00:05:42 UTC (rev 225878)
@@ -235,7 +235,8 @@
virtual bool rendererIsNeeded();
virtual RenderPtr<RenderElement> createInputRenderer(RenderStyle&&);
virtual void addSearchResult();
- virtual void attach();
+ virtual bool needsPostStyleResolutionCallback();
+ virtual void updateAfterStyleResolution();
virtual void detach();
virtual void minOrMaxAttributeChanged();
virtual void stepAttributeChanged();
Modified: trunk/Source/WebCore/loader/ImageLoader.cpp (225877 => 225878)
--- trunk/Source/WebCore/loader/ImageLoader.cpp 2017-12-13 23:58:37 UTC (rev 225877)
+++ trunk/Source/WebCore/loader/ImageLoader.cpp 2017-12-14 00:05:42 UTC (rev 225878)
@@ -36,6 +36,7 @@
#include "HTMLNames.h"
#include "HTMLObjectElement.h"
#include "HTMLParserIdioms.h"
+#include "NoEventDispatchAssertion.h"
#include "Page.h"
#include "RenderImage.h"
#include "RenderSVGImage.h"
@@ -157,6 +158,7 @@
void ImageLoader::updateFromElement()
{
+ RELEASE_ASSERT(NoEventDispatchAssertion::InMainThread::isEventAllowed());
// If we're not making renderers for the page, then don't load images. We don't want to slow
// down the raw HTML parsing case by loading images we don't intend to display.
Document& document = element().document();
Modified: trunk/Source/WebCore/svg/SVGImageElement.cpp (225877 => 225878)
--- trunk/Source/WebCore/svg/SVGImageElement.cpp 2017-12-13 23:58:37 UTC (rev 225877)
+++ trunk/Source/WebCore/svg/SVGImageElement.cpp 2017-12-14 00:05:42 UTC (rev 225878)
@@ -189,14 +189,16 @@
Node::InsertedIntoAncestorResult SVGImageElement::insertedIntoAncestor(InsertionType insertionType, ContainerNode& parentOfInsertedTree)
{
SVGGraphicsElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
- if (!insertionType.connectedToDocument)
- return InsertedIntoAncestorResult::Done;
- // Update image loader, as soon as we're living in the tree.
- // We can only resolve base URIs properly, after that!
- m_imageLoader.updateFromElement();
+ if (insertionType.connectedToDocument)
+ return InsertedIntoAncestorResult::NeedsPostInsertionCallback;
return InsertedIntoAncestorResult::Done;
}
+void SVGImageElement::didFinishInsertingNode()
+{
+ m_imageLoader.updateFromElement();
+}
+
const AtomicString& SVGImageElement::imageSourceURL() const
{
return getAttribute(XLinkNames::hrefAttr);
Modified: trunk/Source/WebCore/svg/SVGImageElement.h (225877 => 225878)
--- trunk/Source/WebCore/svg/SVGImageElement.h 2017-12-13 23:58:37 UTC (rev 225877)
+++ trunk/Source/WebCore/svg/SVGImageElement.h 2017-12-14 00:05:42 UTC (rev 225878)
@@ -50,6 +50,7 @@
void didAttachRenderers() final;
InsertedIntoAncestorResult insertedIntoAncestor(InsertionType, ContainerNode&) final;
+ void didFinishInsertingNode() final;
RenderPtr<RenderElement> createElementRenderer(RenderStyle&&, const RenderTreePosition&) final;
