Release assert in ScriptController::canExecuteScripts via CachedSVGFont::ensureCustom...
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Apr 2018 03:42:06 +0000 (03:42 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Apr 2018 03:42:06 +0000 (03:42 +0000)
Document::updateStyleIfNeeded
https://bugs.webkit.org/show_bug.cgi?id=184950

Reviewed by Zalan Bujtas.

Convert an existing ScriptDisallowedScope::EventAllowedScope which only disables the debug assertions
by ScriptDisallowedScope::DisableAssertionsInScope which also disables the release assertion.

Because SVG font is loaded in a document isolated from the rest of the page (m_externalSVGDocument),
there is no security implication to execute scripts in this isolated document.

Unfortunately, no new tests. I could never make CachedSVGFont::ensureCustomFontData to get called inside
style resolution with m_externalSVGDocument set to nullptr after many attempts. Even EventAllowedScope
I added 13 months ago in r211965, which this patch replaces by DisableAssertionsInScope, is not utilized
by the existing layout tests since removing the assertion doesn't cause any layout test to hit an assertion.

* dom/ScriptDisallowedScope.h: Updated the comment.
* loader/cache/CachedSVGFont.cpp:
(WebCore::CachedSVGFont::ensureCustomFontData): Replaced the asssertion.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@230983 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/dom/ScriptDisallowedScope.h
Source/WebCore/loader/cache/CachedSVGFont.cpp

index f7af59a..195df52 100644 (file)
@@ -1,3 +1,26 @@
+2018-04-24  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Release assert in ScriptController::canExecuteScripts via CachedSVGFont::ensureCustomFontData during
+        Document::updateStyleIfNeeded
+        https://bugs.webkit.org/show_bug.cgi?id=184950
+
+        Reviewed by Zalan Bujtas.
+
+        Convert an existing ScriptDisallowedScope::EventAllowedScope which only disables the debug assertions
+        by ScriptDisallowedScope::DisableAssertionsInScope which also disables the release assertion.
+
+        Because SVG font is loaded in a document isolated from the rest of the page (m_externalSVGDocument),
+        there is no security implication to execute scripts in this isolated document.
+
+        Unfortunately, no new tests. I could never make CachedSVGFont::ensureCustomFontData to get called inside
+        style resolution with m_externalSVGDocument set to nullptr after many attempts. Even EventAllowedScope
+        I added 13 months ago in r211965, which this patch replaces by DisableAssertionsInScope, is not utilized
+        by the existing layout tests since removing the assertion doesn't cause any layout test to hit an assertion.
+
+        * dom/ScriptDisallowedScope.h: Updated the comment.
+        * loader/cache/CachedSVGFont.cpp:
+        (WebCore::CachedSVGFont::ensureCustomFontData): Replaced the asssertion.
+
 2018-04-24  Simon Fraser  <simon.fraser@apple.com>
 
         visitedDependentColor() should take a CSSPropertyID
index 521e94d..9b2b77f 100644 (file)
@@ -129,7 +129,8 @@ public:
     };
 #endif
 
-    // FIXME: Remove this class once the sync layout inside SVGImage::draw is removed
+    // FIXME: Remove this class once the sync layout inside SVGImage::draw is removed,
+    // CachedSVGFont::ensureCustomFontData no longer synchronously creates a document during style resolution,
     // and refactored the code in RenderFrameBase::performLayoutWithFlattening.
     class DisableAssertionsInScope {
     public:
index 8f140d5..aeaa8e5 100644 (file)
@@ -75,7 +75,7 @@ bool CachedSVGFont::ensureCustomFontData(const AtomicString& remoteURI)
             m_externalSVGDocument = SVGDocument::create(nullptr, URL());
             auto decoder = TextResourceDecoder::create("application/xml");
 
-            ScriptDisallowedScope::EventAllowedScope allowedScope(*m_externalSVGDocument);
+            ScriptDisallowedScope::DisableAssertionsInScope disabledScope;
 
             m_externalSVGDocument->setContent(decoder->decodeAndFlush(m_data->data(), m_data->size()));
             sawError = decoder->sawError();