With: https://chromium-review.googlesource.com/c/chromium/src/+/1300553

the virtual keyboard will be in a mostly working state with this as one of the main known issues.

Failing tests in the filter include:
-KeyboardEndToEndOverscrollTest.ToggleKeyboardOnShortOverlappingWindowMovesWindowUpwards
-KeyboardEndToEndOverscrollTest.ToggleKeyboardOnTallOverlappingWindowMovesWindowUpwardsAndAffectsViewport

Hmm, is this only under mash? I remember this working before...

Yes, thus the SingleProcessMash: prefix :) It works fine in classic ash.

I haven't been able to figure out what mechanism is responsible for this and decided not to block putting the CL to enable the VK for SingleProcessMash on fixing this.

Ah oops, missed that ;P

KeyboardController should ask the text field to move in textfield::EnsureCaretNotInRect https://cs.chromium.org/chromium/src/ui/views/controls/textfield/textfield.cc?sq=package:chromium&g=0&l=1666

If you follow that function, then somewhere it should calculate offsets and move the window.

Ah, so this may be another symptom of issue 631527 ( is only partially implemented).

+xiyuan@

That was supposed to be:

Ah, so this may be another symptom of issue 631527 (RemoteTextInputClient is only partially implemented).

I think EnsureCaretNotInRect not working is a separate issue than issue 631527. I filed  issue 906888  for the EnsureCaretNotInRect issue.

Investigating.

The root cause is:

In Classic Ash, in KeyboardController::EnsureCaretInWorkArea(), we call KeyboardUI::GetInputMethod()->GetTextInputClient() (which returns a RenderWidgetHostViewAura instance), ->EnsureCaretNotInRect(), which calls wm::EnsureWindowNotInRect().

In Mash, that returns a RemoteTextInputClient instance which has EnsureCaretNotInRect*() NOTIMPLEMENTED.

Even if we delegate get EnsureCaretInWorkArea to the Chrome client in Mash, when we get InputMethod through the Chrome side keyboard window's root window (keyboard_window->GetRootWindow->GetHost()->GetInputMethod()), it returns the same global singleton which has RemoteTextInputClient as the text input client.

We might be able to get the RenderWidgetHostView* for the keyboard web contents, but we would have to do an ugly cast to access the RenderWidgetHostViewAura interface.

A couple of approaches occur to me:
a) Make the IME code more Mash aware so that from Chrome, Window::GetRootWindow::GetHost::GetInputMethod::GetTextInputClient() returns RenderWidgetHostViewAura.
b) Call wm::EnsureWindowNotInRect() directly from the Chrome keyboard client. This would skip the rest of the implementation in RenderWidgetHostViewAura::EnsureCaretNotInRect(). I haven determined how important that is or whether that can be ported.
c) Fix RemoteTextInputClient to do the correct thing.

I tried (b) but it didn't actually move the window, which may be due to  issue 906888 , or the fact that the rest of the RenderWidgetHostViewAura::EnsureCaretNotInRect() implementation isn't getting called.

I suspect we probably want (c), but I'm not quite sure how that is intended to relate to RenderWidgetHostViewAura.

I vote for (c). RenderWidgetHostViewAura is a ui::TextInputClient. Implementing RemoteTextInputClient::EnsureCaretNotInRect should forward the call to it.

Looks like we need more than just (c). Under mash, KeyboardController is part of ash and use AshKeyboardUI. AshKeyboardUI::GetInputMethod would only be able to return the IME instance of ash (the default shared one). This IME in ash knows nothing about the TextInputClient running inside the browser process. Hence it would not be able properly move it.

We probably need (b) or something like it to make KeyboardController::EnsureCaretInWorkArea call into the browser process when needed.

I thought that was the point of RemoteTextInputClient, to "send updates via mojo", from Ash -> Chrome ?

We can certainly initiate the call from Chrome in ChromeKeyboardControllerClient::OnKeyboardOccludedBoundsChanged(), it's just not clear what that should call. (Just calling wm::EnsureWindowNotInRect() does not seem to do the trick).

RemoteTextInputClient runs inside the browser process for the real IME instance there to call the real TextInputClient running at the mojo app side. It is browser -> mojo app, not ash -> chrome. 

If ChromeKeyboardControllerClient::OnKeyboardOccludedBoundsChanged is hooked up, we could make make RemoteTextInputClient observe ChromeKeyboardControllerClient and forward it to real TextInputClient's EnsureCaretNotInRect.

OK, I see, so it sounds like this is actually blocked on issue 631527, and what we need to do is:

1. Have RemoteTextInputClient::EnsureCaretNotInRect() forward to the real TextInputClient's EnsureCaretNotInRect(). That should fix the problem for SPM since the InputMethod singleton in the same in Ash and Chrome.

2. For MPM, make sure that the call to InputMethod::GetTextInputClient() occurs on the Chrome side of things (that will be pretty straightforward).

Does that sound right? Do we want a separate tracking issue for (1)?

Re #15, I will use issue 631527 to implement RemoteTextInputClient::EnsureCaretNotInRect.

Proposal: https://chromium-review.googlesource.com/c/chromium/src/+/1366879

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ffb4d235d408e6e06f749e65406c2e652e9fa10c

commit ffb4d235d408e6e06f749e65406c2e652e9fa10c
Author: Xiyuan Xia <xiyuan@chromium.org>
Date: Mon Dec 10 17:47:06 2018

SingleProcessMash: Fix browser window move with VK

- Wire up RemoteTextInputClient::EnsureCaretNotInRect to forward
  the call to the actual TextInputClient;
- Fix ime_util_chromeos to handle windows under MUS mode;
- Change ChromeKeyboardBoundsObserver to use WidgetObserver
  to get bounds change of toplevel browser window under mash;

Bug: 631527,  906888 ,  906859 
Change-Id: I1570f8e3254e025106a120cf5ee538c20367c405
Reviewed-on: https://chromium-review.googlesource.com/c/1366879
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615164}
[modify] https://crrev.com/ffb4d235d408e6e06f749e65406c2e652e9fa10c/chrome/browser/ui/ash/keyboard/chrome_keyboard_bounds_observer.cc
[modify] https://crrev.com/ffb4d235d408e6e06f749e65406c2e652e9fa10c/chrome/browser/ui/ash/keyboard/chrome_keyboard_bounds_observer.h
[modify] https://crrev.com/ffb4d235d408e6e06f749e65406c2e652e9fa10c/chrome/browser/ui/views/ime_driver/input_method_bridge_chromeos_unittest.cc
[modify] https://crrev.com/ffb4d235d408e6e06f749e65406c2e652e9fa10c/chrome/browser/ui/views/ime_driver/remote_text_input_client.cc
[modify] https://crrev.com/ffb4d235d408e6e06f749e65406c2e652e9fa10c/services/ws/ime/ime_unittest.cc
[modify] https://crrev.com/ffb4d235d408e6e06f749e65406c2e652e9fa10c/services/ws/public/mojom/ime/ime.mojom
[modify] https://crrev.com/ffb4d235d408e6e06f749e65406c2e652e9fa10c/ui/aura/mus/text_input_client_impl.cc
[modify] https://crrev.com/ffb4d235d408e6e06f749e65406c2e652e9fa10c/ui/aura/mus/text_input_client_impl.h
[modify] https://crrev.com/ffb4d235d408e6e06f749e65406c2e652e9fa10c/ui/views/widget/desktop_aura/desktop_screen_position_client.cc
[modify] https://crrev.com/ffb4d235d408e6e06f749e65406c2e652e9fa10c/ui/wm/core/ime_util_chromeos.cc

CL above fixes the single process mash case. We need more work for the multi-process case. The part that KeyboardController finds relevant IME via AshKeyboardUI::GetInputMethod does not work properly under multi-process mash.

IME issues are tracked by issue 756059 and others. When that is addressed if we have similar issues we can open a separate issue for that. Marking this issue as resolved.