[webkit-changes] [260826] trunk/Source/WebKit

Tue, 28 Apr 2020 07:56:11 -0700

Title: [260826] trunk/Source/WebKit
Revision
260826
Author
[email protected]
Date
2020-04-28 07:54:52 -0700 (Tue, 28 Apr 2020)

Log Message

[iOS] Marked text editor state only needs to be set when layout is up-to-date
https://bugs.webkit.org/show_bug.cgi?id=211087

Patch by Daniel Bates <[email protected]> on 2020-04-28
Reviewed by Darin Adler.

Organize the code to reflect reality: editor state only has marked text rects if layout is
up-to-date. So, move these fields to from EditorState to EditorState::PostLayoutData.

While I am in this area of the code I also moved the editor state originIdentifierForPasteboard
to the top of the struct as it is aesthetically more pleasing to me to have the post layout
data last. It likely also produces better bit packing though this wasn't my primary motivation.

* Shared/EditorState.cpp:
(WebKit::EditorState::encode const):
(WebKit::EditorState::decode):
(WebKit::EditorState::PostLayoutData::encode const):
(WebKit::EditorState::PostLayoutData::decode):
Move encoding and decoding of marked test fields from EditorState to EditorState::PostLayoutData.

(WebKit::operator<<): Fix up code now that marked text fields are in PostLayoutData.
* Shared/EditorState.h:
* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView textFirstRect]): Update code now that this data is in the post layout data struct.
(-[WKContentView textLastRect]): Ditto.
(-[WKContentView _updateChangedSelection:]): Ditto.
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::convertContentToRootViewSelectionRects): Added.
(WebKit::WebPage::getPlatformEditorState const): Update now that marked text fields are in the
post layout data struct. Simplify the early return as well: only continue if we're not missing post
layout data. If we're missing post layout data then we either do not have a frame view or layout is
not up-to-data. Either way we don't want to do anything. I also stored the FrameView in a Ref<> to
future proof the code should future code cause view detachment and destruction. Note that layout
could cause this, but the code currently covers this case. So, taking a Ref<> is about future proofing.
(WebKit::WebPage::getRectsForGranularityWithSelectionOffset): Update to use convertContentToRootViewSelectionRects().
(WebKit::WebPage::getRectsAtSelectionOffsetWithText): Ditto.
(WebKit::WebPage::requestAutocorrectionData): Use Vector::map() to convert the selection rects to
from content to root view FloatRects.
(WebKit::WebPage::convertSelectionRectsToRootView): Deleted.

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (260825 => 260826)


--- trunk/Source/WebKit/ChangeLog	2020-04-28 14:49:04 UTC (rev 260825)
+++ trunk/Source/WebKit/ChangeLog	2020-04-28 14:54:52 UTC (rev 260826)
@@ -1,3 +1,44 @@
+2020-04-28  Daniel Bates  <[email protected]>
+
+        [iOS] Marked text editor state only needs to be set when layout is up-to-date
+        https://bugs.webkit.org/show_bug.cgi?id=211087
+
+        Reviewed by Darin Adler.
+
+        Organize the code to reflect reality: editor state only has marked text rects if layout is
+        up-to-date. So, move these fields to from EditorState to EditorState::PostLayoutData.
+
+        While I am in this area of the code I also moved the editor state originIdentifierForPasteboard
+        to the top of the struct as it is aesthetically more pleasing to me to have the post layout
+        data last. It likely also produces better bit packing though this wasn't my primary motivation.
+
+        * Shared/EditorState.cpp:
+        (WebKit::EditorState::encode const):
+        (WebKit::EditorState::decode):
+        (WebKit::EditorState::PostLayoutData::encode const): 
+        (WebKit::EditorState::PostLayoutData::decode):
+        Move encoding and decoding of marked test fields from EditorState to EditorState::PostLayoutData.
+
+        (WebKit::operator<<): Fix up code now that marked text fields are in PostLayoutData.
+        * Shared/EditorState.h:
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView textFirstRect]): Update code now that this data is in the post layout data struct.
+        (-[WKContentView textLastRect]): Ditto.
+        (-[WKContentView _updateChangedSelection:]): Ditto.
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::convertContentToRootViewSelectionRects): Added.
+        (WebKit::WebPage::getPlatformEditorState const): Update now that marked text fields are in the
+        post layout data struct. Simplify the early return as well: only continue if we're not missing post
+        layout data. If we're missing post layout data then we either do not have a frame view or layout is
+        not up-to-data. Either way we don't want to do anything. I also stored the FrameView in a Ref<> to
+        future proof the code should future code cause view detachment and destruction. Note that layout
+        could cause this, but the code currently covers this case. So, taking a Ref<> is about future proofing.
+        (WebKit::WebPage::getRectsForGranularityWithSelectionOffset): Update to use convertContentToRootViewSelectionRects().
+        (WebKit::WebPage::getRectsAtSelectionOffsetWithText): Ditto.
+        (WebKit::WebPage::requestAutocorrectionData): Use Vector::map() to convert the selection rects to
+        from content to root view FloatRects.
+        (WebKit::WebPage::convertSelectionRectsToRootView): Deleted.
+
 2020-04-28  Per Arne Vollan  <[email protected]>
 
         [iOS] Fix sandbox violation when uploading a file

Modified: trunk/Source/WebKit/Shared/EditorState.cpp (260825 => 260826)


--- trunk/Source/WebKit/Shared/EditorState.cpp	2020-04-28 14:49:04 UTC (rev 260825)
+++ trunk/Source/WebKit/Shared/EditorState.cpp	2020-04-28 14:54:52 UTC (rev 260826)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010, 2011 Apple Inc. All rights reserved.
+ * Copyright (C) 2010-2020 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -34,6 +34,7 @@
 
 void EditorState::encode(IPC::Encoder& encoder) const
 {
+    encoder << originIdentifierForPasteboard;
     encoder << shouldIgnoreSelectionChanges;
     encoder << selectionIsNone;
     encoder << selectionIsRange;
@@ -43,21 +44,15 @@
     encoder << isInPlugin;
     encoder << hasComposition;
     encoder << isMissingPostLayoutData;
-
     if (!isMissingPostLayoutData)
         m_postLayoutData.encode(encoder);
-
-#if PLATFORM(IOS_FAMILY)
-    encoder << firstMarkedRect;
-    encoder << lastMarkedRect;
-    encoder << markedText;
-#endif
-
-    encoder << originIdentifierForPasteboard;
 }
 
 bool EditorState::decode(IPC::Decoder& decoder, EditorState& result)
 {
+    if (!decoder.decode(result.originIdentifierForPasteboard))
+        return false;
+
     if (!decoder.decode(result.shouldIgnoreSelectionChanges))
         return false;
 
@@ -90,18 +85,6 @@
             return false;
     }
 
-#if PLATFORM(IOS_FAMILY)
-    if (!decoder.decode(result.firstMarkedRect))
-        return false;
-    if (!decoder.decode(result.lastMarkedRect))
-        return false;
-    if (!decoder.decode(result.markedText))
-        return false;
-#endif
-
-    if (!decoder.decode(result.originIdentifierForPasteboard))
-        return false;
-
     return true;
 }
 
@@ -136,6 +119,9 @@
     encoder << atStartOfSentence;
     encoder << selectionStartIsAtParagraphBoundary;
     encoder << selectionEndIsAtParagraphBoundary;
+    encoder << firstMarkedRect;
+    encoder << lastMarkedRect;
+    encoder << markedText;
 #endif
 #if PLATFORM(MAC)
     encoder << candidateRequestStartPosition;
@@ -208,6 +194,12 @@
         return false;
     if (!decoder.decode(result.selectionEndIsAtParagraphBoundary))
         return false;
+    if (!decoder.decode(result.firstMarkedRect))
+        return false;
+    if (!decoder.decode(result.lastMarkedRect))
+        return false;
+    if (!decoder.decode(result.markedText))
+        return false;
 #endif
 #if PLATFORM(MAC)
     if (!decoder.decode(result.candidateRequestStartPosition))
@@ -247,15 +239,6 @@
 
 TextStream& operator<<(TextStream& ts, const EditorState& editorState)
 {
-#if PLATFORM(IOS_FAMILY)
-    if (editorState.firstMarkedRect != IntRect())
-        ts.dumpProperty("firstMarkedRect", editorState.firstMarkedRect);
-    if (editorState.lastMarkedRect != IntRect())
-        ts.dumpProperty("lastMarkedRect", editorState.lastMarkedRect);
-    if (editorState.markedText.length())
-        ts.dumpProperty("markedText", editorState.markedText);
-#endif
-
     if (editorState.shouldIgnoreSelectionChanges)
         ts.dumpProperty("shouldIgnoreSelectionChanges", editorState.shouldIgnoreSelectionChanges);
     if (!editorState.selectionIsNone)
@@ -301,6 +284,12 @@
         ts.dumpProperty("baseWritingDirection", static_cast<uint8_t>(editorState.postLayoutData().baseWritingDirection));
 #endif // PLATFORM(COCOA)
 #if PLATFORM(IOS_FAMILY)
+    if (editorState.postLayoutData().firstMarkedRect != IntRect())
+        ts.dumpProperty("firstMarkedRect", editorState.postLayoutData().firstMarkedRect);
+    if (editorState.postLayoutData().lastMarkedRect != IntRect())
+        ts.dumpProperty("lastMarkedRect", editorState.postLayoutData().lastMarkedRect);
+    if (editorState.postLayoutData().markedText.length())
+        ts.dumpProperty("markedText", editorState.postLayoutData().markedText);
     if (editorState.postLayoutData().caretRectAtEnd != IntRect())
         ts.dumpProperty("caretRectAtEnd", editorState.postLayoutData().caretRectAtEnd);
     if (editorState.postLayoutData().selectionRects.size())

Modified: trunk/Source/WebKit/Shared/EditorState.h (260825 => 260826)


--- trunk/Source/WebKit/Shared/EditorState.h	2020-04-28 14:49:04 UTC (rev 260825)
+++ trunk/Source/WebKit/Shared/EditorState.h	2020-04-28 14:54:52 UTC (rev 260826)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010, 2011 Apple Inc. All rights reserved.
+ * Copyright (C) 2010-2020 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -65,8 +65,8 @@
 };
 
 struct EditorState {
+    String originIdentifierForPasteboard;
     bool shouldIgnoreSelectionChanges { false };
-
     bool selectionIsNone { true }; // This will be false when there is a caret selection.
     bool selectionIsRange { false };
     bool isContentEditable { false };
@@ -76,14 +76,6 @@
     bool hasComposition { false };
     bool isMissingPostLayoutData { true };
 
-#if PLATFORM(IOS_FAMILY)
-    WebCore::IntRect firstMarkedRect;
-    WebCore::IntRect lastMarkedRect;
-    String markedText;
-#endif
-
-    String originIdentifierForPasteboard;
-
     struct PostLayoutData {
         uint32_t typingAttributes { AttributeNone };
 #if PLATFORM(IOS_FAMILY) || PLATFORM(GTK) || PLATFORM(WPE)
@@ -114,6 +106,9 @@
         bool atStartOfSentence { false };
         bool selectionStartIsAtParagraphBoundary { false };
         bool selectionEndIsAtParagraphBoundary { false };
+        WebCore::IntRect firstMarkedRect;
+        WebCore::IntRect lastMarkedRect;
+        String markedText;
 #endif
 #if PLATFORM(MAC)
         uint64_t candidateRequestStartPosition { 0 };

Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (260825 => 260826)


--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm	2020-04-28 14:49:04 UTC (rev 260825)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm	2020-04-28 14:54:52 UTC (rev 260826)
@@ -4118,12 +4118,12 @@
 
 - (CGRect)textFirstRect
 {
-    return (_page->editorState().hasComposition) ? _page->editorState().firstMarkedRect : _autocorrectionData.textFirstRect;
+    return _page->editorState().hasComposition ? _page->editorState().postLayoutData().firstMarkedRect : _autocorrectionData.textFirstRect;
 }
 
 - (CGRect)textLastRect
 {
-    return (_page->editorState().hasComposition) ? _page->editorState().lastMarkedRect : _autocorrectionData.textLastRect;
+    return _page->editorState().hasComposition ? _page->editorState().postLayoutData().lastMarkedRect : _autocorrectionData.textLastRect;
 }
 
 - (void)replaceDictatedText:(NSString*)oldText withText:(NSString *)newText
@@ -6522,12 +6522,12 @@
 
 - (void)_updateChangedSelection:(BOOL)force
 {
-    auto& state = _page->editorState();
-    if (state.isMissingPostLayoutData)
+    auto& editorState = _page->editorState();
+    if (editorState.isMissingPostLayoutData)
         return;
 
-    auto& postLayoutData = state.postLayoutData();
-    WebKit::WKSelectionDrawingInfo selectionDrawingInfo(state);
+    auto& postLayoutData = editorState.postLayoutData();
+    WebKit::WKSelectionDrawingInfo selectionDrawingInfo { editorState };
     if (force || selectionDrawingInfo != _lastSelectionDrawingInfo) {
         LOG_WITH_STREAM(Selection, stream << "_updateChangedSelection " << selectionDrawingInfo);
 
@@ -6535,7 +6535,7 @@
 
         // FIXME: We need to figure out what to do if the selection is changed by _javascript_.
         if (_textInteractionAssistant) {
-            _markedText = (_page->editorState().hasComposition) ? _page->editorState().markedText : String();
+            _markedText = editorState.hasComposition ? postLayoutData.markedText : String { };
             [_textInteractionAssistant selectionChanged];
         }

Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.h (260825 => 260826)


--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.h	2020-04-28 14:49:04 UTC (rev 260825)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.h	2020-04-28 14:54:52 UTC (rev 260826)
@@ -1353,8 +1353,6 @@
 #if PLATFORM(IOS_FAMILY)
     void updateViewportSizeForCSSViewportUnits();
 
-    static void convertSelectionRectsToRootView(WebCore::FrameView*, Vector<WebCore::SelectionRect>&);
-
     void getFocusedElementInformation(FocusedElementInformation&);
     void platformInitializeAccessibility();
     void generateSyntheticEditingCommand(SyntheticEditingCommandType);

Modified: trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (260825 => 260826)


--- trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm	2020-04-28 14:49:04 UTC (rev 260825)
+++ trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm	2020-04-28 14:54:52 UTC (rev 260826)
@@ -245,37 +245,37 @@
     return needsLayout;
 }
 
+static void convertContentToRootViewSelectionRects(const FrameView& view, Vector<SelectionRect>& rects)
+{
+    for (auto& rect : rects)
+        rect.setRect(view.contentsToRootView(rect.rect()));
+}
+
 void WebPage::getPlatformEditorState(Frame& frame, EditorState& result) const
 {
     getPlatformEditorStateCommon(frame, result);
 
-    FrameView* view = frame.view();
-    if (!view) {
-        result.isMissingPostLayoutData = true;
+    if (result.isMissingPostLayoutData)
         return;
-    }
+    ASSERT(frame.view());
+    auto& postLayoutData = result.postLayoutData();
+    auto view = makeRef(*frame.view());
 
     if (frame.editor().hasComposition()) {
-        RefPtr<Range> compositionRange = frame.editor().compositionRange();
+        auto compositionRange = frame.editor().compositionRange();
         Vector<WebCore::SelectionRect> compositionRects;
         if (compositionRange) {
             compositionRange->collectSelectionRects(compositionRects);
             if (compositionRects.size())
-                result.firstMarkedRect = view->contentsToRootView(compositionRects[0].rect());
+                postLayoutData.firstMarkedRect = view->contentsToRootView(compositionRects[0].rect());
             if (compositionRects.size() > 1)
-                result.lastMarkedRect = view->contentsToRootView(compositionRects.last().rect());
+                postLayoutData.lastMarkedRect = view->contentsToRootView(compositionRects.last().rect());
             else
-                result.lastMarkedRect = result.firstMarkedRect;
-            result.markedText = plainTextForContext(compositionRange.get());
+                postLayoutData.lastMarkedRect = postLayoutData.firstMarkedRect;
+            postLayoutData.markedText = plainTextForContext(compositionRange.get());
         }
     }
 
-    // We only set the remaining EditorState entries if layout is done as a performance optimization
-    // to avoid the need to force a synchronous layout here to compute these entries.
-    if (view->needsLayout() || result.isMissingPostLayoutData)
-        return;
-
-    auto& postLayoutData = result.postLayoutData();
     const auto& selection = frame.selection().selection();
     postLayoutData.isStableStateUpdate = m_isInStableState;
     bool startNodeIsInsideFixedPosition = false;
@@ -296,7 +296,7 @@
         String selectedText;
         if (selectedRange) {
             createLiveRange(*selectedRange)->collectSelectionRects(postLayoutData.selectionRects);
-            convertSelectionRectsToRootView(view, postLayoutData.selectionRects);
+            convertContentToRootViewSelectionRects(view, postLayoutData.selectionRects);
             selectedText = plainTextForDisplay(*selectedRange);
             postLayoutData.selectedTextLength = selectedText.length();
             const int maxSelectedTextLength = 200;
@@ -2004,7 +2004,7 @@
 
     Vector<WebCore::SelectionRect> selectionRects;
     range->collectSelectionRectsWithoutUnionInteriorLines(selectionRects);
-    convertSelectionRectsToRootView(frame.view(), selectionRects);
+    convertContentToRootViewSelectionRects(*frame.view(), selectionRects);
     send(Messages::WebPageProxy::SelectionRectsCallback(selectionRects, callbackID));
 }
 
@@ -2062,7 +2062,7 @@
 
     Vector<WebCore::SelectionRect> selectionRects;
     range->collectSelectionRectsWithoutUnionInteriorLines(selectionRects);
-    convertSelectionRectsToRootView(frame.view(), selectionRects);
+    convertContentToRootViewSelectionRects(*frame.view(), selectionRects);
     send(Messages::WebPageProxy::SelectionRectsCallback(selectionRects, callbackID));
 }
 
@@ -2257,14 +2257,6 @@
     send(Messages::WebPageProxy::UnsignedCallback(m_selectionAnchor == Start, callbackID));
 }
 
-void WebPage::convertSelectionRectsToRootView(FrameView* view, Vector<SelectionRect>& selectionRects)
-{
-    for (size_t i = 0; i < selectionRects.size(); ++i) {
-        SelectionRect& currentRect = selectionRects[i];
-        currentRect.setRect(view->contentsToRootView(currentRect.rect()));
-    }
-}
-
 void WebPage::requestDictationContext(CallbackID callbackID)
 {
     Frame& frame = m_page->focusController().focusedOrMainFrame();
@@ -2377,19 +2369,14 @@
     if (textForRange == textForAutocorrection)
         range->collectSelectionRects(selectionRects);
 
-    Vector<FloatRect> rectsForText;
-    rectsForText.grow(selectionRects.size());
+    auto rootViewSelectionRects = selectionRects.map([&](const auto& selectionRect) -> FloatRect { return frame.view()->contentsToRootView(selectionRect.rect()); });
 
-    convertSelectionRectsToRootView(frame.view(), selectionRects);
-    for (size_t i = 0; i < selectionRects.size(); i++)
-        rectsForText[i] = selectionRects[i].rect();
-
     bool multipleFonts = false;
     CTFontRef font = nil;
     if (auto* coreFont = frame.editor().fontForSelection(multipleFonts))
         font = coreFont->getCTFont();
 
-    reply({ WTFMove(rectsForText), (__bridge UIFont *)font });
+    reply({ WTFMove(rootViewSelectionRects) , (__bridge UIFont *)font });
 }
 
 void WebPage::applyAutocorrection(const String& correction, const String& originalText, CallbackID callbackID)

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to