Crash in -[WebCascadeList objectAtIndex:] + 195
authormmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 6 Mar 2015 19:08:55 +0000 (19:08 +0000)
committermmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 6 Mar 2015 19:08:55 +0000 (19:08 +0000)
https://bugs.webkit.org/show_bug.cgi?id=141274

Reviewed by David Kilzer.

Source/WebCore:

CTFontDescriptorRefs can live forever in caches inside CoreText, which means our
WebCascadeList can too.

Test: platform/mac/fast/text/crash-complextextcontroller-custom-cascade-list.html

* platform/graphics/FontCascade.cpp:
(WebCore::FontCascade::FontCascade): Initialize WeakPtrFactory.
* platform/graphics/FontCascade.h:
(WebCore::FontCascade::createWeakPtr):
* platform/graphics/mac/ComplexTextControllerCoreText.mm: Migrate the raw pointer
to WeakPtr.
(-[WebCascadeList initWithFont:character:]):
(-[WebCascadeList count]):
(-[WebCascadeList objectAtIndex:]):

Source/WTF:

* wtf/WeakPtr.h:
(WTF::WeakPtrFactory::createWeakPtr): WebCascadeList uses a const FontCascade,
and it calls createWeakPtr() on it. Therefore, createWeakPtr has to be marked
const.
(WTF::WeakPtrFactory::operator=): Removed because it was broken and had no
callers

LayoutTests:

* platform/mac/fast/text/crash-complextextcontroller-custom-cascade-list-expected.txt: Added.
* platform/mac/fast/text/crash-complextextcontroller-custom-cascade-list.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/platform/mac/fast/text/crash-complextextcontroller-custom-cascade-list-expected.txt [new file with mode: 0644]
LayoutTests/platform/mac/fast/text/crash-complextextcontroller-custom-cascade-list.html [new file with mode: 0644]
Source/WTF/ChangeLog
Source/WTF/wtf/WeakPtr.h
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/FontCascade.cpp
Source/WebCore/platform/graphics/FontCascade.h
Source/WebCore/platform/graphics/mac/ComplexTextControllerCoreText.mm

index 51804b7..04747de 100644 (file)
@@ -1,3 +1,13 @@
+2015-03-06  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Crash in -[WebCascadeList objectAtIndex:] + 195
+        https://bugs.webkit.org/show_bug.cgi?id=141274
+
+        Reviewed by David Kilzer.
+
+        * platform/mac/fast/text/crash-complextextcontroller-custom-cascade-list-expected.txt: Added.
+        * platform/mac/fast/text/crash-complextextcontroller-custom-cascade-list.html: Added.
+
 2015-03-06  Said Abou-Hallawa  <sabouhallawa@apple.com>
 
         Setting any of the <object> element plugin controlling attributes does not have any affect.
diff --git a/LayoutTests/platform/mac/fast/text/crash-complextextcontroller-custom-cascade-list-expected.txt b/LayoutTests/platform/mac/fast/text/crash-complextextcontroller-custom-cascade-list-expected.txt
new file mode 100644 (file)
index 0000000..0428723
--- /dev/null
@@ -0,0 +1 @@
+Passed - no crash.
diff --git a/LayoutTests/platform/mac/fast/text/crash-complextextcontroller-custom-cascade-list.html b/LayoutTests/platform/mac/fast/text/crash-complextextcontroller-custom-cascade-list.html
new file mode 100644 (file)
index 0000000..bc87559
--- /dev/null
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <script src="../../../../resources/gc.js"></script>
+    <meta charset="utf-8"/>
+  </head>
+  <body>
+    <p>
+      <math><mi id="sin">sin</mi></math>
+      <math><mi mathvariant="bold">x</mi></math>
+    </p>
+    <p>
+      <math><mo>-</mo><mi>sin</mi><mi>x</mi></math>
+    </p>
+
+  </body>
+<script>
+  document.getElementById("sin").setAttribute("style","text-rendering:auto;");
+  document.getElementById("sin").appendChild(document.createTextNode('\u6888\u9867\uaa6f\u9b40\u7420\uf87f\u8491\ud608\u39bd\ucddb\u5d99\ud529\u8c59\u9e80'));
+  window.gc();
+  if (window.testRunner)
+    testRunner.dumpAsText();
+  document.body.innerText = "Passed - no crash.";
+</script>
+</html>
index 9eb402d..8400280 100644 (file)
@@ -1,3 +1,17 @@
+2015-03-06  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Crash in -[WebCascadeList objectAtIndex:] + 195
+        https://bugs.webkit.org/show_bug.cgi?id=141274
+
+        Reviewed by David Kilzer.
+
+        * wtf/WeakPtr.h:
+        (WTF::WeakPtrFactory::createWeakPtr): WebCascadeList uses a const FontCascade,
+        and it calls createWeakPtr() on it. Therefore, createWeakPtr has to be marked
+        const.
+        (WTF::WeakPtrFactory::operator=): Removed because it was broken and had no
+        callers
+
 2015-03-06  Simon Fraser  <simon.fraser@apple.com>
 
         Allow tree dumping functions to be used in release builds by switching a flag
index 34a1b91..03607f9 100644 (file)
@@ -99,7 +99,6 @@ public:
     operator bool() const { return m_ref->get(); }
 
     WeakPtr& operator=(const WeakPtr& o) { m_ref = o.m_ref.copyRef(); return *this; }
-    template<typename U> WeakPtr& operator=(const WeakPtr<U>& o) { m_ref = o.m_ref.copyRef(); return *this; }
     WeakPtr& operator=(std::nullptr_t) { m_ref = WeakReference<T>::create(nullptr); return *this; }
 
     T* operator->() const { return m_ref->get(); }
@@ -123,7 +122,7 @@ public:
     ~WeakPtrFactory() { m_ref->clear(); }
 
     // We should consider having createWeakPtr populate m_ref the first time createWeakPtr is called.
-    WeakPtr<T> createWeakPtr() { return WeakPtr<T>(m_ref.copyRef()); }
+    WeakPtr<T> createWeakPtr() const { return WeakPtr<T>(m_ref.copyRef()); }
 
     void revokeAll()
     {
index ba15e9c..77b054c 100644 (file)
@@ -1,5 +1,27 @@
 2015-03-06  Myles C. Maxfield  <mmaxfield@apple.com>
 
+        Crash in -[WebCascadeList objectAtIndex:] + 195
+        https://bugs.webkit.org/show_bug.cgi?id=141274
+
+        Reviewed by David Kilzer.
+
+        CTFontDescriptorRefs can live forever in caches inside CoreText, which means our
+        WebCascadeList can too.
+
+        Test: platform/mac/fast/text/crash-complextextcontroller-custom-cascade-list.html
+
+        * platform/graphics/FontCascade.cpp:
+        (WebCore::FontCascade::FontCascade): Initialize WeakPtrFactory.
+        * platform/graphics/FontCascade.h:
+        (WebCore::FontCascade::createWeakPtr):
+        * platform/graphics/mac/ComplexTextControllerCoreText.mm: Migrate the raw pointer
+        to WeakPtr.
+        (-[WebCascadeList initWithFont:character:]):
+        (-[WebCascadeList count]):
+        (-[WebCascadeList objectAtIndex:]):
+
+2015-03-06  Myles C. Maxfield  <mmaxfield@apple.com>
+
         Rename BreakingContextInlineHeaders.h to BreakingContext.h
         https://bugs.webkit.org/show_bug.cgi?id=142404
 
index cc51461..275f5f0 100644 (file)
@@ -108,7 +108,8 @@ TypesettingFeatures FontCascade::s_defaultTypesettingFeatures = 0;
 // ============================================================================================
 
 FontCascade::FontCascade()
-    : m_letterSpacing(0)
+    : m_weakPtrFactory(this)
+    , m_letterSpacing(0)
     , m_wordSpacing(0)
     , m_useBackslashAsYenSymbol(false)
     , m_typesettingFeatures(0)
@@ -117,6 +118,7 @@ FontCascade::FontCascade()
 
 FontCascade::FontCascade(const FontDescription& fd, float letterSpacing, float wordSpacing)
     : m_fontDescription(fd)
+    , m_weakPtrFactory(this)
     , m_letterSpacing(letterSpacing)
     , m_wordSpacing(wordSpacing)
     , m_useBackslashAsYenSymbol(useBackslashAsYenSignForFamily(fd.firstFamily()))
@@ -127,6 +129,7 @@ FontCascade::FontCascade(const FontDescription& fd, float letterSpacing, float w
 // FIXME: We should make this constructor platform-independent.
 FontCascade::FontCascade(const FontPlatformData& fontData, FontSmoothingMode fontSmoothingMode)
     : m_fonts(FontCascadeFonts::createForPlatformFont(fontData))
+    , m_weakPtrFactory(this)
     , m_letterSpacing(0)
     , m_wordSpacing(0)
     , m_useBackslashAsYenSymbol(false)
@@ -144,7 +147,8 @@ FontCascade::FontCascade(const FontPlatformData& fontData, FontSmoothingMode fon
 // FIXME: We should make this constructor platform-independent.
 #if PLATFORM(IOS)
 FontCascade::FontCascade(const FontPlatformData& fontData, PassRefPtr<FontSelector> fontSelector)
-    : m_letterSpacing(0)
+    : m_weakPtrFactory(this)
+    , m_letterSpacing(0)
     , m_wordSpacing(0)
     , m_typesettingFeatures(computeTypesettingFeatures())
 {
@@ -160,6 +164,7 @@ FontCascade::FontCascade(const FontPlatformData& fontData, PassRefPtr<FontSelect
 FontCascade::FontCascade(const FontCascade& other)
     : m_fontDescription(other.m_fontDescription)
     , m_fonts(other.m_fonts)
+    , m_weakPtrFactory(this)
     , m_letterSpacing(other.m_letterSpacing)
     , m_wordSpacing(other.m_wordSpacing)
     , m_useBackslashAsYenSymbol(other.m_useBackslashAsYenSymbol)
index 75d9885..7a481d2 100644 (file)
@@ -34,6 +34,7 @@
 #include "TypesettingFeatures.h"
 #include <wtf/HashMap.h>
 #include <wtf/HashSet.h>
+#include <wtf/WeakPtr.h>
 #include <wtf/unicode/CharacterNames.h>
 
 // "X11/X.h" defines Complex to 0 and conflicts
@@ -208,6 +209,8 @@ public:
 
     bool primaryFontIsSystemFont() const;
 
+    WeakPtr<FontCascade> createWeakPtr() const { return m_weakPtrFactory.createWeakPtr(); }
+
 private:
     enum ForTextEmphasisOrNot { NotForTextEmphasis, ForTextEmphasis };
 
@@ -336,6 +339,7 @@ private:
 
     FontDescription m_fontDescription;
     mutable RefPtr<FontCascadeFonts> m_fonts;
+    WeakPtrFactory<FontCascade> m_weakPtrFactory;
     float m_letterSpacing;
     float m_wordSpacing;
     mutable bool m_useBackslashAsYenSymbol;
index 79dfcd1..3f97131 100644 (file)
@@ -31,6 +31,7 @@
 #include "FontCascade.h"
 #include "TextRun.h"
 #include "WebCoreSystemInterface.h"
+#include <wtf/WeakPtr.h>
 
 #if PLATFORM(IOS)
 #include <CoreText/CoreText.h>
 #include <ApplicationServices/ApplicationServices.h>
 #endif
 
+// Note: CTFontDescriptorRefs can live forever in caches inside CoreText, so this object can too.
 @interface WebCascadeList : NSArray {
     @private
-    const WebCore::FontCascade* _font;
+    WeakPtr<WebCore::FontCascade> _font;
     UChar32 _character;
     NSUInteger _count;
     Vector<RetainPtr<CTFontDescriptorRef>, 16> _fontDescriptors;
@@ -57,7 +59,7 @@
     if (!(self = [super init]))
         return nil;
 
-    _font = font;
+    _font = font->createWeakPtr();
     _character = character;
 
     // By the time a WebCascadeList is used, the FontCascade has already been asked to realize all of its
 
 - (NSUInteger)count
 {
-    return _count;
+    return _font ? _count : 0;
 }
 
 - (id)objectAtIndex:(NSUInteger)index
 {
+    if (!_font)
+        return nil;
+
     CTFontDescriptorRef fontDescriptor;
     if (index < _fontDescriptors.size()) {
         if ((fontDescriptor = _fontDescriptors[index].get()))