Changeset 91738 in webkit


Ignore:
Timestamp:
Jul 25, 2011, 8:21:52 PM (13 years ago)
Author:
mitz@apple.com
Message:

<rdar://problem/9835028> Font loading during layout can cause layout code to be re-entered via resource load delegate
​https://bugs.webkit.org/show_bug.cgi?id=65123

Source/WebCore:

Reviewed by Anders Carlsson and Darin Adler.

Since CSSFontFaceSource::getFontData() can get called during layout, avoid calling out to loader
code from under it, and instead defer that work using a zero-delay timer.

  • css/CSSFontFaceSource.cpp:

(WebCore::CSSFontFaceSource::CSSFontFaceSource):
(WebCore::CSSFontFaceSource::~CSSFontFaceSource):
(WebCore::CSSFontFaceSource::getFontData): Rather than starting the font load here, schedule
a zero-delay timer to do it.
(WebCore::CSSFontFaceSource::startLoadingTimerFired): Added. Starts loading the font if needed.

  • css/CSSFontFaceSource.h:

LayoutTests:

Reviewed by Darin Adler.

Updated tests that depended on fonts loading synchronously during layout.
Unfortunately, font loading does not fire any DOM events, so in most cases
a constant timeout had to be introduced.

  • fast/blockflow/broken-ideograph-small-caps.html:
  • fast/css/color-leakage.html:
  • fast/css/custom-font-xheight.html:
  • fast/css/font-face-multiple-faces.html:
  • fast/css/font-face-multiple-remote-sources.html:
  • fast/css/font-face-remote.html:
  • fast/css/font-face-woff.html:
  • svg/W3C-SVG-1.1-SE/text-intro-09-b.svg:
  • svg/W3C-SVG-1.1/fonts-elem-07-b.svg:
  • svg/custom/svg-fonts-fallback.xhtml:
  • svg/custom/svg-fonts-in-html.html:
  • svg/custom/svg-fonts-segmented.xhtml:
  • svg/custom/svg-fonts-with-no-element-reference.html:
  • svg/custom/svg-fonts-without-missing-glyph.xhtml:
  • svg/text/text-overflow-ellipsis-svgfont.html:
Location:
trunk
Files:
19 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r91732 r91738  
     12011-07-25  Dan Bernstein  <mitz@apple.com>
     2
     3        <rdar://problem/9835028> Font loading during layout can cause layout code to be re-entered via resource load delegate
     4        https://bugs.webkit.org/show_bug.cgi?id=65123
     5
     6        Reviewed by Darin Adler.
     7
     8        Updated tests that depended on fonts loading synchronously during layout.
     9        Unfortunately, font loading does not fire any DOM events, so in most cases
     10        a constant timeout had to be introduced.
     11
     12        * fast/blockflow/broken-ideograph-small-caps.html:
     13        * fast/css/color-leakage.html:
     14        * fast/css/custom-font-xheight.html:
     15        * fast/css/font-face-multiple-faces.html:
     16        * fast/css/font-face-multiple-remote-sources.html:
     17        * fast/css/font-face-remote.html:
     18        * fast/css/font-face-woff.html:
     19        * svg/W3C-SVG-1.1-SE/text-intro-09-b.svg:
     20        * svg/W3C-SVG-1.1/fonts-elem-07-b.svg:
     21        * svg/custom/svg-fonts-fallback.xhtml:
     22        * svg/custom/svg-fonts-in-html.html:
     23        * svg/custom/svg-fonts-segmented.xhtml:
     24        * svg/custom/svg-fonts-with-no-element-reference.html:
     25        * svg/custom/svg-fonts-without-missing-glyph.xhtml:
     26        * svg/text/text-overflow-ellipsis-svgfont.html:
     27
    1282011-07-25  Adrienne Walker  <enne@google.com>
    229

  • trunk/LayoutTests/fast/blockflow/broken-ideograph-small-caps.html

    r71979 r91738  
    5858</style>
    5959 
    60 <script type="text/javascript">
    61 
    62 </script>
    63  
    6460</head>
    6561<body>
    … …  
    7066</div>
    7167
     68<script>
     69    if (window.layoutTestController) {
     70        layoutTestController.waitUntilDone();
     71        document.body.offsetTop;
     72        setTimeout(function() { layoutTestController.notifyDone(); }, 100);
     73    }
     74</script>
    7275</body>
    7376</html>

  • trunk/LayoutTests/fast/css/color-leakage.html

    r89397 r91738  
    33  <!-- The Ahem font rendered in this test should be displayed black, not blue. -->
    44    <style>
    5       @font-face {
    6         font-family: 'Ahem';
    7         src: url('resources/Ahem.ttf');
    8       }
    95      div {
    106        display: block;

  • trunk/LayoutTests/fast/css/custom-font-xheight.html

    r60250 r91738  
    3838
    3939<script>
    40 if (window.layoutTestController)
     40if (window.layoutTestController) {
    4141    layoutTestController.dumpAsText();
     42    layoutTestController.waitUntilDone();
     43}
    4244
    4345function test()
    4446{
    45     var totalHeight = document.defaultView.getComputedStyle(document.getElementById("test"), null).getPropertyCSSValue("height");
    46     totalHeight = totalHeight.getFloatValue(CSSPrimitiveValue.CSS_PX);
    47     if (totalHeight > 150 && totalHeight < 300)
    48         document.getElementById("result").innerHTML = "PASS";
    49     else
    50         document.getElementById("result").innerHTML = "FAIL: " + totalHeight + "px";
     47    document.body.offsetTop;
     48    setTimeout(function() {
     49        var totalHeight = document.defaultView.getComputedStyle(document.getElementById("test"), null).getPropertyCSSValue("height");
     50        totalHeight = totalHeight.getFloatValue(CSSPrimitiveValue.CSS_PX);
     51        if (totalHeight > 150 && totalHeight < 300)
     52            document.getElementById("result").innerHTML = "PASS";
     53        else
     54            document.getElementById("result").innerHTML = "FAIL: " + totalHeight + "px";
     55
     56        if (window.layoutTestController)
     57            layoutTestController.notifyDone();
     58    }, 100);
    5159}
    5260</script>

  • trunk/LayoutTests/fast/css/font-face-multiple-faces.html

    r34794 r91738  
    149149    AHEM <i>AHEM</i>
    150150</div>
     151<script>
     152    if (window.layoutTestController) {
     153        layoutTestController.waitUntilDone();
     154        document.body.offsetTop;
     155        setTimeout(function() { layoutTestController.notifyDone(); }, 100);
     156    }
     157</script>

  • trunk/LayoutTests/fast/css/font-face-multiple-remote-sources.html

    r30399 r91738  
    1616    </div>
    1717    <script>
    18         document.body.offsetTop;
     18        if (window.layoutTestController) {
     19            layoutTestController.waitUntilDone();
     20            document.body.offsetTop;
     21            setTimeout(function() { layoutTestController.notifyDone(); }, 100);
     22        }
    1923    </script>
    2024</body>

  • trunk/LayoutTests/fast/css/font-face-remote.html

    r29867 r91738  
    99    </style>
    1010</head>
    11 <body onload="finished()">
     11<body>
    1212    <div>
    1313        FAIL_<br>
    … …  
    1818    </div>
    1919    <script>
    20         if (window.layoutTestController)
     20        if (window.layoutTestController) {
    2121            layoutTestController.waitUntilDone();
    22 
    23         // Kick off loading of the font
    24         document.body.offsetTop;
    25 
    26         function finished()
    27         {
    28             if (window.layoutTestController)
    29                 layoutTestController.notifyDone();
     22            document.body.offsetTop;
     23            setTimeout(function() { layoutTestController.notifyDone(); }, 100);
    3024        }
    3125    </script>

  • trunk/LayoutTests/fast/css/font-face-woff.html

    r58517 r91738  
    99
    1010<p style="font-family: family1; font-size: 4em;">Failure</p>
     11<script>
     12    if (window.layoutTestController) {
     13        layoutTestController.waitUntilDone();
     14        document.body.offsetTop;
     15        setTimeout(function() { layoutTestController.notifyDone(); }, 100);
     16    }
     17</script>

  • trunk/LayoutTests/svg/W3C-SVG-1.1-SE/text-intro-09-b.svg

    r91493 r91738  
    8787      text-anchor="middle" y="18" stroke-width="0.5" stroke="black" fill="white">DRAFT</text>
    8888  </g>-->
     89<script>
     90    if (window.layoutTestController) {
     91        layoutTestController.waitUntilDone();
     92        document.documentElement.offsetTop;
     93        setTimeout(function() { layoutTestController.notifyDone(); }, 100);
     94    }
     95</script>
    8996</svg>

  • trunk/LayoutTests/svg/W3C-SVG-1.1/fonts-elem-07-b.svg

    r28763 r91738  
    114114        <text id="revision" x="10" y="340" font-size="40" stroke="none" fill="black">$Revision: 1.7 $</text>
    115115        <rect id="test-frame" x="1" y="1" width="478" height="358" fill="none" stroke="#000000"/>
     116        <script>
     117            if (window.layoutTestController) {
     118                layoutTestController.waitUntilDone();
     119                document.documentElement.offsetTop;
     120                setTimeout(function() { layoutTestController.notifyDone(); }, 100);
     121            }
     122        </script>
    116123</svg>

  • trunk/LayoutTests/svg/custom/svg-fonts-fallback.xhtml

    r89732 r91738  
    7777    <span style='font-family: TestFont2; font-size: 40px; '>a &#xbe2; o</span><br/>
    7878</p>
    79 
     79<script>
     80    if (window.layoutTestController) {
     81        layoutTestController.waitUntilDone();
     82        document.documentElement.offsetTop;
     83        setTimeout(function() { layoutTestController.notifyDone(); }, 100);
     84    }
     85</script>
    8086</body>
    8187</html>

  • trunk/LayoutTests/svg/custom/svg-fonts-in-html.html

    r29839 r91738  
    209209<div id="extraDiv1"><span></span></div><div id="extraDiv2"><span></span></div><div id="extraDiv3"><span></span></div>
    210210<div id="extraDiv4"><span></span></div><div id="extraDiv5"><span></span></div><div id="extraDiv6"><span></span></div>
    211 
     211<script>
     212    if (window.layoutTestController) {
     213        layoutTestController.waitUntilDone();
     214        document.documentElement.offsetTop;
     215        setTimeout(function() { layoutTestController.notifyDone(); }, 100);
     216    }
     217</script>
    212218</body>
    213219</html>

  • trunk/LayoutTests/svg/custom/svg-fonts-segmented.xhtml

    r89732 r91738  
    3838<!-- 'ABC' should be rendered using Times, 'def' using Courier, 'o' using the SVG Font, and 'O' again by Times -->
    3939<p style='font-family: TestFont; font-size: 40px; '>ABCdefoooO</p>
    40 
     40<script>
     41    if (window.layoutTestController) {
     42        layoutTestController.waitUntilDone();
     43        document.documentElement.offsetTop;
     44        setTimeout(function() { layoutTestController.notifyDone(); }, 100);
     45    }
     46</script>
    4147</body>
    4248</html>

  • trunk/LayoutTests/svg/custom/svg-fonts-with-no-element-reference.html

    r59194 r91738  
    3232<p class="first">This text should be rendered with a first font.</p>
    3333<p class="second">This text should be rendered with a second font.</p>
     34<script>
     35    if (window.layoutTestController) {
     36        layoutTestController.waitUntilDone();
     37        document.documentElement.offsetTop;
     38        setTimeout(function() { layoutTestController.notifyDone(); }, 100);
     39    }
     40</script>
    3441</body>
    3542

  • trunk/LayoutTests/svg/custom/svg-fonts-without-missing-glyph.xhtml

    r63422 r91738  
    3131<p class="target">XXX</p>
    3232<p class="target">AAA</p>
    33 
     33<script>
     34    if (window.layoutTestController) {
     35        layoutTestController.waitUntilDone();
     36        document.documentElement.offsetTop;
     37        setTimeout(function() { layoutTestController.notifyDone(); }, 100);
     38    }
     39</script>
    3440</body>
    3541</html>

  • trunk/LayoutTests/svg/text/text-overflow-ellipsis-svgfont.html

    r89732 r91738  
    3737</div>
    3838
     39<script>
     40    if (window.layoutTestController) {
     41        layoutTestController.waitUntilDone();
     42        document.documentElement.offsetTop;
     43        setTimeout(function() { layoutTestController.notifyDone(); }, 100);
     44    }
     45</script>
    3946</body>
    4047</html>

  • trunk/Source/WebCore/ChangeLog

    r91736 r91738  
     12011-07-25  Dan Bernstein  <mitz@apple.com>
     2
     3        <rdar://problem/9835028> Font loading during layout can cause layout code to be re-entered via resource load delegate
     4        https://bugs.webkit.org/show_bug.cgi?id=65123
     5
     6        Reviewed by Anders Carlsson and Darin Adler.
     7
     8        Since CSSFontFaceSource::getFontData() can get called during layout, avoid calling out to loader
     9        code from under it, and instead defer that work using a zero-delay timer.
     10
     11        * css/CSSFontFaceSource.cpp:
     12        (WebCore::CSSFontFaceSource::CSSFontFaceSource):
     13        (WebCore::CSSFontFaceSource::~CSSFontFaceSource):
     14        (WebCore::CSSFontFaceSource::getFontData): Rather than starting the font load here, schedule
     15        a zero-delay timer to do it.
     16        (WebCore::CSSFontFaceSource::startLoadingTimerFired): Added. Starts loading the font if needed.
     17        * css/CSSFontFaceSource.h:
     18
    1192011-07-25  Al Patrick  <apatrick@chromium.org>
    220

  • trunk/Source/WebCore/css/CSSFontFaceSource.cpp

    r91710 r91738  
    5151    , m_font(font)
    5252    , m_face(0)
     53    , m_loadStartTimer(this, &CSSFontFaceSource::startLoadingTimerFired)
    5354#if ENABLE(SVG_FONTS)
    5455    , m_hasExternalSVGFont(false)
    … …  
    6162CSSFontFaceSource::~CSSFontFaceSource()
    6263{
     64    m_loadStartTimer.stop();
    6365    if (m_font)
    6466        m_font->removeClient(this);
    … …  
    173175        }
    174176    } else {
    175         // Kick off the load now.
    176         if (CachedResourceLoader* cachedResourceLoader = fontSelector->cachedResourceLoader())
    177             m_font->beginLoadIfNeeded(cachedResourceLoader);
     177        // Kick off the load now. Do it on a zero-delay timer rather than synchronously, because we may be in
     178        // the middle of layout, and the loader may invoke arbitrary delegate or event handler code.
     179        m_fontSelector = fontSelector;
     180        if (!m_loadStartTimer.isActive())
     181            m_loadStartTimer.startOneShot(0);
     182
    178183        // FIXME: m_string is a URL so it makes no sense to pass it as a family name.
    179184        SimpleFontData* tempData = fontCache()->getCachedFontData(fontDescription, m_string);
    … …  
    190195}
    191196
     197void CSSFontFaceSource::startLoadingTimerFired(Timer<WebCore::CSSFontFaceSource>*)
     198{
     199    ASSERT(m_font);
     200    ASSERT(m_fontSelector);
     201
     202    if (CachedResourceLoader* cachedResourceLoader = m_fontSelector->cachedResourceLoader())
     203        m_font->beginLoadIfNeeded(cachedResourceLoader);
     204
     205    m_fontSelector = nullptr;
     206}
     207
    192208#if ENABLE(SVG_FONTS)
    193209SVGFontFaceElement* CSSFontFaceSource::svgFontFaceElement() const

  • trunk/Source/WebCore/css/CSSFontFaceSource.h

    r91710 r91738  
    2929#include "CachedResourceClient.h"
    3030#include "CachedResourceHandle.h"
     31#include "Timer.h"
    3132#include <wtf/HashMap.h>
    3233#include <wtf/text/AtomicString.h>
    … …  
    7172
    7273private:
     74    void startLoadingTimerFired(Timer<CSSFontFaceSource>*);
     75
    7376    AtomicString m_string; // URI for remote, built-in font name for local.
    7477    CachedResourceHandle<CachedFont> m_font; // For remote fonts, a pointer to our cached resource.
    7578    CSSFontFace* m_face; // Our owning font face.
    7679    HashMap<unsigned, SimpleFontData*> m_fontDataTable; // The hash key is composed of size synthetic styles.
     80
     81    Timer<CSSFontFaceSource> m_startLoadingTimer;
     82    RefPtr<CSSFontSelector> m_fontSelector;
    7783
    7884#if ENABLE(SVG_FONTS)
Note: See TracChangeset for help on using the changeset viewer.