Easiest fix of course would be to just remove the check. I don't think we had a keyboard window size check in the original code.

The code above is in ChromeKeyboardWebContents::MaybeRunLoadCallback() in chrome_keyboard_web_contents.cc:

https://cs.chromium.org/chromium/src/chrome/browser/ui/ash/keyboard/chrome_keyboard_web_contents.cc?type=cs&q=ChromeKeyboardWebContents::MaybeRunLoadCallback&g=0&l=265

The expected behavior is:
a) Ash provides an initial width for the keyboard (1280x0).
b) The extension window sizes itself correctly based on the width. That triggers ChromeKeyboardWebContents::OnWindowBoundsChanged() which calls MaybeRunLoadCallback().
c) The callback provides the correct size to Ash.

Without the size check, the load callback may be called before the size change occurs and Ash will not be notified of the keyboard size.

The root cause seems to be that the extension is not sizing the window properly, or that resize is somehow not triggering OnWindowBoundsChanged().

However, IIRC, we made a change somewhere to inform the embed host when the window size changes, so passing |contents_size| through the load callback may no longer be necessary? I can investigate that.

+mukai@, +xiyuan@

FYI: crrev.com/c/1403923 is the CL you referred. The bounds change is propagated as keyboard webcontents --(WS)--> OnWindowBoundsChanged --> AshKeyboardUI::AshKeyboardView::OnWindowBoundsChanged --> GetWidget()->SetBounds(). You may need to add a few lines of code AshKeyboardUI::AshKeyboardView::onBoundsChanged for initial loading?

Are we sure the problem is because of the load callback? I just ran the test and it seems the VK really not showing up. Clicking on the omnibox manually makes it to show up.

Okay. For me, the test fails at vkb.IsShown, even before we try to click omnibox to trigger VK.

2019/01/14 10:24:15 [10:24:16.936] Error at virtual_keyboard_omnibox.go:38: Failed to check if the virtual keyboard is initially hidden: cdp.Runtime: Evaluate: context deadline exceeded

re #5, that is due to different  issue 921405 . Do you have the latest test code? I've updated the test code to circumvent that issue. Now it should fail at the correct step.

Right. I don't have https://chromium-review.googlesource.com/1404514. Thus saw the failure of stucking in vkb.IsShown.

However, if we have problem with automation api, omnibox.doDefault() [1] would not run and VK would not be triggered because VK only shows up if we have a mouse click (or tap) on omnibox.

[1] https://cs.corp.google.com/chromeos_public/src/platform/tast-tests/src/chromiumos/tast/local/bundles/cros/ui/virtual_keyboard_omnibox.go?rcl=c27298efdba27aa8c87a86c5ed6fbb5b799f8029&l=51

Yeah that is what I thought originally (that doDefault is broken). However, I deployed a custom build to my device with logging, and it appears that doDefault is working fine. In fact, we do get the click event and the VK actually entered into LOADING_EXTENSION, but got stuck there for some reason.

Also, doDefault is used by ChromeVox as well and it appears to work fine when I tested it.

I also removed the contents_size_ check and the test passes, so I don't think it's doDefault's problem.

However, I'm not exactly sure why clicking the omnibox manually afterwards fixes it though. I would assume that if it's stuck at LOADING_EXTENSION it would be stuck forever?

Do you think this is a separate issue from  issue 921405 , "chrome automation API not detecting the virtual keyboard"?

I think so, because even if you have the VK up on the screen (e.g. using the sticky a11y keyboard), the automation API can't seem to find it. So I don't think it's about the VK not showing.

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

commit 10d0f2afca046fcf4d35db0b5cf91836322928d2
Author: Steven Bennetts <stevenjb@chromium.org>
Date: Wed Jan 16 22:38:51 2019

SPM Keyboard: Allow contents size set after load.

Currently we wait for the keyboard contents size to be set before
invoking the load callback in
ChromeKeyboardWebContents::MaybeRunLoadCallback().

With crrev.com/c/1403923 this is no longer necessary,
AshKeyboardUI::AshKeyboardView observes the embedding host which
receives an event when the window bounds changes, correctly setting
the size after load.

This makes the keyboard startup flow more robust and fixes
tast.ui.VirtualKeyboardOmnibox for SPM.

Bug:  921410 
Change-Id: I165e23f5c1f19d9dbb2f965d425a57eb719d7e22
Reviewed-on: https://chromium-review.googlesource.com/c/1415811
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Darren Shen <shend@chromium.org>
Reviewed-by: Jun Mukai <mukai@chromium.org>
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623413}
[modify] https://crrev.com/10d0f2afca046fcf4d35db0b5cf91836322928d2/ash/keyboard/ash_keyboard_ui.cc
[modify] https://crrev.com/10d0f2afca046fcf4d35db0b5cf91836322928d2/chrome/browser/ui/ash/keyboard/chrome_keyboard_web_contents.cc

Note: The changes in comment #12 and issue 921405#6 are required for tast.ui.VirtualKeyboardOmnibox to pass with SPM enabled.

Great, thanks for your help!

Update: I'm not sure this change was required, but I think it makes things less fragile, so I think we should keep it.