Changeset 158085 in webkit

Timestamp:

Oct 26, 2013 8:44:26 AM (10 years ago)

Author:

Antti Koivisto

Message:

Source/WebCore: fast/frames/seamless/seamless-nested-crash.html asserts on wk2 only
​https://bugs.webkit.org/show_bug.cgi?id=123354

Reviewed by Andreas Kling.

• rendering/SimpleLineLayout.cpp:

(WebCore::SimpleLineLayout::canUseFor):

Don't enable simple line layout if the primary font is loading. The code expects
to use the primary font metrics for all lines but those won't match the fallbacks
when font is not loaded.

LayoutTests: fast/frames/seamless/seamless-nested-crash.html asserts on wk2 only
​https://bugs.webkit.org/show_bug.cgi?id=123354

Reviewed by Andreas Kling.

• TestExpectations: Remove failure expectation

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/css/CSSFontFaceSource.cpp (modified) (2 diffs)
• Source/WebCore/css/CSSFontSelector.cpp (modified) (5 diffs)
• Source/WebCore/css/CSSFontSelector.h (modified) (1 diff)
• Source/WebCore/rendering/SimpleLineLayout.cpp (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2013-10-26  Antti Koivisto  <antti@apple.com>
+
+        fast/frames/seamless/seamless-nested-crash.html asserts on wk2 only
+        https://bugs.webkit.org/show_bug.cgi?id=123354
+        
+        Reviewed by Andreas Kling.
+
+        * TestExpectations: Remove failure expectation
+
 2013-10-25  Alexey Proskuryakov  <ap@apple.com>
 

trunk/LayoutTests/TestExpectations

@@ -76 +76 @@
 webkit.org/b/122679 security/crypto-subtle-gc-2.html [ Skip ]
 webkit.org/b/122679 security/crypto-subtle-gc-3.html [ Skip ]
-
-# hits simple line layout sanity check assert
-webkit.org/b/123354 fast/frames/seamless/seamless-nested-crash.html [ Skip ]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-10-26  Antti Koivisto  <antti@apple.com>
+
+        fast/frames/seamless/seamless-nested-crash.html asserts on wk2 only
+        https://bugs.webkit.org/show_bug.cgi?id=123354
+
+        Reviewed by Andreas Kling.
+
+        * rendering/SimpleLineLayout.cpp:
+        (WebCore::SimpleLineLayout::canUseFor):
+        
+            Don't enable simple line layout if the primary font is loading. The code expects
+            to use the primary font metrics for all lines but those won't match the fallbacks
+            when font is not loaded.
+
 2013-10-26  Carlos Garcia Campos  <cgarcia@igalia.com>
 

trunk/Source/WebCore/css/CSSFontFaceSource.cpp

@@ -116 +116 @@
                        | (fontDescription.orientation() == Vertical ? 4 : 0) | (syntheticBold ? 2 : 0) | (syntheticItalic ? 1 : 0);
 
-    RefPtr<SimpleFontData>& fontData = m_fontDataTable.add(hashKey, nullptr).iterator->value;
+    RefPtr<SimpleFontData> fontData = m_fontDataTable.add(hashKey, nullptr).iterator->value;
     if (fontData)
-        return fontData; // No release, because fontData is a reference to a RefPtr that is held in the m_fontDataTable.
+        return fontData.release();
 
     // If we are still loading, then we let the system pick a font.
@@ -171 +171 @@
         fontSelector->beginLoadingFontSoon(m_font.get());
 
-        // This temporary font is not retained and should not be returned.
-        FontCachePurgePreventer fontCachePurgePreventer;
-        SimpleFontData* temporaryFont = fontCache()->getNonRetainedLastResortFallbackFont(fontDescription);
-        fontData = SimpleFontData::create(temporaryFont->platformData(), true, true);
+        RefPtr<SimpleFontData> fallback = fontSelector->userStandardFont(fontDescription);
+        if (!fallback)
+            fallback = fontCache()->getLastResortFallbackFont(fontDescription);
+        fontData = SimpleFontData::create(fallback->platformData(), /*isCustomFont*/ true, /*isLoading*/ true);
     }
 
-    return fontData; // No release, because fontData is a reference to a RefPtr that is held in the m_fontDataTable.
+    return fontData.release();
 }
 

trunk/Source/WebCore/css/CSSFontSelector.cpp

@@ -364 +364 @@
 }
 
-static PassRefPtr<FontData> fontDataForGenericFamily(Document* document, const FontDescription& fontDescription, const AtomicString& familyName)
+static PassRefPtr<SimpleFontData> fontDataForGenericFamily(Document* document, const FontDescription& fontDescription, const AtomicString& familyName)
 {
     if (!document || !document->frame())
@@ -392 +392 @@
         return fontCache()->getCachedFontData(fontDescription, genericFamily);
 
-    return 0;
+    return nullptr;
 }
 
@@ -477 +477 @@
 }
 
+PassRefPtr<SimpleFontData> CSSFontSelector::userStandardFont(const FontDescription& description)
+{
+    return fontDataForGenericFamily(m_document, description, standardFamily);
+}
+
 PassRefPtr<FontData> CSSFontSelector::getFontData(const FontDescription& fontDescription, const AtomicString& familyName)
 {
@@ -483 +488 @@
             return fontDataForGenericFamily(m_document, fontDescription, familyName);
         if (fontDescription.genericFamily() == FontDescription::StandardFamily && !fontDescription.isSpecifiedFont())
-            return fontDataForGenericFamily(m_document, fontDescription, "-webkit-standard");
+            return fontDataForGenericFamily(m_document, fontDescription, standardFamily);
         return 0;
     }
@@ -493 +498 @@
         // settings.
         if (fontDescription.genericFamily() == FontDescription::StandardFamily && !fontDescription.isSpecifiedFont())
-            return fontDataForGenericFamily(m_document, fontDescription, "-webkit-standard");
+            return fontDataForGenericFamily(m_document, fontDescription, standardFamily);
         return fontDataForGenericFamily(m_document, fontDescription, familyName);
     }

trunk/Source/WebCore/css/CSSFontSelector.h

@@ -61 +61 @@
     virtual PassRefPtr<FontData> getFallbackFontData(const FontDescription&, size_t) OVERRIDE;
     CSSSegmentedFontFace* getFontFace(const FontDescription&, const AtomicString& family);
+    virtual PassRefPtr<SimpleFontData> userStandardFont(const FontDescription&) OVERRIDE;
 
     virtual bool resolvesFamilyFor(const FontDescription&) const OVERRIDE;

trunk/Source/WebCore/rendering/SimpleLineLayout.cpp

@@ -150 +150 @@
         return false;
 
-    auto primaryFontData = style.font().primaryFont();
+    // We assume that all lines have metrics based purely on the primary font.
+    auto& primaryFontData = *style.font().primaryFont();
+    if (primaryFontData.isLoading())
+        return false;
 
     unsigned length = textRenderer.textLength();
@@ -158 +161 @@
         // This is because we don't currently do subruns within lines. Fixing this would improve coverage significantly.
         UChar character = textRenderer.characterAt(i);
-        if (isWhitespace(character))
+        if (isWhitespace(character)) {
             ++consecutiveSpaceCount;
-        else {
-            if (consecutiveSpaceCount != i && consecutiveSpaceCount > 1)
-                return false;
-            consecutiveSpaceCount = 0;
+            continue;
         }
+        if (consecutiveSpaceCount != i && consecutiveSpaceCount > 1)
+            return false;
+        consecutiveSpaceCount = 0;
+
         // These would be easy to support.
         if (character == noBreakSpace)
@@ -179 +183 @@
                 return false;
         }
-        if (!primaryFontData->glyphForCharacter(character))
+
+        if (!primaryFontData.glyphForCharacter(character))
             return false;
     }