Commit 9fc16442 authored by Trent Apted's avatar Trent Apted Committed by Commit Bot

MacViews: Always null-check BridgedContentView's textInputClient_

When a real IME window is up, AppKit spins a nested runloop that
interacts with the IME window over XPC while we're trying to
invalidate our NSTextInputClient (e.g. because the window has been
deleted). However, this may also cause AppKit to *use* the very
NSTextInputClient that we're trying to invalidate.

Specifically, this seems to happen with phonetic languages such as
Korean and Vietnamese. These use rule-based transforms that do not
commonly pop-up a candidate window. In such cases, a key such as
Enter may simultaneously commit a composition _and_ trigger the omnibox
action, which moves focus away from the omnibox thereby invalidating
the textInputClient_.

-BridgedContentView insertText: was the last hold-out that didn't
have a null check. AttributedSubstringForRangeHelper() and
GetFirstRectForRangeHelper() handle a null client already.

  SystemPreferences -> Keyboard -> Input Sources, then interact with
  the omnibox.

Test: On Mac, add/enable the "2-Set Korean" input method under
Bug: 817097
Change-Id: Ia7d1ebb796b9c98437b563ea405cad785e92ea30
Reviewed-on: https://chromium-review.googlesource.com/1124073Reviewed-by: default avatarElly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572396}
parent 33c9d271
......@@ -1325,6 +1325,14 @@ ui::TextEditCommand GetTextEditCommandForMenuAction(SEL action) {
// NSTextInputClient protocol implementation.
// IMPORTANT: Always null-check |textInputClient_|. It can change (or be
// cleared) in -setTextInputClient:, which requires informing AppKit that the
// -inputContext has changed and to update its raw pointer. However, the AppKit
// method which does that may also spin a nested run loop communicating with an
// IME window and cause it to *use* the exact same NSTextInputClient (i.e.,
// |self|) that we're trying to invalidate in -setTextInputClient:.
// See https://crbug.com/817097#c12 for further details on this atrocity.
- (NSAttributedString*)
attributedSubstringForProposedRange:(NSRange)range
actualRange:(NSRangePointer)actualRange {
......@@ -1383,13 +1391,9 @@ ui::TextEditCommand GetTextEditCommandForMenuAction(SEL action) {
}
- (void)insertText:(id)text replacementRange:(NSRange)replacementRange {
if (!hostedView_)
if (!hostedView_ || !textInputClient_)
return;
// Verify inputContext is not nil, i.e. |textInputClient_| is valid and no
// menu is active.
DCHECK([self inputContext]);
textInputClient_->DeleteRange(gfx::Range(replacementRange));
[self insertTextInternal:text];
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment