Skip to content

Commit

Permalink
Merge r235120 - Replace TextCheckingTypeMask with OptionSet
Browse files Browse the repository at this point in the history 
https://bugs.webkit.org/show_bug.cgi?id=188678

Reviewed by Antti Koivisto.

Source/WebCore:

Replaces TextCheckingTypeMask with an OptionSet to improve type safety and code clarity. Additionally
change the values of TextCheckingType such that all the enumerators fit within an uint8_t.

* PlatformMac.cmake:
* SourcesCocoa.txt:
* WebCore.xcodeproj/project.pbxproj:
* accessibility/AccessibilityObject.cpp:
(WebCore::AccessibilityObject::hasMisspelling const):
* accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
(AXAttributeStringSetSpelling):
* editing/AlternativeTextController.cpp:
(WebCore::AlternativeTextController::timerFired):
(WebCore::AlternativeTextController::processMarkersOnTextToBeReplacedByResult):
* editing/Editor.cpp:
(WebCore::Editor::replaceSelectionWithFragment):
(WebCore::Editor::markMisspellingsAfterTypingToWord):
(WebCore::Editor::markAllMisspellingsAndBadGrammarInRanges):
(WebCore::isAutomaticTextReplacementType):
(WebCore::Editor::markAndReplaceFor): For now, change a local variable from const to non-const to work
around the following MSVC compiler bug: <https://developercommunity.visualstudio.com/content/problem/316713/msvc-cant-compile-webkits-optionsetcontainsany.html>.
(WebCore::Editor::markMisspellingsAndBadGrammar):
(WebCore::Editor::updateMarkersForWordsAffectedByEditing):
(WebCore::Editor::editorUIUpdateTimerFired):
(WebCore::Editor::resolveTextCheckingTypeMask):
* editing/Editor.h:
* editing/SpellChecker.cpp:
(WebCore::SpellCheckRequest::SpellCheckRequest):
(WebCore::SpellCheckRequest::create):
(WebCore::SpellChecker::didCheckSucceed):
* editing/SpellChecker.h:
* editing/TextCheckingHelper.cpp:
(WebCore::findGrammaticalErrors):
(WebCore::findMisspellings):
(WebCore::TextCheckingHelper::findFirstMisspellingOrBadGrammar):
(WebCore::TextCheckingHelper::guessesForMisspelledOrUngrammaticalRange const):
(WebCore::checkTextOfParagraph):
* editing/TextCheckingHelper.h:
* loader/EmptyClients.cpp:
* platform/text/TextCheckerClient.h:
* platform/text/TextChecking.h: Remove TextCheckingTypeMask. Reorganized the fields of TextCheckingRequestData
to coallesce padding and move it to the end of class. Also used default initializer syntax and defaulted (= default)
the default constructor of TextCheckingRequestData, removing the need for a user-defined default constructor.
(WebCore::TextCheckingRequestData::TextCheckingRequestData):
(WebCore::TextCheckingRequestData::text const): Changed return type from String to const String&
to avoid unnecessary ref-count churn for callers that do not need to take a shared ownership in
this string.
(WebCore::TextCheckingRequestData::checkingTypes const): Renamed; formerly named mask.
(WebCore::TextCheckingRequestData::mask const): Deleted.
* platform/text/mac/TextCheckingMac.mm: Added.
(WebCore::nsTextCheckingTypes):
* testing/Internals.cpp:
(WebCore::Internals::handleAcceptedCandidate):

Source/WebKit:

* Scripts/webkit/messages.py: Add WebCore::TextCheckingType to the special case map so that
the generator knows what header has the definition for this type.
* Shared/WebCoreArgumentCoders.cpp:
(IPC::ArgumentCoder<TextCheckingRequestData>::encode):
(IPC::ArgumentCoder<TextCheckingRequestData>::decode):
* UIProcess/Cocoa/WebViewImpl.mm:
(WebKit::coreTextCheckingType):
(WebKit::textCheckingResultFromNSTextCheckingResult):
* UIProcess/TextChecker.h:
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::checkTextOfParagraph):
* UIProcess/WebPageProxy.h:
* UIProcess/WebPageProxy.messages.in:
* UIProcess/gtk/TextCheckerGtk.cpp:
(WebKit::TextChecker::requestCheckingOfString):
(WebKit::TextChecker::checkTextOfParagraph): Also simplified return expressions.
* UIProcess/ios/TextCheckerIOS.mm:
(WebKit::TextChecker::checkTextOfParagraph):
* UIProcess/mac/TextCheckerMac.mm:
(WebKit::TextChecker::checkTextOfParagraph):
* UIProcess/win/TextCheckerWin.cpp:
(WebKit::TextChecker::checkTextOfParagraph):
* WebProcess/WebCoreSupport/WebEditorClient.cpp:
(WebKit::WebEditorClient::shouldEraseMarkersAfterChangeSelection const):
(WebKit::WebEditorClient::checkTextOfParagraph):
* WebProcess/WebCoreSupport/WebEditorClient.h:

Source/WebKitLegacy/mac:

Currently we have code in WebEditorClient::checkTextOfParagraph() that incorrectly assumes
that the enumerators of TextCheckingType have a one-to-one correspondence with NSTextCheckingType.
(This is not the case because there is not corresponding NSTextCheckingType for TextCheckingTypeShowCorrectionPanel).
We now explicitly convert from OptionSet<TextCheckingType> to NSTextCheckingTypes.

* WebCoreSupport/WebEditorClient.h:
* WebCoreSupport/WebEditorClient.mm:
(WebEditorClient::checkTextOfParagraph):
(WebEditorClient::shouldEraseMarkersAfterChangeSelection const):
(core): Fix up code style nits; compare resultType on the right-hand side instead of the
left as this is more readable and unncessary now that modern compilers like Clang have
diagnostics to catch accidental assignments when equality was intended.
(WebEditorClient::didCheckSucceed):
* WebView/WebView.mm:
(coreTextCheckingType):
(textCheckingResultFromNSTextCheckingResult):
  • Loading branch information
@dydz @carlosgcampos
dydz authored and carlosgcampos committed Aug 24, 2018
1 parent 96580fd commit 8a1bcff
Show file tree
Hide file tree
Showing 36 changed files with 393 additions and 211 deletions.
60 changes: 60 additions & 0 deletions Source/WebCore/ChangeLog
View file
@@ -1,3 +1,63 @@
2018-08-21 Daniel Bates <dabates@apple.com>

Replace TextCheckingTypeMask with OptionSet
https://bugs.webkit.org/show_bug.cgi?id=188678

Reviewed by Antti Koivisto.

Replaces TextCheckingTypeMask with an OptionSet to improve type safety and code clarity. Additionally
change the values of TextCheckingType such that all the enumerators fit within an uint8_t.

* PlatformMac.cmake:
* SourcesCocoa.txt:
* WebCore.xcodeproj/project.pbxproj:
* accessibility/AccessibilityObject.cpp:
(WebCore::AccessibilityObject::hasMisspelling const):
* accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
(AXAttributeStringSetSpelling):
* editing/AlternativeTextController.cpp:
(WebCore::AlternativeTextController::timerFired):
(WebCore::AlternativeTextController::processMarkersOnTextToBeReplacedByResult):
* editing/Editor.cpp:
(WebCore::Editor::replaceSelectionWithFragment):
(WebCore::Editor::markMisspellingsAfterTypingToWord):
(WebCore::Editor::markAllMisspellingsAndBadGrammarInRanges):
(WebCore::isAutomaticTextReplacementType):
(WebCore::Editor::markAndReplaceFor): For now, change a local variable from const to non-const to work
around the following MSVC compiler bug: <https://developercommunity.visualstudio.com/content/problem/316713/msvc-cant-compile-webkits-optionsetcontainsany.html>.
(WebCore::Editor::markMisspellingsAndBadGrammar):
(WebCore::Editor::updateMarkersForWordsAffectedByEditing):
(WebCore::Editor::editorUIUpdateTimerFired):
(WebCore::Editor::resolveTextCheckingTypeMask):
* editing/Editor.h:
* editing/SpellChecker.cpp:
(WebCore::SpellCheckRequest::SpellCheckRequest):
(WebCore::SpellCheckRequest::create):
(WebCore::SpellChecker::didCheckSucceed):
* editing/SpellChecker.h:
* editing/TextCheckingHelper.cpp:
(WebCore::findGrammaticalErrors):
(WebCore::findMisspellings):
(WebCore::TextCheckingHelper::findFirstMisspellingOrBadGrammar):
(WebCore::TextCheckingHelper::guessesForMisspelledOrUngrammaticalRange const):
(WebCore::checkTextOfParagraph):
* editing/TextCheckingHelper.h:
* loader/EmptyClients.cpp:
* platform/text/TextCheckerClient.h:
* platform/text/TextChecking.h: Remove TextCheckingTypeMask. Reorganized the fields of TextCheckingRequestData
to coallesce padding and move it to the end of class. Also used default initializer syntax and defaulted (= default)
the default constructor of TextCheckingRequestData, removing the need for a user-defined default constructor.
(WebCore::TextCheckingRequestData::TextCheckingRequestData):
(WebCore::TextCheckingRequestData::text const): Changed return type from String to const String&
to avoid unnecessary ref-count churn for callers that do not need to take a shared ownership in
this string.
(WebCore::TextCheckingRequestData::checkingTypes const): Renamed; formerly named mask.
(WebCore::TextCheckingRequestData::mask const): Deleted.
* platform/text/mac/TextCheckingMac.mm: Added.
(WebCore::nsTextCheckingTypes):
* testing/Internals.cpp:
(WebCore::Internals::handleAcceptedCandidate):

2018-08-21 Fujii Hironori <Hironori.Fujii@sony.com>

Don't place "using namespace XXX;" in global space for unified source builds
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/PlatformMac.cmake
View file
Expand Up @@ -452,6 +452,7 @@ list(APPEND WebCore_SOURCES

platform/text/mac/LocaleMac.mm
platform/text/mac/TextBoundaries.mm
platform/text/mac/TextCheckingMac.mm
platform/text/mac/TextEncodingRegistryMac.mm

rendering/RenderThemeCocoa.mm
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/SourcesCocoa.txt
View file
Expand Up @@ -527,6 +527,7 @@ platform/text/ios/TextEncodingRegistryIOS.mm

platform/text/mac/LocaleMac.mm
platform/text/mac/TextBoundaries.mm
platform/text/mac/TextCheckingMac.mm
platform/text/mac/TextEncodingRegistryMac.mm

rendering/RenderThemeCocoa.mm
Expand Down
2 changes: 2 additions & 0 deletions Source/WebCore/WebCore.xcodeproj/project.pbxproj
View file
Expand Up @@ -13534,6 +13534,7 @@
CE7B2DB11586ABAD0098B3FA /* TextAlternativeWithRange.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = TextAlternativeWithRange.h; sourceTree = "<group>"; };
CE7B2DB21586ABAD0098B3FA /* TextAlternativeWithRange.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = TextAlternativeWithRange.mm; sourceTree = "<group>"; };
CE7E17821C83A49100AD06AF /* ContentSecurityPolicyHash.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = ContentSecurityPolicyHash.h; path = csp/ContentSecurityPolicyHash.h; sourceTree = "<group>"; };
CEA84720212622AD00940809 /* TextCheckingMac.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = TextCheckingMac.mm; sourceTree = "<group>"; };
CEBB8C3120786DCB00039547 /* FetchIdioms.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = FetchIdioms.h; sourceTree = "<group>"; };
CEBB8C3220786DCB00039547 /* FetchIdioms.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = FetchIdioms.cpp; sourceTree = "<group>"; };
CECADFC2153778FF00E37068 /* DictationAlternative.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = DictationAlternative.cpp; sourceTree = "<group>"; };
Expand Down Expand Up @@ -23915,6 +23916,7 @@
F5973DDE15CFB2030027F804 /* LocaleMac.h */,
F5973DDF15CFB2030027F804 /* LocaleMac.mm */,
B2AFFC8C0D00A5DF0030074D /* TextBoundaries.mm */,
CEA84720212622AD00940809 /* TextCheckingMac.mm */,
A1F55DC41F54D3F000EDB75F /* TextEncodingRegistryMac.mm */,
);
path = mac;
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/accessibility/AccessibilityObject.cpp
View file
Expand Up @@ -437,7 +437,7 @@ bool AccessibilityObject::hasMisspelling() const

if (unifiedTextCheckerEnabled(frame)) {
Vector<TextCheckingResult> results;
checkTextOfParagraph(*textChecker, stringValue(), TextCheckingTypeSpelling, results, frame->selection().selection());
checkTextOfParagraph(*textChecker, stringValue(), TextCheckingType::Spelling, results, frame->selection().selection());
if (!results.isEmpty())
isMisspelled = true;
return isMisspelled;
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm
View file
Expand Up @@ -909,7 +909,7 @@ static void AXAttributeStringSetSpelling(NSMutableAttributedString* attrString,

// checkTextOfParagraph is the only spelling/grammar checker implemented in WK1 and WK2
Vector<TextCheckingResult> results;
checkTextOfParagraph(*checker, text, TextCheckingTypeSpelling, results, node->document().frame()->selection().selection());
checkTextOfParagraph(*checker, text, TextCheckingType::Spelling, results, node->document().frame()->selection().selection());

size_t size = results.size();
for (unsigned i = 0; i < size; i++) {
Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/editing/AlternativeTextController.cpp
View file
Expand Up @@ -284,7 +284,7 @@ void AlternativeTextController::timerFired()
VisiblePosition p = startOfWord(start, LeftWordIfOnBoundary);
VisibleSelection adjacentWords = VisibleSelection(p, start);
auto adjacentWordRange = adjacentWords.toNormalizedRange();
m_frame.editor().markAllMisspellingsAndBadGrammarInRanges(TextCheckingTypeSpelling | TextCheckingTypeReplacement | TextCheckingTypeShowCorrectionPanel, adjacentWordRange.copyRef(), adjacentWordRange.copyRef(), nullptr);
m_frame.editor().markAllMisspellingsAndBadGrammarInRanges({ TextCheckingType::Spelling, TextCheckingType::Replacement, TextCheckingType::ShowCorrectionPanel }, adjacentWordRange.copyRef(), adjacentWordRange.copyRef(), nullptr);
}
break;
case AlternativeTextTypeReversion: {
Expand Down Expand Up @@ -540,7 +540,7 @@ bool AlternativeTextController::processMarkersOnTextToBeReplacedByResult(const T
{
DocumentMarkerController& markerController = m_frame.document()->markers();
if (markerController.hasMarkers(rangeWithAlternative, DocumentMarker::Replacement)) {
if (result.type == TextCheckingTypeCorrection)
if (result.type == TextCheckingType::Correction)
recordSpellcheckerResponseForModifiedCorrection(rangeWithAlternative, stringToBeReplaced, result.replacement);
return false;
}
Expand Down

0 comments on commit 8a1bcff

Please sign in to comment.