Title: [181172] trunk
Revision
181172
Author
mmaxfi...@apple.com
Date
2015-03-06 11:08:55 -0800 (Fri, 06 Mar 2015)

Log Message

Crash in -[WebCascadeList objectAtIndex:] + 195
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.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (181171 => 181172)


--- trunk/LayoutTests/ChangeLog	2015-03-06 19:06:30 UTC (rev 181171)
+++ trunk/LayoutTests/ChangeLog	2015-03-06 19:08:55 UTC (rev 181172)
@@ -1,3 +1,13 @@
+2015-03-06  Myles C. Maxfield  <mmaxfi...@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  <sabouhall...@apple.com>
 
         Setting any of the <object> element plugin controlling attributes does not have any affect.

Added: trunk/LayoutTests/platform/mac/fast/text/crash-complextextcontroller-custom-cascade-list-expected.txt (0 => 181172)


--- trunk/LayoutTests/platform/mac/fast/text/crash-complextextcontroller-custom-cascade-list-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/platform/mac/fast/text/crash-complextextcontroller-custom-cascade-list-expected.txt	2015-03-06 19:08:55 UTC (rev 181172)
@@ -0,0 +1 @@
+Passed - no crash.

Added: trunk/LayoutTests/platform/mac/fast/text/crash-complextextcontroller-custom-cascade-list.html (0 => 181172)


--- trunk/LayoutTests/platform/mac/fast/text/crash-complextextcontroller-custom-cascade-list.html	                        (rev 0)
+++ trunk/LayoutTests/platform/mac/fast/text/crash-complextextcontroller-custom-cascade-list.html	2015-03-06 19:08:55 UTC (rev 181172)
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <script src=""
+    <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>

Modified: trunk/Source/WTF/ChangeLog (181171 => 181172)


--- trunk/Source/WTF/ChangeLog	2015-03-06 19:06:30 UTC (rev 181171)
+++ trunk/Source/WTF/ChangeLog	2015-03-06 19:08:55 UTC (rev 181172)
@@ -1,3 +1,17 @@
+2015-03-06  Myles C. Maxfield  <mmaxfi...@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.fra...@apple.com>
 
         Allow tree dumping functions to be used in release builds by switching a flag

Modified: trunk/Source/WTF/wtf/WeakPtr.h (181171 => 181172)


--- trunk/Source/WTF/wtf/WeakPtr.h	2015-03-06 19:06:30 UTC (rev 181171)
+++ trunk/Source/WTF/wtf/WeakPtr.h	2015-03-06 19:08:55 UTC (rev 181172)
@@ -99,7 +99,6 @@
     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 @@
     ~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()
     {

Modified: trunk/Source/WebCore/ChangeLog (181171 => 181172)


--- trunk/Source/WebCore/ChangeLog	2015-03-06 19:06:30 UTC (rev 181171)
+++ trunk/Source/WebCore/ChangeLog	2015-03-06 19:08:55 UTC (rev 181172)
@@ -1,5 +1,27 @@
 2015-03-06  Myles C. Maxfield  <mmaxfi...@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  <mmaxfi...@apple.com>
+
         Rename BreakingContextInlineHeaders.h to BreakingContext.h
         https://bugs.webkit.org/show_bug.cgi?id=142404
 

Modified: trunk/Source/WebCore/platform/graphics/FontCascade.cpp (181171 => 181172)


--- trunk/Source/WebCore/platform/graphics/FontCascade.cpp	2015-03-06 19:06:30 UTC (rev 181171)
+++ trunk/Source/WebCore/platform/graphics/FontCascade.cpp	2015-03-06 19:08:55 UTC (rev 181172)
@@ -108,7 +108,8 @@
 // ============================================================================================
 
 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(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 @@
 // 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 @@
 // 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 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)

Modified: trunk/Source/WebCore/platform/graphics/FontCascade.h (181171 => 181172)


--- trunk/Source/WebCore/platform/graphics/FontCascade.h	2015-03-06 19:06:30 UTC (rev 181171)
+++ trunk/Source/WebCore/platform/graphics/FontCascade.h	2015-03-06 19:08:55 UTC (rev 181172)
@@ -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 @@
 
     bool primaryFontIsSystemFont() const;
 
+    WeakPtr<FontCascade> createWeakPtr() const { return m_weakPtrFactory.createWeakPtr(); }
+
 private:
     enum ForTextEmphasisOrNot { NotForTextEmphasis, ForTextEmphasis };
 
@@ -336,6 +339,7 @@
 
     FontDescription m_fontDescription;
     mutable RefPtr<FontCascadeFonts> m_fonts;
+    WeakPtrFactory<FontCascade> m_weakPtrFactory;
     float m_letterSpacing;
     float m_wordSpacing;
     mutable bool m_useBackslashAsYenSymbol;

Modified: trunk/Source/WebCore/platform/graphics/mac/ComplexTextControllerCoreText.mm (181171 => 181172)


--- trunk/Source/WebCore/platform/graphics/mac/ComplexTextControllerCoreText.mm	2015-03-06 19:06:30 UTC (rev 181171)
+++ trunk/Source/WebCore/platform/graphics/mac/ComplexTextControllerCoreText.mm	2015-03-06 19:08:55 UTC (rev 181172)
@@ -31,6 +31,7 @@
 #include "FontCascade.h"
 #include "TextRun.h"
 #include "WebCoreSystemInterface.h"
+#include <wtf/WeakPtr.h>
 
 #if PLATFORM(IOS)
 #include <CoreText/CoreText.h>
@@ -38,9 +39,10 @@
 #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
@@ -70,11 +72,14 @@
 
 - (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()))
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to