Hi,

Can we get an update on this bug?

Thanks.

raising priority due to impact on customers

Is this a regression or it has always been like that?

Hi,

We are not sure if it's regression because the customer only started using Chrome devices recently but it was always working with Chrome Browsers. We also do not have a way on our end to check if it was working with previous ChromeOS versions unless we ask the customer to recover some of their devices back to old versions.

Hope this helps.

I don't think it's a regression.
I've tried with a very old version 55.0.2883.105 (8872.76.0) and it's the same as the latest.
I've reproduced using pushed policy and user settings

Sounds like not a Blink issue.

Hi! Is there any update?

Hi,

Can we request an update on this? Our customer is waiting for the status of this bug.

Thanks.

Assigning to component owner

mfullante: There's no update because nobody has taken ownership of this issue yet.

tkent: Can you try to help triage this again from the Blink side? This does sound like a Blink issue to me. Settings just tells the browser to load a given URL at startup. How it behaves w.r.t. <input> focus at start up is still up to Blink.

BTW, one does not need a Chromebook to reproduce this issue. Chrome for Chrome OS builds on Linux can reproduce this as well.

I confirmed that autofocus processing for <input> was correctly handled.  However blink::WebView doesn't have focus [1], focus ring is not drawn, and keyboard events are not delivered to the <input>.

[1] It means blink::WebView::SetFocus(true) is not called.

So This is an issue of CrOS startup code.

tkent: Thanks for the info.

In the browser process, on Linux, the call stack is:
#2 content::RenderWidgetHostImpl::SetPageFocus()
#3 content::RenderWidgetHostImpl::Focus()
#4 content::RenderWidgetHostImpl::GotFocus()
#5 content::RenderWidgetHostViewAura::OnWindowFocused()
#6 wm::FocusController::SetFocusedWindow()
#7 wm::FocusController::FocusAndActivateWindow()
#8 wm::FocusController::FocusWindow()
#9 aura::Window::Focus()
#10 content::RenderWidgetHostViewAura::Focus()
#11 content::WebContentsViewAura::Focus()
#12 content::WebContentsViewAura::SetInitialFocus()
#13 content::WebContentsViewAura::RestoreFocus()
#14 content::WebContentsImpl::RestoreFocus()
#15 BrowserView::RestoreFocus()
#16 BrowserView::Show()
#17 StartupBrowserCreatorImpl::OpenTabsInBrowser()

This triggers WidgetInputHandlerImpl::SetFocus() on the render side, which eventually triggers WebViewImpl::SetFocus().

For Chrome OS, the execution path diverges FocusController::SetFocusedWindow(). Instead of calling content::RenderWidgetHostViewAura::OnWindowFocused(), it calls NativeWidgetAura::OnWindowFocused(). I didn't keep going to see where that goes.

sadrul / msw: Any idea what's going on here?

Maybe not calling RWHVA::OnFocus is a cause/symptom and not expected behavior?
You can print the aura window hierarchy with --ash-debug-shortcuts and Ctrl-Alt-Shift-W.

When opening a browser and simply navigating to that page, you get this (looks good):
"" (0x3d26948bc2c0) type=1 [active] visible 221,112 467x240 [snapped] subpixel offset=[0.333333 0.000000]
   NativeViewHostAuraClip (0x3d26938dd2c0) type=3 visible 0,110 467x130
      WebContentsViewAura (0x3d26938dd9a0) type=3 visible 0,0 467x130
         RenderWidgetHostViewAura (0x3d269402a580) type=3 [focused] visible 0,0 467x130 [snapped]

When using that pages as a startup tab, focus is on the browser window:
"" (0x28dcd91e6b00) type=1 [active] [focused] visible 254,112 733x380 [snapped] subpixel offset=[-0.363631 -0.181822]
   NativeViewHostAuraClip (0x28dcd93dd160) type=3 visible 0,110 733x270
      WebContentsViewAura (0x28dcd93422c0) type=3 visible 0,0 733x270
         RenderWidgetHostViewAura (0x28dcda59a2c0) type=3 visible 0,0 733x270 [snapped]

Is it the RenderWidgetHostViewAura window that's being passed to FocusController::SetFocusedWindow?
(is that SetFocusedWindow itself failing, or is it being called with an unexpected window?)

Steven, could you please triage.

This is not an area I have any recent familiarity with. +oshima@, +estade@ in case they have any thoughts.

ChromeOS startup tabs behave a little differently from desktop chrome. There is usually a login screen when StartupBrowserCreatorImpl creates the startup tabs (unless it is a browser restart case). The browser windows are created behind the login screen and receive focus when the login screen is dismissed. Maybe we did not set focus to RWHVAura in this case.

Am I correct that "Pages to Load on Startup" menas "Open a specific page or set of pages" on "On Startup" settings?


Assuming that's the case, a couple of observations:

* It works fine with session restore ("Continue where you left off").
 That is, logout with that page opened, then login.

* For both "Continue where you left off" and "Open a specific page o set of pages",
 I can see that page is loading then textfield appears, so the login screen may not
 be related (although not 100% certain).

* Html focus request doesn't move the focus from native ui to web contents, so we may be
 just starting brower window in different way when the startup page is specified.
 (and BrowserView::GetInitiallyFocusedView() returns nullptr) +sky@ in case he knows.

Hi,

Can we get an update on the status of this please. Our customer is really waiting to have this resolved and wants to know if there's any update.

Thank you.

@sky, could you take a look at #21? Your name was mentioned, as a potential person who might know what is going on. Feel free to un-assign if not. 

My guess is that when the logic screen is visible we are not allowing the browser window to become active, which is likely throwing off focus. I recommend tracing through the call to focus the aura::Window from the page. Most likely something in BaseFocusRules/AshFocusRules CanActivateWindow/CanFocusWindow is returning false.

Thanks for the response. Hopefully someone can make progress based on that information. (I am not the right person to follow up with it though, was just trying to help).

sky@ who can we assign this to? 

Someone on the ChromeOS UI team should be able to investigate this.

Let me give it a try.

The problem is related to the login screen. It is not the session state but rather the login screen widget is still there when the call stack in #16 is invoked. Hence, RenderWidgetHostViewAura::Focus() fails because the login screen is the top-most top-level window and takes the focus.

After login screen is dismissed, the browser frame becomes the active widget. But we don't move focus RWHV as msw@ observed in #17.

The problem is gone if I add RestoreFocus() to BrowserView::OnWidgetActivationChanged. Does that sound like a right fix?

Strange thing is that if we go session restore code path, RenderWidgetHostViewAura::Focus() succeeds with almost the same underlying stack from sign-in as oshima@ observed.

I'll dig more.

The difference between session restore and start up page case is that session restore code has a SetInitialFocus call after BrowserView::Show [1].

I think we want to RestoreFocus at a proper timing during BrowserView::Show. In current code, we do it before showing the browser window [2]. From the comment above RestoreFocus, it would be a no-op if the browser window is not visible. We probably should move it. OnWidgetActivationChanged looks like a reasonable place.

[1] https://cs.chromium.org/chromium/src/chrome/browser/sessions/session_restore.cc?rcl=2808de2075afd9537de964bb07e1f7d9c8b81e5b&l=719

[2] https://cs.chromium.org/chromium/src/chrome/browser/ui/views/frame/browser_view.cc?rcl=0fdad893c8889305deafb6f08d7fd1ca18df5231&l=684

Simple fix that follows session restore code:
https://chromium-review.googlesource.com/c/chromium/src/+/1330764

However, I think we could use the chance to clean up BrowserView::Show code, which seems to be challenging.
https://chromium-review.googlesource.com/c/chromium/src/+/1313728

The test should work for both fixes.

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

commit 2a66b11ee57844b24718b518f170706bc9242eab
Author: Xiyuan Xia <xiyuan@chromium.org>
Date: Thu Dec 06 15:52:16 2018

cros: Fix startup page focus

- Move RestoreFocus from BrowserView::Show to OnWidgetActivationChanged
  to do it when the browser window is activated;
- Remove SetInitialFocus in SessionRestoreImpl::ShowBrowser since
  BrowserView change should do that as part of RestoreFocus;
- Also remove the debugging code for SetInitialFocus crash
  under SessionRestoreImpl::RestoreTab since ShowBrowser no longer
  calls SetInitialFocus;

Bug:  859257 , 850626,  908524 
Change-Id: Ia24e4d68e1386de84765f914bb75319c9887dfd1
Reviewed-on: https://chromium-review.googlesource.com/c/1313728
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614364}
[modify] https://crrev.com/2a66b11ee57844b24718b518f170706bc9242eab/chrome/browser/sessions/session_restore.cc
[modify] https://crrev.com/2a66b11ee57844b24718b518f170706bc9242eab/chrome/browser/ui/startup/startup_browser_creator_interactive_uitest.cc
[modify] https://crrev.com/2a66b11ee57844b24718b518f170706bc9242eab/chrome/browser/ui/views/frame/browser_view.cc
[modify] https://crrev.com/2a66b11ee57844b24718b518f170706bc9242eab/chrome/browser/ui/views/frame/browser_view.h
[modify] https://crrev.com/2a66b11ee57844b24718b518f170706bc9242eab/chrome/browser/ui/views/frame/browser_view_interactive_uitest.cc
[modify] https://crrev.com/2a66b11ee57844b24718b518f170706bc9242eab/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
[modify] https://crrev.com/2a66b11ee57844b24718b518f170706bc9242eab/chrome/browser/ui/views/toolbar/toolbar_view_interactive_uitest.cc
[modify] https://crrev.com/2a66b11ee57844b24718b518f170706bc9242eab/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/2a66b11ee57844b24718b518f170706bc9242eab/content/browser/web_contents/web_contents_impl.h