Mac: Fix a crash in TextInputClientMac::GetFirstRectForRange() likely during tab closure

Mac: Fix a crash in TextInputClientMac::GetFirstRectForRange() likely during tab closure

Delayed events from IME / HIToolbox can arrive for an NSView after
RenderWidgetHostViewMac::Destroy() has been called. In this case, the
render_widget_host_ has been set to null, which can result in a crash
under TextInputClientMac::GetFirstRectForRange() when it tries to
determine the RenderWidgetHostImpl from a null pointer.

Per suggestions/analysis in the bug, to fix, add a null check in
RenderWidgetHostViewCocoa's implementation of -[NSTextInputClient
firstRectForCharacterRange:actualRange]. Then, return NSZeroRect. Rather
than crashing the browser, this would have the effect of showing the IME
window in the bottom left of the window.

BUG=481281
Committed: https://crrev.com/d46c7beeacc54dac31acb1865963cefb96a0b064
Cr-Commit-Position: refs/heads/master@{#350261}

[tapted, 2015-09-21 05:18:09]

tapted@chromium.org changed reviewers:
+ asvitkine@chromium.org

[tapted, 2015-09-21 05:41:25]

Hi Alexei, please take a look.

[Alexei Svitkine (slow), 2015-09-21 15:09:16]

Bottom-left doesn't seem ideal to me. If the window still exists, can we return
a position at the top of the content area instead?

Alternatively, can we somehow make sure the IME input isn't shown at all?

Also, is there a way to test this with a unit test?

[groby-ooo-7-16, 2015-09-21 18:16:08]

groby@chromium.org changed reviewers:
+ groby@chromium.org

[groby-ooo-7-16, 2015-09-21 18:16:08]

Do we really care about the placement? It's during tab closure only that this
can be entered, if the comments are to be trusted

[tapted, 2015-09-22 07:25:55]

Patchset #3 (id:40001) has been deleted

[tapted, 2015-09-22 07:28:17]

On 2015/09/21 15:09:16, Alexei Svitkine (slow) wrote:
> Bottom-left doesn't seem ideal to me. If the window still exists, can we
return
> a position at the top of the content area instead?

RenderWidgetHostViewMac::Destroy() also does [cocoa_view_ removeFromSuperview],
so there's no nice way to reach the NSWindow or determine the view's real
position on screen.

> Alternatively, can we somehow make sure the IME input isn't shown at all?

Done - [self cancelComposition] should do this, and looks safe.

> Also, is there a way to test this with a unit test?

Done.

[Alexei Svitkine (slow), 2015-09-22 14:55:59]

LGTM++

[groby-ooo-7-16, 2015-09-22 19:57:14]

LGTM, since I accidentally ended up on reviewers.

[tapted, 2015-09-22 22:53:14]

The CQ bit was checked by tapted@chromium.org

[commit-bot: I haz the power, 2015-09-22 22:53:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1357883002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1357883002/60001

[commit-bot: I haz the power, 2015-09-22 23:28:57]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2015-09-22 23:29:24]

Patchset 3 (id:??) landed as
https://crrev.com/d46c7beeacc54dac31acb1865963cefb96a0b064
Cr-Commit-Position: refs/heads/master@{#350261}