Attachment 343800 Details for Bug 187137 – Fixes the bug

[patch] Fixes the bug

bug-187137-20180628002021.patch (text/plain), 4.34 KB, created by Ryosuke Niwa on 2018-06-28 00:20:22 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Ryosuke Niwa

Created: 2018-06-28 00:20:22 PDT

Size: 4.34 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 233300)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,29 @@
>+2018-06-27  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        Release assert in ScriptController::canExecuteScripts via WebCore::SVGUseElement::insertedIntoAncestor
>+        https://bugs.webkit.org/show_bug.cgi?id=187137
>+        <rdar://problem/41081885>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The bug was caused by SVGUseElement::notifyFinished firing a DOM event via SVGUseElement::updateExternalDocument
>+        inside SVGUseElement::insertedIntoAncestor. Ideally, we make every call to notifyFinished asynchronous
>+        but simply delay the call to updateExternalDocument() until didFinishInsertingNode() for now.
>+
>+        No new tests since the failure is caught with the newly added assertion in notifyFinished by existing SVG tests
>+        such as svg/batik/filters/filterRegions.svg and svg/batik/text/smallFonts.svg. Unfortunately, I could not
>+        construct a test case which hits this release assertion since the real crash happens when the cached resource
>+        had an error but in the all cases I could find, the resource response with an error results in a reload or
>+        an asynchronous failure callback.
>+
>+        * loader/cache/CachedResource.cpp:
>+        (WebCore::CachedResource::didAddClient): Added a FIXME.
>+        * svg/SVGUseElement.cpp:
>+        (WebCore::SVGUseElement::insertedIntoAncestor): Delay the call to updateExternalDocument.
>+        (WebCore::SVGUseElement::didFinishInsertingNode): Invoke updateExternalDocument.
>+        (WebCore::SVGUseElement::notifyFinished): Added an assertion.
>+        * svg/SVGUseElement.h:
>+
> 2018-06-27  Timothy Hatcher  <timothy@apple.com>
> 
>         Don't expose new semantic -apple-system color keywords on iOS.
>Index: Source/WebCore/loader/cache/CachedResource.cpp
>===================================================================
>--- Source/WebCore/loader/cache/CachedResource.cpp	(revision 233226)
>+++ Source/WebCore/loader/cache/CachedResource.cpp	(working copy)
>@@ -505,6 +505,8 @@ void CachedResource::didAddClient(Cached
> 
>     if (m_clientsAwaitingCallback.remove(&client))
>         m_clients.add(&client);
>+
>+    // FIXME: Make calls to notifyFinished async
>     if (!isLoading() && !stillNeedsLoad())
>         client.notifyFinished(*this);
> }
>Index: Source/WebCore/svg/SVGUseElement.cpp
>===================================================================
>--- Source/WebCore/svg/SVGUseElement.cpp	(revision 233226)
>+++ Source/WebCore/svg/SVGUseElement.cpp	(working copy)
>@@ -115,11 +115,17 @@ Node::InsertedIntoAncestorResult SVGUseE
>             document().addSVGUseElement(*this);
>         SVGExternalResourcesRequired::insertedIntoDocument(this);
>         invalidateShadowTree();
>-        updateExternalDocument();
>+        // FIXME: Move back the call to updateExternalDocument() here once notifyFinished is made always async.
>+        return InsertedIntoAncestorResult::NeedsPostInsertionCallback;
>     }
>     return InsertedIntoAncestorResult::Done;
> }
> 
>+void SVGUseElement::didFinishInsertingNode()
>+{
>+    updateExternalDocument();
>+}
>+
> void SVGUseElement::removedFromAncestor(RemovalType removalType, ContainerNode& oldParentOfRemovedTree)
> {
>     // Check m_shadowTreeNeedsUpdate before calling SVGElement::removedFromAncestor which calls SVGElement::invalidateInstances
>@@ -558,6 +564,7 @@ bool SVGUseElement::selfHasRelativeLengt
> 
> void SVGUseElement::notifyFinished(CachedResource& resource)
> {
>+    ASSERT(ScriptDisallowedScope::InMainThread::isScriptAllowed());
>     invalidateShadowTree();
>     if (resource.errorOccurred())
>         dispatchEvent(Event::create(eventNames().errorEvent, false, false));
>Index: Source/WebCore/svg/SVGUseElement.h
>===================================================================
>--- Source/WebCore/svg/SVGUseElement.h	(revision 233226)
>+++ Source/WebCore/svg/SVGUseElement.h	(working copy)
>@@ -61,6 +61,7 @@ private:
> 
>     bool isValid() const override;
>     InsertedIntoAncestorResult insertedIntoAncestor(InsertionType, ContainerNode&) override;
>+    void didFinishInsertingNode() final;
>     void removedFromAncestor(RemovalType, ContainerNode&) override;
>     void buildPendingResource() override;
>     void parseAttribute(const QualifiedName&, const AtomicString&) override;

Actions: View | Formatted Diff | Diff

Attachments on bug 187137: 343800 | 343803