Title: [213093] trunk/Source
Revision
213093
Author
mmaxfi...@apple.com
Date
2017-02-27 14:39:31 -0800 (Mon, 27 Feb 2017)

Log Message

Use RAII for ICU breaking iterators
https://bugs.webkit.org/show_bug.cgi?id=168203

Reviewed by Simon Fraser.

Source/WebCore:

No new tests because there is no behavior change.

* platform/graphics/ComplexTextController.cpp:
(WebCore::ComplexTextController::offsetForPosition):
* rendering/RenderText.cpp:
(WebCore::RenderText::previousOffset):
(WebCore::RenderText::nextOffset):

Source/WTF:

* wtf/text/TextBreakIterator.h:
(WTF::CachedTextBreakIterator::CachedTextBreakIterator):
(WTF::CachedTextBreakIterator::~CachedTextBreakIterator):
(WTF::CachedTextBreakIterator::preceding):
(WTF::CachedTextBreakIterator::following):
(WTF::CachedTextBreakIterator::isBoundary):

Modified Paths

Diff

Modified: trunk/Source/WTF/ChangeLog (213092 => 213093)


--- trunk/Source/WTF/ChangeLog	2017-02-27 22:36:48 UTC (rev 213092)
+++ trunk/Source/WTF/ChangeLog	2017-02-27 22:39:31 UTC (rev 213093)
@@ -1,3 +1,17 @@
+2017-02-27  Myles C. Maxfield  <mmaxfi...@apple.com>
+
+        Use RAII for ICU breaking iterators
+        https://bugs.webkit.org/show_bug.cgi?id=168203
+
+        Reviewed by Simon Fraser.
+
+        * wtf/text/TextBreakIterator.h:
+        (WTF::CachedTextBreakIterator::CachedTextBreakIterator):
+        (WTF::CachedTextBreakIterator::~CachedTextBreakIterator):
+        (WTF::CachedTextBreakIterator::preceding):
+        (WTF::CachedTextBreakIterator::following):
+        (WTF::CachedTextBreakIterator::isBoundary):
+
 2017-02-24  Jer Noble  <jer.no...@apple.com>
 
         Add public method to MediaTime for doing timeScale conversion.

Modified: trunk/Source/WTF/wtf/text/TextBreakIterator.h (213092 => 213093)


--- trunk/Source/WTF/wtf/text/TextBreakIterator.h	2017-02-27 22:36:48 UTC (rev 213092)
+++ trunk/Source/WTF/wtf/text/TextBreakIterator.h	2017-02-27 22:39:31 UTC (rev 213093)
@@ -80,7 +80,7 @@
 private:
     friend class TextBreakIteratorCache;
 
-    // Use TextBreakIteratorCache instead of constructing one of these directly.
+    // Use CachedTextBreakIterator instead of constructing one of these directly.
     WTF_EXPORT TextBreakIterator(StringView, Mode, const AtomicString& locale);
 
     void setText(StringView string)
@@ -105,8 +105,14 @@
     AtomicString m_locale;
 };
 
+class CachedTextBreakIterator;
+
 class TextBreakIteratorCache {
-public:
+// Use CachedTextBreakIterator instead of dealing with the cache directly.
+private:
+    friend class NeverDestroyed<TextBreakIteratorCache>;
+    friend class CachedTextBreakIterator;
+
     static TextBreakIteratorCache& singleton()
     {
         static NeverDestroyed<TextBreakIteratorCache> cache;
@@ -139,17 +145,53 @@
             m_unused.remove(0);
     }
 
-private:
-    friend class NeverDestroyed<TextBreakIteratorCache>;
-
     TextBreakIteratorCache()
     {
     }
 
     static constexpr int capacity = 2;
+    // FIXME: Break this up into different Vectors per mode.
     Vector<TextBreakIterator, capacity> m_unused;
 };
 
+// RAII for TextBreakIterator and TextBreakIteratorCache.
+class CachedTextBreakIterator {
+public:
+    CachedTextBreakIterator(StringView string, TextBreakIterator::Mode mode, const AtomicString& locale)
+        : m_backing(TextBreakIteratorCache::singleton().take(string, mode, locale))
+    {
+    }
+
+    ~CachedTextBreakIterator()
+    {
+        TextBreakIteratorCache::singleton().put(WTFMove(m_backing));
+    }
+
+    CachedTextBreakIterator() = delete;
+    CachedTextBreakIterator(const CachedTextBreakIterator&) = delete;
+    CachedTextBreakIterator(CachedTextBreakIterator&&) = default;
+    CachedTextBreakIterator& operator=(const CachedTextBreakIterator&) = delete;
+    CachedTextBreakIterator& operator=(CachedTextBreakIterator&&) = default;
+
+    std::optional<unsigned> preceding(unsigned location) const
+    {
+        return m_backing.preceding(location);
+    }
+
+    std::optional<unsigned> following(unsigned location) const
+    {
+        return m_backing.following(location);
+    }
+
+    bool isBoundary(unsigned location) const
+    {
+        return m_backing.isBoundary(location);
+    }
+
+private:
+    TextBreakIterator m_backing;
+};
+
 // Note: The returned iterator is good only until you get another iterator, with the exception of acquireLineBreakIterator.
 
 enum class LineBreakIteratorMode { Default, Loose, Normal, Strict };
@@ -304,6 +346,7 @@
 
 }
 
+using WTF::CachedTextBreakIterator;
 using WTF::LazyLineBreakIterator;
 using WTF::LineBreakIteratorMode;
 using WTF::NonSharedCharacterBreakIterator;

Modified: trunk/Source/WebCore/ChangeLog (213092 => 213093)


--- trunk/Source/WebCore/ChangeLog	2017-02-27 22:36:48 UTC (rev 213092)
+++ trunk/Source/WebCore/ChangeLog	2017-02-27 22:39:31 UTC (rev 213093)
@@ -1,3 +1,18 @@
+2017-02-27  Myles C. Maxfield  <mmaxfi...@apple.com>
+
+        Use RAII for ICU breaking iterators
+        https://bugs.webkit.org/show_bug.cgi?id=168203
+
+        Reviewed by Simon Fraser.
+
+        No new tests because there is no behavior change.
+
+        * platform/graphics/ComplexTextController.cpp:
+        (WebCore::ComplexTextController::offsetForPosition):
+        * rendering/RenderText.cpp:
+        (WebCore::RenderText::previousOffset):
+        (WebCore::RenderText::nextOffset):
+
 2017-02-27  Basuke Suzuki  <basuke.suz...@am.sony.com>
 
         [WinCairo][MiniBrowser] Add ca-bundle to display secure pages

Modified: trunk/Source/WebCore/platform/graphics/ComplexTextController.cpp (213092 => 213093)


--- trunk/Source/WebCore/platform/graphics/ComplexTextController.cpp	2017-02-27 22:36:48 UTC (rev 213092)
+++ trunk/Source/WebCore/platform/graphics/ComplexTextController.cpp	2017-02-27 22:39:31 UTC (rev 213093)
@@ -217,7 +217,7 @@
                 // could use the glyph's "ligature carets". This is available in CoreText via CTFontGetLigatureCaretPositions().
                 unsigned hitIndex = hitGlyphStart + (hitGlyphEnd - hitGlyphStart) * (m_run.ltr() ? x / adjustedAdvance : 1 - x / adjustedAdvance);
                 unsigned stringLength = complexTextRun.stringLength();
-                TextBreakIterator cursorPositionIterator = TextBreakIteratorCache::singleton().take(StringView(complexTextRun.characters(), stringLength), TextBreakIterator::Mode::Cursor, nullAtom);
+                CachedTextBreakIterator cursorPositionIterator(StringView(complexTextRun.characters(), stringLength), TextBreakIterator::Mode::Cursor, nullAtom);
                 unsigned clusterStart;
                 if (cursorPositionIterator.isBoundary(hitIndex))
                     clusterStart = hitIndex;
@@ -229,8 +229,6 @@
 
                 unsigned clusterEnd = cursorPositionIterator.following(hitIndex).value_or(stringLength);
 
-                TextBreakIteratorCache::singleton().put(WTFMove(cursorPositionIterator));
-
                 float clusterWidth;
                 // FIXME: The search stops at the boundaries of complexTextRun. In theory, it should go on into neighboring ComplexTextRuns
                 // derived from the same CTLine. In practice, we do not expect there to be more than one CTRun in a CTLine, as no

Modified: trunk/Source/WebCore/rendering/RenderText.cpp (213092 => 213093)


--- trunk/Source/WebCore/rendering/RenderText.cpp	2017-02-27 22:36:48 UTC (rev 213092)
+++ trunk/Source/WebCore/rendering/RenderText.cpp	2017-02-27 22:39:31 UTC (rev 213093)
@@ -1505,9 +1505,8 @@
         return current - 1;
 
     StringImpl* textImpl = m_text.impl();
-    TextBreakIterator iterator = TextBreakIteratorCache::singleton().take(StringView(textImpl->characters16(), textImpl->length()), TextBreakIterator::Mode::Cursor, nullAtom);
+    CachedTextBreakIterator iterator(StringView(textImpl->characters16(), textImpl->length()), TextBreakIterator::Mode::Cursor, nullAtom);
     auto result = iterator.preceding(current).value_or(current - 1);
-    TextBreakIteratorCache::singleton().put(WTFMove(iterator));
     return result;
 }
 
@@ -1679,9 +1678,8 @@
         return current + 1;
 
     StringImpl* textImpl = m_text.impl();
-    TextBreakIterator iterator = TextBreakIteratorCache::singleton().take(StringView(textImpl->characters16(), textImpl->length()), TextBreakIterator::Mode::Cursor, nullAtom);
+    CachedTextBreakIterator iterator(StringView(textImpl->characters16(), textImpl->length()), TextBreakIterator::Mode::Cursor, nullAtom);
     auto result = iterator.following(current).value_or(current + 1);
-    TextBreakIteratorCache::singleton().put(WTFMove(iterator));
     return result;
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to