Remove some unnecessary indirection when getting Document’s Editor
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 11 May 2020 22:44:28 +0000 (22:44 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 11 May 2020 22:44:28 +0000 (22:44 +0000)
https://bugs.webkit.org/show_bug.cgi?id=211744

Reviewed by Geoffrey Garen.

Source/WebCore:

After r261018, there's no longer a need to reach into Document's Frame to get the Editor instance, since
Document itself owns the Editor (and Frame's implementation of editor() just calls back into the document
anyways). No change in behavior.

* accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
(AXAttributeStringSetSpelling):
* dom/Document.cpp:
(WebCore::Document::setFocusedElement):
(WebCore::Document::registerAttachmentIdentifier):
(WebCore::Document::didInsertAttachmentElement):
(WebCore::Document::didRemoveAttachmentElement):
* testing/Internals.cpp:
(WebCore::Internals::markerCountForNode):
(WebCore::Internals::setMarkedTextMatchesAreHighlighted):
(WebCore::Internals::lastSpellCheckRequestSequence):
(WebCore::Internals::lastSpellCheckProcessedSequence):
(WebCore::Internals::hasSpellingMarker):
(WebCore::Internals::hasAutocorrectedMarker):
(WebCore::Internals::setContinuousSpellCheckingEnabled):
(WebCore::Internals::setAutomaticQuoteSubstitutionEnabled):
(WebCore::Internals::setAutomaticLinkDetectionEnabled):
(WebCore::Internals::setAutomaticDashSubstitutionEnabled):
(WebCore::Internals::setAutomaticTextReplacementEnabled):
(WebCore::Internals::setAutomaticSpellingCorrectionEnabled):
(WebCore::Internals::handleAcceptedCandidate):
(WebCore::Internals::isOverwriteModeEnabled):
(WebCore::Internals::toggleOverwriteModeEnabled):
(WebCore::Internals::rangeOfString):
(WebCore::Internals::countMatchesForText):
(WebCore::Internals::hasGrammarMarker):

Source/WebKit:

See WebCore ChangeLog for more information. No change in behavior.

* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::performActionOnElement):

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

Source/WebCore/ChangeLog
Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm
Source/WebCore/dom/Document.cpp
Source/WebCore/testing/Internals.cpp
Source/WebKit/ChangeLog
Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm

index e8147c4..0f01d87 100644 (file)
@@ -1,3 +1,41 @@
+2020-05-11  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Remove some unnecessary indirection when getting Document’s Editor
+        https://bugs.webkit.org/show_bug.cgi?id=211744
+
+        Reviewed by Geoffrey Garen.
+
+        After r261018, there's no longer a need to reach into Document's Frame to get the Editor instance, since
+        Document itself owns the Editor (and Frame's implementation of editor() just calls back into the document
+        anyways). No change in behavior.
+
+        * accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
+        (AXAttributeStringSetSpelling):
+        * dom/Document.cpp:
+        (WebCore::Document::setFocusedElement):
+        (WebCore::Document::registerAttachmentIdentifier):
+        (WebCore::Document::didInsertAttachmentElement):
+        (WebCore::Document::didRemoveAttachmentElement):
+        * testing/Internals.cpp:
+        (WebCore::Internals::markerCountForNode):
+        (WebCore::Internals::setMarkedTextMatchesAreHighlighted):
+        (WebCore::Internals::lastSpellCheckRequestSequence):
+        (WebCore::Internals::lastSpellCheckProcessedSequence):
+        (WebCore::Internals::hasSpellingMarker):
+        (WebCore::Internals::hasAutocorrectedMarker):
+        (WebCore::Internals::setContinuousSpellCheckingEnabled):
+        (WebCore::Internals::setAutomaticQuoteSubstitutionEnabled):
+        (WebCore::Internals::setAutomaticLinkDetectionEnabled):
+        (WebCore::Internals::setAutomaticDashSubstitutionEnabled):
+        (WebCore::Internals::setAutomaticTextReplacementEnabled):
+        (WebCore::Internals::setAutomaticSpellingCorrectionEnabled):
+        (WebCore::Internals::handleAcceptedCandidate):
+        (WebCore::Internals::isOverwriteModeEnabled):
+        (WebCore::Internals::toggleOverwriteModeEnabled):
+        (WebCore::Internals::rangeOfString):
+        (WebCore::Internals::countMatchesForText):
+        (WebCore::Internals::hasGrammarMarker):
+
 2020-05-11  Devin Rousso  <drousso@apple.com>
 
         Web Inspector: show JavaScript Worker name as an internal property
index 1514203..e4376fd 100644 (file)
@@ -1078,7 +1078,7 @@ static void AXAttributeStringSetSpelling(NSMutableAttributedString* attrString,
 
     if (unifiedTextCheckerEnabled(node->document().frame())) {
         // Check the spelling directly since document->markersForNode() does not store the misspelled marking when the cursor is in a word.
-        TextCheckerClient* checker = node->document().frame()->editor().textChecker();
+        TextCheckerClient* checker = node->document().editor().textChecker();
         
         // checkTextOfParagraph is the only spelling/grammar checker implemented in WK1 and WK2
         Vector<TextCheckingResult> results;
@@ -1098,7 +1098,7 @@ static void AXAttributeStringSetSpelling(NSMutableAttributedString* attrString,
     for (unsigned currentPosition = 0; currentPosition < text.length(); ) {
         int misspellingLocation = -1;
         int misspellingLength = 0;
-        node->document().frame()->editor().textChecker()->checkSpellingOfString(text.substring(currentPosition), &misspellingLocation, &misspellingLength);
+        node->document().editor().textChecker()->checkSpellingOfString(text.substring(currentPosition), &misspellingLocation, &misspellingLength);
         if (misspellingLocation == -1 || !misspellingLength)
             break;
         
index 6252a6e..49f98e2 100644 (file)
@@ -4347,7 +4347,7 @@ bool Document::setFocusedElement(Element* element, FocusDirection direction, Foc
         }
 
         if (oldFocusedElement->isRootEditableElement())
-            frame()->editor().didEndEditing();
+            editor().didEndEditing();
 
         if (view()) {
             if (Widget* oldWidget = widgetForElement(oldFocusedElement.get()))
@@ -4412,7 +4412,7 @@ bool Document::setFocusedElement(Element* element, FocusDirection direction, Foc
         }
 
         if (m_focusedElement->isRootEditableElement())
-            frame()->editor().didBeginEditing();
+            editor().didBeginEditing();
 
         // eww, I suck. set the qt focus correctly
         // ### find a better place in the code for this
@@ -8150,8 +8150,9 @@ Vector<RefPtr<WebAnimation>> Document::matchingAnimations(const WTF::Function<bo
 
 void Document::registerAttachmentIdentifier(const String& identifier)
 {
-    if (auto* frame = this->frame())
-        frame->editor().registerAttachmentIdentifier(identifier);
+    // FIXME: Can this null check for Frame be removed?
+    if (frame())
+        editor().registerAttachmentIdentifier(identifier);
 }
 
 void Document::didInsertAttachmentElement(HTMLAttachmentElement& attachment)
@@ -8167,10 +8168,11 @@ void Document::didInsertAttachmentElement(HTMLAttachmentElement& attachment)
 
     m_attachmentIdentifierToElementMap.set(identifier, attachment);
 
-    if (auto* frame = this->frame()) {
+    // FIXME: Can this null check for Frame be removed?
+    if (frame()) {
         if (previousIdentifierIsNotUnique)
-            frame->editor().cloneAttachmentData(previousIdentifier, identifier);
-        frame->editor().didInsertAttachmentElement(attachment);
+            editor().cloneAttachmentData(previousIdentifier, identifier);
+        editor().didInsertAttachmentElement(attachment);
     }
 }
 
@@ -8182,8 +8184,9 @@ void Document::didRemoveAttachmentElement(HTMLAttachmentElement& attachment)
 
     m_attachmentIdentifierToElementMap.remove(identifier);
 
+    // FIXME: Can this null check for Frame be removed?
     if (frame())
-        frame()->editor().didRemoveAttachmentElement(attachment);
+        editor().didRemoveAttachmentElement(attachment);
 }
 
 RefPtr<HTMLAttachmentElement> Document::attachmentForIdentifier(const String& identifier) const
index 3caa810..c6b9a97 100644 (file)
@@ -1711,7 +1711,7 @@ ExceptionOr<unsigned> Internals::markerCountForNode(Node& node, const String& ma
     if (!markerTypesFrom(markerType, markerTypes))
         return Exception { SyntaxError };
 
-    node.document().frame()->editor().updateEditorUINowIfScheduled();
+    node.document().editor().updateEditorUINowIfScheduled();
     return node.document().markers().markersFor(node, markerTypes).size();
 }
 
@@ -1723,7 +1723,7 @@ ExceptionOr<RenderedDocumentMarker*> Internals::markerAt(Node& node, const Strin
     if (!markerTypesFrom(markerType, markerTypes))
         return Exception { SyntaxError };
 
-    node.document().frame()->editor().updateEditorUINowIfScheduled();
+    node.document().editor().updateEditorUINowIfScheduled();
 
     Vector<RenderedDocumentMarker*> markers = node.document().markers().markersFor(node, markerTypes);
     if (markers.size() <= index)
@@ -1775,7 +1775,7 @@ ExceptionOr<void> Internals::setMarkedTextMatchesAreHighlighted(bool flag)
     Document* document = contextDocument();
     if (!document || !document->frame())
         return Exception { InvalidAccessError };
-    document->frame()->editor().setMarkedTextMatchesAreHighlighted(flag);
+    document->editor().setMarkedTextMatchesAreHighlighted(flag);
     return { };
 }
 
@@ -2115,7 +2115,7 @@ ExceptionOr<uint64_t> Internals::lastSpellCheckRequestSequence()
     if (!document || !document->frame())
         return Exception { InvalidAccessError };
 
-    return document->frame()->editor().spellChecker().lastRequestIdentifier().toUInt64();
+    return document->editor().spellChecker().lastRequestIdentifier().toUInt64();
 }
 
 ExceptionOr<uint64_t> Internals::lastSpellCheckProcessedSequence()
@@ -2124,7 +2124,7 @@ ExceptionOr<uint64_t> Internals::lastSpellCheckProcessedSequence()
     if (!document || !document->frame())
         return Exception { InvalidAccessError };
 
-    return document->frame()->editor().spellChecker().lastProcessedIdentifier().toUInt64();
+    return document->editor().spellChecker().lastProcessedIdentifier().toUInt64();
 }
 
 Vector<String> Internals::userPreferredLanguages() const
@@ -2331,7 +2331,7 @@ bool Internals::hasSpellingMarker(int from, int length)
 
     updateEditorUINowIfScheduled();
 
-    return document->frame()->editor().selectionStartHasMarkerFor(DocumentMarker::Spelling, from, length);
+    return document->editor().selectionStartHasMarkerFor(DocumentMarker::Spelling, from, length);
 }
 
 bool Internals::hasAutocorrectedMarker(int from, int length)
@@ -2342,7 +2342,7 @@ bool Internals::hasAutocorrectedMarker(int from, int length)
 
     updateEditorUINowIfScheduled();
 
-    return document->frame()->editor().selectionStartHasMarkerFor(DocumentMarker::Autocorrected, from, length);
+    return document->editor().selectionStartHasMarkerFor(DocumentMarker::Autocorrected, from, length);
 }
 
 void Internals::setContinuousSpellCheckingEnabled(bool enabled)
@@ -2350,8 +2350,8 @@ void Internals::setContinuousSpellCheckingEnabled(bool enabled)
     if (!contextDocument() || !contextDocument()->frame())
         return;
 
-    if (enabled != contextDocument()->frame()->editor().isContinuousSpellCheckingEnabled())
-        contextDocument()->frame()->editor().toggleContinuousSpellChecking();
+    if (enabled != contextDocument()->editor().isContinuousSpellCheckingEnabled())
+        contextDocument()->editor().toggleContinuousSpellChecking();
 }
 
 void Internals::setAutomaticQuoteSubstitutionEnabled(bool enabled)
@@ -2360,8 +2360,8 @@ void Internals::setAutomaticQuoteSubstitutionEnabled(bool enabled)
         return;
 
 #if USE(AUTOMATIC_TEXT_REPLACEMENT)
-    if (enabled != contextDocument()->frame()->editor().isAutomaticQuoteSubstitutionEnabled())
-        contextDocument()->frame()->editor().toggleAutomaticQuoteSubstitution();
+    if (enabled != contextDocument()->editor().isAutomaticQuoteSubstitutionEnabled())
+        contextDocument()->editor().toggleAutomaticQuoteSubstitution();
 #else
     UNUSED_PARAM(enabled);
 #endif
@@ -2373,8 +2373,8 @@ void Internals::setAutomaticLinkDetectionEnabled(bool enabled)
         return;
 
 #if USE(AUTOMATIC_TEXT_REPLACEMENT)
-    if (enabled != contextDocument()->frame()->editor().isAutomaticLinkDetectionEnabled())
-        contextDocument()->frame()->editor().toggleAutomaticLinkDetection();
+    if (enabled != contextDocument()->editor().isAutomaticLinkDetectionEnabled())
+        contextDocument()->editor().toggleAutomaticLinkDetection();
 #else
     UNUSED_PARAM(enabled);
 #endif
@@ -2393,8 +2393,8 @@ void Internals::setAutomaticDashSubstitutionEnabled(bool enabled)
         return;
 
 #if USE(AUTOMATIC_TEXT_REPLACEMENT)
-    if (enabled != contextDocument()->frame()->editor().isAutomaticDashSubstitutionEnabled())
-        contextDocument()->frame()->editor().toggleAutomaticDashSubstitution();
+    if (enabled != contextDocument()->editor().isAutomaticDashSubstitutionEnabled())
+        contextDocument()->editor().toggleAutomaticDashSubstitution();
 #else
     UNUSED_PARAM(enabled);
 #endif
@@ -2406,8 +2406,8 @@ void Internals::setAutomaticTextReplacementEnabled(bool enabled)
         return;
 
 #if USE(AUTOMATIC_TEXT_REPLACEMENT)
-    if (enabled != contextDocument()->frame()->editor().isAutomaticTextReplacementEnabled())
-        contextDocument()->frame()->editor().toggleAutomaticTextReplacement();
+    if (enabled != contextDocument()->editor().isAutomaticTextReplacementEnabled())
+        contextDocument()->editor().toggleAutomaticTextReplacement();
 #else
     UNUSED_PARAM(enabled);
 #endif
@@ -2419,8 +2419,8 @@ void Internals::setAutomaticSpellingCorrectionEnabled(bool enabled)
         return;
 
 #if USE(AUTOMATIC_TEXT_REPLACEMENT)
-    if (enabled != contextDocument()->frame()->editor().isAutomaticSpellingCorrectionEnabled())
-        contextDocument()->frame()->editor().toggleAutomaticSpellingCorrection();
+    if (enabled != contextDocument()->editor().isAutomaticSpellingCorrectionEnabled())
+        contextDocument()->editor().toggleAutomaticSpellingCorrection();
 #else
     UNUSED_PARAM(enabled);
 #endif
@@ -2435,7 +2435,7 @@ void Internals::handleAcceptedCandidate(const String& candidate, unsigned locati
     result.type = TextCheckingType::None;
     result.range = { location, length };
     result.replacement = candidate;
-    contextDocument()->frame()->editor().handleAcceptedCandidate(result);
+    contextDocument()->editor().handleAcceptedCandidate(result);
 }
 
 void Internals::changeSelectionListType()
@@ -2450,7 +2450,7 @@ bool Internals::isOverwriteModeEnabled()
     if (!document || !document->frame())
         return false;
 
-    return document->frame()->editor().isOverwriteModeEnabled();
+    return document->editor().isOverwriteModeEnabled();
 }
 
 void Internals::toggleOverwriteModeEnabled()
@@ -2459,7 +2459,7 @@ void Internals::toggleOverwriteModeEnabled()
     if (!document || !document->frame())
         return;
 
-    document->frame()->editor().toggleOverwriteModeEnabled();
+    document->editor().toggleOverwriteModeEnabled();
 }
 
 static ExceptionOr<FindOptions> parseFindOptions(const Vector<String>& optionList)
@@ -2504,7 +2504,7 @@ ExceptionOr<RefPtr<Range>> Internals::rangeOfString(const String& text, RefPtr<R
     if (parsedOptions.hasException())
         return parsedOptions.releaseException();
 
-    return document->frame()->editor().rangeOfString(text, referenceRange.get(), parsedOptions.releaseReturnValue());
+    return document->editor().rangeOfString(text, referenceRange.get(), parsedOptions.releaseReturnValue());
 }
 
 ExceptionOr<unsigned> Internals::countMatchesForText(const String& text, const Vector<String>& findOptions, const String& markMatches)
@@ -2518,7 +2518,7 @@ ExceptionOr<unsigned> Internals::countMatchesForText(const String& text, const V
         return parsedOptions.releaseException();
 
     bool mark = markMatches == "mark";
-    return document->frame()->editor().countMatchesForText(text, nullptr, parsedOptions.releaseReturnValue(), 1000, mark, nullptr);
+    return document->editor().countMatchesForText(text, nullptr, parsedOptions.releaseReturnValue(), 1000, mark, nullptr);
 }
 
 ExceptionOr<unsigned> Internals::countFindMatches(const String& text, const Vector<String>& findOptions)
@@ -2651,7 +2651,7 @@ bool Internals::hasGrammarMarker(int from, int length)
     if (!document || !document->frame())
         return false;
 
-    return document->frame()->editor().selectionStartHasMarkerFor(DocumentMarker::Grammar, from, length);
+    return document->editor().selectionStartHasMarkerFor(DocumentMarker::Grammar, from, length);
 }
 
 unsigned Internals::numberOfScrollableAreas()
index 8b0a9b3..c0b995c 100644 (file)
@@ -1,3 +1,15 @@
+2020-05-11  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Remove some unnecessary indirection when getting Document’s Editor
+        https://bugs.webkit.org/show_bug.cgi?id=211744
+
+        Reviewed by Geoffrey Garen.
+
+        See WebCore ChangeLog for more information. No change in behavior.
+
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::performActionOnElement):
+
 2020-05-11  Simon Fraser  <simon.fraser@apple.com>
 
         Fix possible crash in ViewGestureController on macOS
index 784c50d..ef4d257 100644 (file)
@@ -2975,9 +2975,9 @@ void WebPage::performActionOnElement(uint32_t action)
                     title = linkElement->textContent();
                 title = stripLeadingAndTrailingHTMLSpaces(title);
             }
-            m_interactionNode->document().frame()->editor().writeImageToPasteboard(*Pasteboard::createForCopyAndPaste(), element, url, title);
+            m_interactionNode->document().editor().writeImageToPasteboard(*Pasteboard::createForCopyAndPaste(), element, url, title);
         } else if (element.isLink()) {
-            m_interactionNode->document().frame()->editor().copyURL(element.document().completeURL(stripLeadingAndTrailingHTMLSpaces(element.attributeWithoutSynchronization(HTMLNames::hrefAttr))), element.textContent());
+            m_interactionNode->document().editor().copyURL(element.document().completeURL(stripLeadingAndTrailingHTMLSpaces(element.attributeWithoutSynchronization(HTMLNames::hrefAttr))), element.textContent());
         }
     } else if (static_cast<SheetAction>(action) == SheetAction::SaveImage) {
         if (!is<RenderImage>(*element.renderer()))