Mac: Fix IME predictive completions in content

Issue: Predictive completions in some IMEs are not working properly
when typing in a webpage.

Cause: Appkit <-(sync)-> RWHVCocoa <-(async)-> Blink

In an IME composition commit, RWHV commits the text to Blink
through an async mojo call. Later when Blink finishes commit, it updates
the text in RWHV through a separate mojo call.

For predictive completions to work, IMEs calls RWHV's
|attributedSubstringForProposedRange:| to retrieve some text
as the context for prediction. Oftentimes the call happens before
Blink's async update to RWHV, resulting in a wrong predictive context.

This issue troubles both Chromium and Firefox but not Safari/Webkit.
Webkit is using an undocumented async IME API.

Also after macOS 10.12+, IMEs expect |selectedRange:| to return a empty
range positioned at the end of the committed text. If not respected the
predictive completions won't work.

Solution:
- Temporarily insert the committed text to RWHV's available text
- Follow the |selectedRange:| requirement

Bug: 710101
Change-Id: I3e4267362ea3e0a6b9f7cebce23fe59d1a6ece6a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2606140Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Keren Zhu <kerenzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845832}

content/app_shim_remote_cocoa/render_widget_host_view_cocoa.h

@@ -135,10 +135,11 @@ struct DidOverscrollParams;
   // the whole content yet.
   NSRange _markedRange;
 
-  // The text selection, cached from the RenderWidgetHostView. This is only ever
-  // updated from the renderer.
-  base::string16 _textSelectionText;
-  size_t _textSelectionOffset;
+  // The text selection, cached from the RenderWidgetHostView.
+  // |_availableText| contains the selected text and is a substring of the
+  // full string in the renderer.
+  base::string16 _availableText;
+  size_t _availableTextOffset;
   gfx::Range _textSelectionRange;
 
   // The composition range, cached from the RenderWidgetHostView. This is only

content/app_shim_remote_cocoa/render_widget_host_view_cocoa.mm

@@ -277,7 +277,7 @@ void ExtractUnderlines(NSAttributedString* string,
   if (!textCheckingTypes)
     return;
 
-  NSString* availableText = base::SysUTF16ToNSString(_textSelectionText);
+  NSString* availableText = base::SysUTF16ToNSString(_availableText);
 
   if (!availableText)
     return;
@@ -295,7 +295,7 @@ void ExtractUnderlines(NSAttributedString* string,
   base::scoped_nsobject<NSTextCheckingResult> scopedCandidateResult;
   for (NSTextCheckingResult* result in textCheckingResults) {
     NSTextCheckingResult* adjustedResult =
-        [result resultByAdjustingRangesWithOffset:_textSelectionOffset];
+        [result resultByAdjustingRangesWithOffset:_availableTextOffset];
     if (!NSLocationInRange(cursorLocation,
                            NSMakeRange(adjustedResult.range.location,
                                        adjustedResult.range.length + 1)))
@@ -344,13 +344,13 @@ void ExtractUnderlines(NSAttributedString* string,
     return;
 
   NSRange availableTextRange =
-      NSMakeRange(_textSelectionOffset, _textSelectionText.length());
+      NSMakeRange(_availableTextOffset, _availableText.length());
 
   if (NSMaxRange(correction.range) > NSMaxRange(availableTextRange))
     return;
 
   NSAttributedString* attString = [[[NSAttributedString alloc]
-      initWithString:base::SysUTF16ToNSString(_textSelectionText)] autorelease];
+      initWithString:base::SysUTF16ToNSString(_availableText)] autorelease];
   NSRange trailingRange = NSMakeRange(
       NSMaxRange(correction.range),
       NSMaxRange(availableTextRange) - NSMaxRange(correction.range));
@@ -358,7 +358,7 @@ void ExtractUnderlines(NSAttributedString* string,
   if (trailingRange.length > 0 &&
       trailingRange.location < NSMaxRange(availableTextRange)) {
     NSRange trailingRangeInAvailableText = NSMakeRange(
-        trailingRange.location - _textSelectionOffset, trailingRange.length);
+        trailingRange.location - _availableTextOffset, trailingRange.length);
     if (@available(macOS 10.12, *)) {
       NSString* trailingString =
           [attString.string substringWithRange:trailingRangeInAvailableText];
@@ -386,12 +386,12 @@ void ExtractUnderlines(NSAttributedString* string,
   if (!touchBarItem.candidateListVisible)
     return;
   if (!_textSelectionRange.IsValid() ||
-      _textSelectionOffset > _textSelectionRange.GetMin())
+      _availableTextOffset > _textSelectionRange.GetMin())
     return;
 
   NSRange selectionRange = _textSelectionRange.ToNSRange();
-  NSString* selectionText = base::SysUTF16ToNSString(_textSelectionText);
-  selectionRange.location -= _textSelectionOffset;
+  NSString* selectionText = base::SysUTF16ToNSString(_availableText);
+  selectionRange.location -= _availableTextOffset;
   if (NSMaxRange(selectionRange) > selectionText.length)
     return;
 
@@ -446,8 +446,8 @@ void ExtractUnderlines(NSAttributedString* string,
 - (void)setTextSelectionText:(base::string16)text
                       offset:(size_t)offset
                        range:(gfx::Range)range {
-  _textSelectionText = text;
-  _textSelectionOffset = offset;
+  _availableText = text;
+  _availableTextOffset = offset;
   _textSelectionRange = range;
   _substitutionWasApplied = NO;
   [NSSpellChecker.sharedSpellChecker dismissCorrectionIndicatorForView:self];
@@ -466,7 +466,7 @@ void ExtractUnderlines(NSAttributedString* string,
     return;
   NSTextCheckingResult* selectedResult = anItem.candidates[index];
   NSRange replacementRange = selectedResult.range;
-  replacementRange.location += _textSelectionOffset;
+  replacementRange.location += _availableTextOffset;
   [self insertText:selectedResult.replacementString
       replacementRange:replacementRange];
 
@@ -488,13 +488,12 @@ void ExtractUnderlines(NSAttributedString* string,
 }
 
 - (base::string16)selectedText {
-  gfx::Range textRange(_textSelectionOffset,
-                       _textSelectionOffset + _textSelectionText.size());
+  gfx::Range textRange(_availableTextOffset,
+                       _availableTextOffset + _availableText.size());
   gfx::Range intersectionRange = textRange.Intersect(_textSelectionRange);
   if (intersectionRange.is_empty())
     return base::string16();
-  return _textSelectionText.substr(
-      intersectionRange.start() - _textSelectionOffset,
+  return _availableText.substr(intersectionRange.start() - _availableTextOffset,
                                intersectionRange.length());
 }
 
@@ -1824,19 +1823,9 @@ extern NSString* NSTextInputReplacementRangeAttributeName;
   gfx::Range expectedRange;
   const base::string16* expectedText;
 
-  if (!_compositionRange.is_empty()) {
-    // This method might get called after TextInputState.type is reset to none,
-    // in which case there will be no composition range information
-    // https://crbug.com/698672
-    expectedText = &_markedText;
-    expectedRange = _compositionRange.Intersect(
-        gfx::Range(_compositionRange.start(),
-                   _compositionRange.start() + expectedText->length()));
-  } else {
-    expectedText = &_textSelectionText;
-    size_t offset = _textSelectionOffset;
+  expectedText = &_availableText;
+  size_t offset = _availableTextOffset;
   expectedRange = gfx::Range(offset, offset + expectedText->size());
-  }
 
   gfx::Range gfxActualRange = expectedRange.Intersect(requestedRange);
   if (!gfxActualRange.IsValid())
@@ -1980,16 +1969,44 @@ extern NSString* NSTextInputReplacementRangeAttributeName;
   // Text inserting might be initiated by other source instead of keyboard
   // events, such as the Characters dialog. In this case the text should be
   // sent as an input method event as well.
-  // TODO(suzhe): It's hard for us to support replacementRange without accessing
-  // the full web content.
   BOOL isAttributedString = [string isKindOfClass:[NSAttributedString class]];
-  NSString* im_text = isAttributedString ? [string string] : string;
+  NSString* imText = isAttributedString ? [string string] : string;
   if (_handlingKeyDown && replacementRange.location == NSNotFound) {
-    _textToBeInserted.append(base::SysNSStringToUTF16(im_text));
+    // The user uses keyboard to type in a char without an IME or select a word
+    // on the IME. Don't commit the change to the render, because the event is
+    // being processed in |keyEvent:|. The commit will happen later after
+    // |interpretKeyEvents:| returns.
+    _textToBeInserted.append(base::SysNSStringToUTF16(imText));
     _shouldRequestTextSubstitutions = YES;
   } else {
+    // The user uses mouse or touch bar to select a word on the IME.
     gfx::Range replacement_range(replacementRange);
-    _host->ImeCommitText(base::SysNSStringToUTF16(im_text), replacement_range);
+    _host->ImeCommitText(base::SysNSStringToUTF16(imText), replacement_range);
+  }
+
+  if (replacementRange.location == NSNotFound) {
+    // Cancel selection after a IME commit by setting a zero-length selection
+    // at the end of insertion point.
+    // This is required for macOS 10.12+, otherwise the predictive completions
+    // of IMEs won't work. See crbug.com/710101.
+    int insertEndpoint = _markedRange.location + [imText length];
+    _textSelectionRange = gfx::Range(insertEndpoint, insertEndpoint);
+    // IMEs read |_availableText| preceding the insertion point as the context
+    // for predictive completion. Unfortunately by the moment IME reads the
+    // text, Blink likely hasn't finished the commit so the IME will read a
+    // wrong context. We hack it by temporarily inserting the committed text
+    // into
+    // |_availableText|. This variable will ultimately be asynchronously updated
+    // by Blink.
+    // TODO(crbug.com/1169288): Mac's IME API is synchronous and it plays badly
+    // with async APIs between the browser and the renderer. Probably replace
+    // the sync |interpretKeyEvents:| with the async
+    // |handleEventByInputMethod:|, which is an undocumented API used in
+    // Webkit2.
+    if (_markedRange.location >= _availableTextOffset &&
+        _markedRange.location <= _availableTextOffset + _availableText.length())
+      _availableText.insert(_markedRange.location - _availableTextOffset,
+                            base::SysNSStringToUTF16(imText));
   }
 
   // Inserting text will delete all marked text automatically.