[Enchant] Clean up TextCheckerEnchant
authorcarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 3 Apr 2018 07:16:45 +0000 (07:16 +0000)
committercarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 3 Apr 2018 07:16:45 +0000 (07:16 +0000)
https://bugs.webkit.org/show_bug.cgi?id=184233

Reviewed by Michael Catanzaro.

Source/WebCore:

Several cleanups and improvements:

* platform/text/enchant/TextCheckerEnchant.cpp:
(WebCore::TextCheckerEnchant::singleton): Make TextCheckerEnchant a singleton since it's always used as a
singleton.
(WebCore::TextCheckerEnchant::EnchantDictDeleter::operator() const):
(WebCore::TextCheckerEnchant::TextCheckerEnchant):
(WebCore::TextCheckerEnchant::ignoreWord): Convert to utf8 once instead of on every loop iteration.
(WebCore::TextCheckerEnchant::learnWord): Ditton.
(WebCore::TextCheckerEnchant::checkSpellingOfWord): m_enchantDictionaries is now a Vector of std::unique_ptr.
(WebCore::TextCheckerEnchant::getGuessesForWord): Convert to utf8 once instead of on every loop iteration.
(WebCore::TextCheckerEnchant::updateSpellCheckingLanguages): Get only the first language instead of building a
vector to get its first item. Use WTFMove to replace m_enchantDictionaries with the new Vector.
(WebCore::TextCheckerEnchant::loadedSpellCheckingLanguages const): Use a lambda to get the list of languages
already converted to String and iterate only once.
(WebCore::TextCheckerEnchant::availableSpellCheckingLanguages const): Ditto.
(WebCore::enchantDictDescribeCallback): Deleted.
(WebCore::TextCheckerEnchant::~TextCheckerEnchant): Deleted.
(WebCore::TextCheckerEnchant::freeEnchantBrokerDictionaries): Deleted.
* platform/text/enchant/TextCheckerEnchant.h:

Source/WebKit:

Use TextCheckerEnchant as a singleton now, instead of implementing the singleton here.

* UIProcess/gtk/TextCheckerGtk.cpp:
(WebKit::TextChecker::checkSpellingOfString):
(WebKit::TextChecker::getGuessesForWord):
(WebKit::TextChecker::learnWord):
(WebKit::TextChecker::ignoreWord):
(WebKit::TextChecker::setSpellCheckingLanguages):
(WebKit::TextChecker::loadedSpellCheckingLanguages):
(WebKit::enchantTextChecker): Deleted.

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

Source/WebCore/ChangeLog
Source/WebCore/platform/text/enchant/TextCheckerEnchant.cpp
Source/WebCore/platform/text/enchant/TextCheckerEnchant.h
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/gtk/TextCheckerGtk.cpp

index 7da2cd2..d258b19 100644 (file)
@@ -1,3 +1,31 @@
+2018-04-02  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [Enchant] Clean up TextCheckerEnchant
+        https://bugs.webkit.org/show_bug.cgi?id=184233
+
+        Reviewed by Michael Catanzaro.
+
+        Several cleanups and improvements:
+
+        * platform/text/enchant/TextCheckerEnchant.cpp:
+        (WebCore::TextCheckerEnchant::singleton): Make TextCheckerEnchant a singleton since it's always used as a
+        singleton.
+        (WebCore::TextCheckerEnchant::EnchantDictDeleter::operator() const):
+        (WebCore::TextCheckerEnchant::TextCheckerEnchant):
+        (WebCore::TextCheckerEnchant::ignoreWord): Convert to utf8 once instead of on every loop iteration.
+        (WebCore::TextCheckerEnchant::learnWord): Ditton.
+        (WebCore::TextCheckerEnchant::checkSpellingOfWord): m_enchantDictionaries is now a Vector of std::unique_ptr.
+        (WebCore::TextCheckerEnchant::getGuessesForWord): Convert to utf8 once instead of on every loop iteration.
+        (WebCore::TextCheckerEnchant::updateSpellCheckingLanguages): Get only the first language instead of building a
+        vector to get its first item. Use WTFMove to replace m_enchantDictionaries with the new Vector.
+        (WebCore::TextCheckerEnchant::loadedSpellCheckingLanguages const): Use a lambda to get the list of languages
+        already converted to String and iterate only once.
+        (WebCore::TextCheckerEnchant::availableSpellCheckingLanguages const): Ditto.
+        (WebCore::enchantDictDescribeCallback): Deleted.
+        (WebCore::TextCheckerEnchant::~TextCheckerEnchant): Deleted.
+        (WebCore::TextCheckerEnchant::freeEnchantBrokerDictionaries): Deleted.
+        * platform/text/enchant/TextCheckerEnchant.h:
+
 2018-04-03  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         [GTK] Segfaults in enchant_broker_free_dict()
index c926c1f..86f1253 100644 (file)
 
 #if ENABLE(SPELLCHECK)
 
-#include <glib.h>
 #include <unicode/ubrk.h>
 #include <wtf/Language.h>
+#include <wtf/NeverDestroyed.h>
+#include <wtf/text/CString.h>
 #include <wtf/text/TextBreakIterator.h>
+#include <wtf/text/WTFString.h>
 
 namespace WebCore {
 
-static const size_t maximumNumberOfSuggestions = 10;
-
-static void enchantDictDescribeCallback(const char* const languageTag, const char* const, const char* const, const char* const, void* data)
+TextCheckerEnchant& TextCheckerEnchant::singleton()
 {
-    Vector<CString>* dictionaries = static_cast<Vector<CString>*>(data);
-    dictionaries->append(languageTag);
+    static NeverDestroyed<TextCheckerEnchant> textChecker;
+    return textChecker;
 }
 
-TextCheckerEnchant::TextCheckerEnchant()
-    : m_broker(enchant_broker_init())
-    , m_enchantDictionaries(0)
+void TextCheckerEnchant::EnchantDictDeleter::operator()(EnchantDict* dictionary) const
 {
+    enchant_broker_free_dict(TextCheckerEnchant::singleton().m_broker, dictionary);
 }
 
-TextCheckerEnchant::~TextCheckerEnchant()
+TextCheckerEnchant::TextCheckerEnchant()
+    : m_broker(enchant_broker_init())
 {
-    if (!m_broker)
-        return;
-
-    freeEnchantBrokerDictionaries();
-    enchant_broker_free(m_broker);
 }
 
 void TextCheckerEnchant::ignoreWord(const String& word)
 {
+    auto utf8Word = word.utf8();
     for (auto& dictionary : m_enchantDictionaries)
-        enchant_dict_add_to_session(dictionary, word.utf8().data(), -1);
+        enchant_dict_add_to_session(dictionary.get(), utf8Word.data(), utf8Word.length());
 }
 
 void TextCheckerEnchant::learnWord(const String& word)
 {
+    auto utf8Word = word.utf8();
     for (auto& dictionary : m_enchantDictionaries)
-        enchant_dict_add(dictionary, word.utf8().data(), -1);
+        enchant_dict_add(dictionary.get(), utf8Word.data(), utf8Word.length());
 }
 
 void TextCheckerEnchant::checkSpellingOfWord(const String& word, int start, int end, int& misspellingLocation, int& misspellingLength)
@@ -69,7 +66,7 @@ void TextCheckerEnchant::checkSpellingOfWord(const String& word, int start, int
     CString string = word.substring(start, end - start).utf8();
 
     for (auto& dictionary : m_enchantDictionaries) {
-        if (!enchant_dict_check(dictionary, string.data(), string.length())) {
+        if (!enchant_dict_check(dictionary.get(), string.data(), string.length())) {
             // Stop checking, this word is ok in at least one dict.
             misspellingLocation = -1;
             misspellingLength = 0;
@@ -108,15 +105,18 @@ void TextCheckerEnchant::checkSpellingOfString(const String& string, int& misspe
 
 Vector<String> TextCheckerEnchant::getGuessesForWord(const String& word)
 {
-    Vector<String> guesses;
     if (!hasDictionary())
-        return guesses;
+        return { };
+
+    static const size_t maximumNumberOfSuggestions = 10;
 
+    Vector<String> guesses;
+    auto utf8Word = word.utf8();
     for (auto& dictionary : m_enchantDictionaries) {
         size_t numberOfSuggestions;
         size_t i;
 
-        char** suggestions = enchant_dict_suggest(dictionary, word.utf8().data(), -1, &numberOfSuggestions);
+        char** suggestions = enchant_dict_suggest(dictionary.get(), utf8Word.data(), utf8Word.length(), &numberOfSuggestions);
         if (numberOfSuggestions <= 0)
             continue;
 
@@ -126,7 +126,7 @@ Vector<String> TextCheckerEnchant::getGuessesForWord(const String& word)
         for (i = 0; i < numberOfSuggestions; i++)
             guesses.append(String::fromUTF8(suggestions[i]));
 
-        enchant_dict_free_string_list(dictionary, suggestions);
+        enchant_dict_free_string_list(dictionary.get(), suggestions);
     }
 
     return guesses;
@@ -134,8 +134,7 @@ Vector<String> TextCheckerEnchant::getGuessesForWord(const String& word)
 
 void TextCheckerEnchant::updateSpellCheckingLanguages(const Vector<String>& languages)
 {
-    Vector<EnchantDict*> spellDictionaries;
-
+    Vector<UniqueEnchantDict> spellDictionaries;
     if (!languages.isEmpty()) {
         for (auto& language : languages) {
             CString currentLanguage = language.utf8();
@@ -146,60 +145,52 @@ void TextCheckerEnchant::updateSpellCheckingLanguages(const Vector<String>& lang
         }
     } else {
         // Languages are not specified by user, try to get default language.
-        CString utf8Language = defaultLanguage().utf8();
-        const char* language = utf8Language.data();
-        if (enchant_broker_dict_exists(m_broker, language)) {
-            if (auto* dict = enchant_broker_request_dict(m_broker, language))
+        CString language = defaultLanguage().utf8();
+        if (enchant_broker_dict_exists(m_broker, language.data())) {
+            if (auto* dict = enchant_broker_request_dict(m_broker, language.data()))
                 spellDictionaries.append(dict);
         } else {
             // No dictionaries selected, we get the first one from the list.
-            Vector<CString> allDictionaries;
-            enchant_broker_list_dicts(m_broker, enchantDictDescribeCallback, &allDictionaries);
-            if (!allDictionaries.isEmpty()) {
-                if (auto* dict = enchant_broker_request_dict(m_broker, allDictionaries.first().data()))
+            CString dictLanguage;
+            enchant_broker_list_dicts(m_broker, [](const char* const languageTag, const char* const, const char* const, const char* const, void* data) {
+                auto* dictLanguage = static_cast<CString*>(data);
+                if (dictLanguage->isNull())
+                    *dictLanguage = languageTag;
+            }, &dictLanguage);
+            if (!dictLanguage.isNull()) {
+                if (auto* dict = enchant_broker_request_dict(m_broker, dictLanguage.data()))
                     spellDictionaries.append(dict);
             }
         }
     }
-    freeEnchantBrokerDictionaries();
-    m_enchantDictionaries = spellDictionaries;
+    m_enchantDictionaries = WTFMove(spellDictionaries);
 }
 
 Vector<String> TextCheckerEnchant::loadedSpellCheckingLanguages() const
 {
-    Vector<String> languages;
     if (!hasDictionary())
-        return languages;
-
-    // Get a Vector<CString> with the list of languages in use.
-    Vector<CString> currentDictionaries;
-    for (auto& dictionary : m_enchantDictionaries)
-        enchant_dict_describe(dictionary, enchantDictDescribeCallback, &currentDictionaries);
-
-    for (auto& dictionary : currentDictionaries)
-        languages.append(String::fromUTF8(dictionary.data()));
+        return { };
 
+    Vector<String> languages;
+    for (auto& dictionary : m_enchantDictionaries) {
+        enchant_dict_describe(dictionary.get(), [](const char* const languageTag, const char* const, const char* const, const char* const, void* data) {
+            auto* languages = static_cast<Vector<String>*>(data);
+            languages->append(String::fromUTF8(languageTag));
+        }, &languages);
+    }
     return languages;
 }
 
 Vector<String> TextCheckerEnchant::availableSpellCheckingLanguages() const
 {
-    Vector<CString> allDictionaries;
-    enchant_broker_list_dicts(m_broker, enchantDictDescribeCallback, &allDictionaries);
-
     Vector<String> languages;
-    for (auto& dictionary : allDictionaries)
-        languages.append(String::fromUTF8(dictionary.data()));
-
+    enchant_broker_list_dicts(m_broker, [](const char* const languageTag, const char* const, const char* const, const char* const, void* data) {
+        auto* languages = static_cast<Vector<String>*>(data);
+        languages->append(String::fromUTF8(languageTag));
+    }, &languages);
     return languages;
 }
 
-void TextCheckerEnchant::freeEnchantBrokerDictionaries()
-{
-    for (auto& dictionary : m_enchantDictionaries)
-        enchant_broker_free_dict(m_broker, dictionary);
-}
-
 } // namespace WebCore
 
 #endif // ENABLE(SPELLCHECK)
index d3d5293..8413df5 100644 (file)
  * Boston, MA 02110-1301, USA.
  */
 
-#ifndef TextCheckerEnchant_h
-#define TextCheckerEnchant_h
+#pragma once
 
 #if ENABLE(SPELLCHECK)
 
 #include <enchant.h>
-#include <wtf/FastMalloc.h>
+#include <wtf/Forward.h>
 #include <wtf/Vector.h>
-#include <wtf/text/CString.h>
-#include <wtf/text/WTFString.h>
 
 namespace WebCore {
 
 class TextCheckerEnchant {
-    WTF_MAKE_FAST_ALLOCATED;
-
+    WTF_MAKE_NONCOPYABLE(TextCheckerEnchant); WTF_MAKE_FAST_ALLOCATED;
+    friend class WTF::NeverDestroyed<TextCheckerEnchant>;
 public:
-    TextCheckerEnchant();
-    virtual ~TextCheckerEnchant();
+    static TextCheckerEnchant& singleton();
 
     void ignoreWord(const String&);
     void learnWord(const String&);
@@ -47,15 +43,21 @@ public:
     Vector<String> availableSpellCheckingLanguages() const;
 
 private:
-    void freeEnchantBrokerDictionaries();
+    TextCheckerEnchant();
+    ~TextCheckerEnchant() = delete;
+
     void checkSpellingOfWord(const String&, int start, int end, int& misspellingLocation, int& misspellingLength);
 
+    struct EnchantDictDeleter {
+        void operator()(EnchantDict*) const;
+    };
+
+    using UniqueEnchantDict = std::unique_ptr<EnchantDict, EnchantDictDeleter>;
+
     EnchantBroker* m_broker;
-    Vector<EnchantDict*> m_enchantDictionaries;
+    Vector<UniqueEnchantDict> m_enchantDictionaries;
 };
 
 } // namespace WebCore
 
 #endif // ENABLE(SPELLCHECK)
-
-#endif
index cf482f0..4d1e378 100644 (file)
@@ -1,3 +1,21 @@
+2018-04-02  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [Enchant] Clean up TextCheckerEnchant
+        https://bugs.webkit.org/show_bug.cgi?id=184233
+
+        Reviewed by Michael Catanzaro.
+
+        Use TextCheckerEnchant as a singleton now, instead of implementing the singleton here.
+
+        * UIProcess/gtk/TextCheckerGtk.cpp:
+        (WebKit::TextChecker::checkSpellingOfString):
+        (WebKit::TextChecker::getGuessesForWord):
+        (WebKit::TextChecker::learnWord):
+        (WebKit::TextChecker::ignoreWord):
+        (WebKit::TextChecker::setSpellCheckingLanguages):
+        (WebKit::TextChecker::loadedSpellCheckingLanguages):
+        (WebKit::enchantTextChecker): Deleted.
+
 2018-04-03  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         [GTK] NetworkProcess from WebKitGtk+ 2.19.9x SIGSEVs in NetworkStorageSession (secret search callback)
index b5d85c7..faff092 100644 (file)
 namespace WebKit {
 using namespace WebCore;
 
-#if ENABLE(SPELLCHECK)
-static WebCore::TextCheckerEnchant& enchantTextChecker()
-{
-    static NeverDestroyed<WebCore::TextCheckerEnchant> checker;
-    return checker;
-}
-#endif
-
 TextCheckerState& checkerState()
 {
     static TextCheckerState textCheckerState;
@@ -149,7 +141,7 @@ void TextChecker::checkSpellingOfString(int64_t /* spellDocumentTag */, StringVi
 #if ENABLE(SPELLCHECK)
     misspellingLocation = -1;
     misspellingLength = 0;
-    enchantTextChecker().checkSpellingOfString(text.toStringWithoutCopying(), misspellingLocation, misspellingLength);
+    TextCheckerEnchant::singleton().checkSpellingOfString(text.toStringWithoutCopying(), misspellingLocation, misspellingLength);
 #else
     UNUSED_PARAM(text);
     UNUSED_PARAM(misspellingLocation);
@@ -181,7 +173,7 @@ void TextChecker::updateSpellingUIWithGrammarString(int64_t /* spellDocumentTag
 void TextChecker::getGuessesForWord(int64_t /* spellDocumentTag */, const String& word, const String& /* context */, int32_t /* insertionPoint */, Vector<String>& guesses, bool)
 {
 #if ENABLE(SPELLCHECK)
-    guesses = enchantTextChecker().getGuessesForWord(word);
+    guesses = TextCheckerEnchant::singleton().getGuessesForWord(word);
 #else
     UNUSED_PARAM(word);
     UNUSED_PARAM(guesses);
@@ -191,7 +183,7 @@ void TextChecker::getGuessesForWord(int64_t /* spellDocumentTag */, const String
 void TextChecker::learnWord(int64_t /* spellDocumentTag */, const String& word)
 {
 #if ENABLE(SPELLCHECK)
-    enchantTextChecker().learnWord(word);
+    TextCheckerEnchant::singleton().learnWord(word);
 #else
     UNUSED_PARAM(word);
 #endif
@@ -200,7 +192,7 @@ void TextChecker::learnWord(int64_t /* spellDocumentTag */, const String& word)
 void TextChecker::ignoreWord(int64_t /* spellDocumentTag */, const String& word)
 {
 #if ENABLE(SPELLCHECK)
-    enchantTextChecker().ignoreWord(word);
+    TextCheckerEnchant::singleton().ignoreWord(word);
 #else
     UNUSED_PARAM(word);
 #endif
@@ -296,7 +288,7 @@ Vector<TextCheckingResult> TextChecker::checkTextOfParagraph(int64_t spellDocume
 void TextChecker::setSpellCheckingLanguages(const Vector<String>& languages)
 {
 #if ENABLE(SPELLCHECK)
-    enchantTextChecker().updateSpellCheckingLanguages(languages);
+    TextCheckerEnchant::singleton().updateSpellCheckingLanguages(languages);
 #else
     UNUSED_PARAM(languages);
 #endif
@@ -305,7 +297,7 @@ void TextChecker::setSpellCheckingLanguages(const Vector<String>& languages)
 Vector<String> TextChecker::loadedSpellCheckingLanguages()
 {
 #if ENABLE(SPELLCHECK)
-    return enchantTextChecker().loadedSpellCheckingLanguages();
+    return TextCheckerEnchant::singleton().loadedSpellCheckingLanguages();
 #else
     return Vector<String>();
 #endif