-[WKAutocorrectionContext emptyAutocorrectionContext:] generates invalid empty context
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 19 Apr 2019 23:48:41 +0000 (23:48 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 19 Apr 2019 23:48:41 +0000 (23:48 +0000)
https://bugs.webkit.org/show_bug.cgi?id=197119

Reviewed by Wenson Hsieh.

Use the existing EditingRange type to represent the location and length of the marked text
range for an autocorrection instead of managing integers. This type avoid the need to handle
the special case for an empty range represented as NSMakeRange(NSNotFound, 0). Currently
WKAutocorrectionContext incorrectly represents the empty range as NSMakeRange(WTF::notFound, 0).

While I am here, simplify the existing WebAutocorrectionContext encoder/decoder code and rename
+[WKAutocorrectionContext autocorrectionContextWithContext:] to +autocorrectionContextWithWebContext
to better reflect the expected source of the conversion: a Web-type.

* Shared/ios/WebAutocorrectionContext.h:
(WebKit::WebAutocorrectionContext::encode const): Reformat while I am here to make this logic easy
to amend without losing SVN history.
(WebKit::WebAutocorrectionContext::decode): Simplify the code while I am here.
* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView _handleAutocorrectionContext:]): Update for renaming.
(+[WKAutocorrectionContext emptyAutocorrectionContext]): Update for renaming.
(+[WKAutocorrectionContext autocorrectionContextWithWebContext:]): Renamed; formerly named autocorrectionContextWithContext.
(+[WKAutocorrectionContext autocorrectionContextWithContext:]): Deleted.
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::autocorrectionContext): Update to make use of EditingRange. Also instantiate
the struct and return it, initializing its fields individually instead of using the constructor to
make this code less error prone. It's easy to introduce an error with the constructor notation when
amending the the struct because so many of the arguments are of the same data type. Individually
initializing the struct fields makes it less likely for an ordering mistake to be introduced.

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

Source/WebKit/ChangeLog
Source/WebKit/Shared/ios/WebAutocorrectionContext.h
Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm

index 8cda674..1f5b487 100644 (file)
@@ -1,3 +1,35 @@
+2019-04-19  Daniel Bates  <dabates@apple.com>
+
+        -[WKAutocorrectionContext emptyAutocorrectionContext:] generates invalid empty context
+        https://bugs.webkit.org/show_bug.cgi?id=197119
+
+        Reviewed by Wenson Hsieh.
+
+        Use the existing EditingRange type to represent the location and length of the marked text
+        range for an autocorrection instead of managing integers. This type avoid the need to handle
+        the special case for an empty range represented as NSMakeRange(NSNotFound, 0). Currently
+        WKAutocorrectionContext incorrectly represents the empty range as NSMakeRange(WTF::notFound, 0).
+
+        While I am here, simplify the existing WebAutocorrectionContext encoder/decoder code and rename
+        +[WKAutocorrectionContext autocorrectionContextWithContext:] to +autocorrectionContextWithWebContext
+        to better reflect the expected source of the conversion: a Web-type.
+
+        * Shared/ios/WebAutocorrectionContext.h:
+        (WebKit::WebAutocorrectionContext::encode const): Reformat while I am here to make this logic easy
+        to amend without losing SVN history.
+        (WebKit::WebAutocorrectionContext::decode): Simplify the code while I am here.
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView _handleAutocorrectionContext:]): Update for renaming.
+        (+[WKAutocorrectionContext emptyAutocorrectionContext]): Update for renaming.
+        (+[WKAutocorrectionContext autocorrectionContextWithWebContext:]): Renamed; formerly named autocorrectionContextWithContext.
+        (+[WKAutocorrectionContext autocorrectionContextWithContext:]): Deleted.
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::autocorrectionContext): Update to make use of EditingRange. Also instantiate
+        the struct and return it, initializing its fields individually instead of using the constructor to
+        make this code less error prone. It's easy to introduce an error with the constructor notation when
+        amending the the struct because so many of the arguments are of the same data type. Individually
+        initializing the struct fields makes it less likely for an ordering mistake to be introduced.
+
 2019-04-19  Dean Jackson  <dino@apple.com>
 
         Add more _WKElementActionTypes and provide API to create with custom types
index 3d9ce61..5fab054 100644 (file)
@@ -26,8 +26,8 @@
 #pragma once
 
 #include "Decoder.h"
+#include "EditingRange.h"
 #include "Encoder.h"
-#include <wtf/NotFound.h>
 #include <wtf/Optional.h>
 #include <wtf/text/WTFString.h>
 
@@ -38,51 +38,35 @@ struct WebAutocorrectionContext {
     String markedText;
     String selectedText;
     String contextAfter;
-    uint64_t location { notFound };
-    uint64_t length { 0 };
+    EditingRange markedTextRange;
 
     template<class Encoder> void encode(Encoder&) const;
     template<class Decoder> static Optional<WebAutocorrectionContext> decode(Decoder&);
 };
 
-template<class Encoder> inline void WebAutocorrectionContext::encode(Encoder& encoder) const
+template<class Encoder> void WebAutocorrectionContext::encode(Encoder& encoder) const
 {
-    encoder << contextBefore << markedText << selectedText << contextAfter << location << length;
+    encoder << contextBefore;
+    encoder << markedText;
+    encoder << selectedText;
+    encoder << contextAfter;
+    encoder << markedTextRange;
 }
 
-template<class Decoder> inline Optional<WebAutocorrectionContext> WebAutocorrectionContext::decode(Decoder& decoder)
+template<class Decoder> Optional<WebAutocorrectionContext> WebAutocorrectionContext::decode(Decoder& decoder)
 {
-    Optional<String> contextBefore;
-    decoder >> contextBefore;
-    if (!contextBefore)
+    WebAutocorrectionContext correction;
+    if (!decoder.decode(correction.contextBefore))
         return WTF::nullopt;
-
-    Optional<String> markedText;
-    decoder >> markedText;
-    if (!markedText)
+    if (!decoder.decode(correction.markedText))
         return WTF::nullopt;
-
-    Optional<String> selectedText;
-    decoder >> selectedText;
-    if (!selectedText)
+    if (!decoder.decode(correction.selectedText))
         return WTF::nullopt;
-
-    Optional<String> contextAfter;
-    decoder >> contextAfter;
-    if (!contextAfter)
+    if (!decoder.decode(correction.contextAfter))
         return WTF::nullopt;
-
-    Optional<uint64_t> location;
-    decoder >> location;
-    if (!location)
+    if (!decoder.decode(correction.markedTextRange))
         return WTF::nullopt;
-
-    Optional<uint64_t> length;
-    decoder >> length;
-    if (!length)
-        return WTF::nullopt;
-
-    return {{ WTFMove(*contextBefore), WTFMove(*markedText), WTFMove(*selectedText), WTFMove(*contextAfter), WTFMove(*location), WTFMove(*length) }};
+    return correction;
 }
 
 } // namespace WebKit
index fe544a6..33e6e97 100644 (file)
@@ -290,7 +290,7 @@ constexpr double fasterTapSignificantZoomThreshold = 0.8;
 
 @interface WKAutocorrectionContext : UIWKAutocorrectionContext
 + (WKAutocorrectionContext *)emptyAutocorrectionContext;
-+ (WKAutocorrectionContext *)autocorrectionContextWithContext:(const WebKit::WebAutocorrectionContext&)context;
++ (WKAutocorrectionContext *)autocorrectionContextWithWebContext:(const WebKit::WebAutocorrectionContext&)context;
 @end
 
 @interface UITextInteractionAssistant (UITextInteractionAssistant_Internal)
@@ -3718,7 +3718,7 @@ static void selectionChangedWithTouch(WKContentView *view, const WebCore::IntPoi
 
 - (void)_handleAutocorrectionContext:(const WebKit::WebAutocorrectionContext&)context
 {
-    [self _invokePendingAutocorrectionContextHandler:[WKAutocorrectionContext autocorrectionContextWithContext:context]];
+    [self _invokePendingAutocorrectionContextHandler:[WKAutocorrectionContext autocorrectionContextWithWebContext:context]];
 }
 
 #if !USE(UIKIT_KEYBOARD_ADDITIONS)
@@ -7686,23 +7686,18 @@ ALLOW_DEPRECATED_DECLARATIONS_END
 
 + (WKAutocorrectionContext *)emptyAutocorrectionContext
 {
-    return [self autocorrectionContextWithContext:WebKit::WebAutocorrectionContext { }];
+    return [self autocorrectionContextWithWebContext:WebKit::WebAutocorrectionContext { }];
 }
 
-+ (WKAutocorrectionContext *)autocorrectionContextWithContext:(const WebKit::WebAutocorrectionContext&)webContext
++ (WKAutocorrectionContext *)autocorrectionContextWithWebContext:(const WebKit::WebAutocorrectionContext&)webCorrection
 {
-    WKAutocorrectionContext *context = [[WKAutocorrectionContext alloc] init];
-
-    if (!webContext.contextBefore.isEmpty())
-        context.contextBeforeSelection = webContext.contextBefore;
-    if (!webContext.selectedText.isEmpty())
-        context.selectedText = webContext.selectedText;
-    if (!webContext.markedText.isEmpty())
-        context.markedText = webContext.markedText;
-    if (!webContext.contextAfter.isEmpty())
-        context.contextAfterSelection = webContext.contextAfter;
-    context.rangeInMarkedText = NSMakeRange(webContext.location, webContext.length);
-    return [context autorelease];
+    auto correction = adoptNS([[WKAutocorrectionContext alloc] init]);
+    [correction setContextBeforeSelection:nsStringNilIfEmpty(webCorrection.contextBefore)];
+    [correction setSelectedText:nsStringNilIfEmpty(webCorrection.selectedText)];
+    [correction setMarkedText:nsStringNilIfEmpty(webCorrection.markedText)];
+    [correction setContextAfterSelection:nsStringNilIfEmpty(webCorrection.contextAfter)];
+    [correction setRangeInMarkedText:webCorrection.markedTextRange];
+    return correction.autorelease();
 }
 
 @end
index ab3e68d..558c38d 100644 (file)
@@ -2180,8 +2180,7 @@ WebAutocorrectionContext WebPage::autocorrectionContext()
     String markedText;
     String selectedText;
     String contextAfter;
-    uint64_t location = NSNotFound;
-    uint64_t length = 0;
+    EditingRange markedTextRange;
 
     auto& frame = m_page->focusController().focusedOrMainFrame();
     RefPtr<Range> range;
@@ -2205,8 +2204,8 @@ WebAutocorrectionContext WebPage::autocorrectionContext()
             markedTextAfter = plainTextReplacingNoBreakSpace(range.get());
         markedText = markedTextBefore + selectedText + markedTextAfter;
         if (!markedText.isEmpty()) {
-            location = markedTextBefore.length();
-            length = selectedText.length();
+            markedTextRange.location = markedTextBefore.length();
+            markedTextRange.length = selectedText.length();
         }
     } else {
         if (startPosition != startOfEditableContent(startPosition)) {
@@ -2240,7 +2239,14 @@ WebAutocorrectionContext WebPage::autocorrectionContext()
                 contextAfter = plainTextReplacingNoBreakSpace(Range::create(*frame.document(), endPosition, nextPosition).ptr());
         }
     }
-    return { WTFMove(contextBefore), WTFMove(markedText), WTFMove(selectedText), WTFMove(contextAfter), location, length };
+
+    WebAutocorrectionContext correction;
+    correction.contextBefore = WTFMove(contextBefore);
+    correction.markedText = WTFMove(markedText);
+    correction.selectedText = WTFMove(selectedText);
+    correction.contextAfter = WTFMove(contextAfter);
+    correction.markedTextRange = WTFMove(markedTextRange);
+    return correction;
 }
 
 void WebPage::requestAutocorrectionContext()