New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 634248 link

Starred by 14 users

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Browser windows pop to the foreground too aggressively

Project Member Reported by grt@chromium.org, Aug 4 2016

Issue description

54.0.2817.0 (Official Build) canary (64-bit)

Possibly started on canary within the last few days. Let's say I have two browser windows from two different profiles hosting 10-20 tabs with session restore turned on for each. If I close one of these windows, then do something like "Open link as" from the other to open some link, the new browser window pops up as expected. I then immediately Cmd-` to switch back to the browser I started with. As the new browser starts doing its loads, it brings itself back to the foreground. I Cmd-` to go back to where I was, and it pops back to the FG again. Super ultra duper annoying. I don't recall this happening last week, so perhaps something changed recently.

I have no idea who might own this, so CCing pinkerton and rpop to triage/assign as appropriate. Thanks.
 
Labels: Needs-Feedback
I've tried unsuccessfully to reproduce this issue on two different machines. Maybe this is a timing issue? 

Alternatively, I have had problems with Window focus on multiple virtual desktops, specifically with the Omnibox. e.g. somehow the omnibox of one window retains focus, even when another window is in the "foreground" in a different virtual desktop. Attempting to type a character will automatically change desktops and focus the omnibox. Does any of this sound related to your problem?
Owner: grt@chromium.org
Status: Assigned (was: Untriaged)
Assigning to grt@ to get their attention. Pop it back to Untriaged status when you're done.

Comment 3 by grt@chromium.org, Aug 19 2016

Crumb. I can't repro it right now, either, although it happened to me earlier today. The next time it happens I'll try really hard to get a very specific set of steps to repro. Thanks for taking a stab at it.

Comment 4 by grt@chromium.org, Aug 24 2016

Labels: -Needs-Feedback
Owner: erikc...@chromium.org
It just happened to me again. Here's what I did:
- hmm, canary is out of date. tap the hotdogs menu from a Browser viewing Profile N and restart Chrome
- Chrome relaunches
- the three Browsers i saw before open: primary/corp, chromium, and "Profile N"
- minimize the first two
- start interacting with "Profile N"
- primary/corp Browser un-minimizes itself and brings itself to the foreground
- wha!?!?
Hope this helps
Status: Started (was: Assigned)
Thanks for following up!

Here's what I'm observing (pretty much deterministically):

1) Open three profiles. Have gmail open in each of them. Have each profile set to "Continue where you left off"
2) Quit Chrome. 
3) Relaunch Chrome.
4) Minimize each of the windows as quickly as possible.
5) Notice that when gmail finishes loading (on one of the windows), it unminimizes!

Comment 6 by grt@chromium.org, Aug 25 2016

I'm not insane! Thanks for digging in.
While I haven't been able to reproduce this exact problem on a Chromium build, I've got something comparable. Repro steps:

1) Build Chromium, with 1 modification. Add "sleep(1);" to the top of RenderFrameImpl::OpenURL. The bug is still present without this change, but it makes the bug a lot easier to notice.
2) Create two windows. In the first window, navigate to a new page. Immediately switch focus to the second window using Cmd + `.
3) Wait 1 second. Notice that focus bounces back to the first window.

With the patch in (1), if we try to restore a session with 2 profiles, each one with a single tab navigated to gmail.com, we see that focus hops between the two windows more than 10 times!

Tracing through the actual logic:
1) The renderer calls RenderFrameImpl::OpenURL a bunch of times.
2) Browser receives message, eventually calls chrome::Navigate, which calls Browser::UpdateUIForNavigationInTab.
3) This has the logic:
"""
 918   if (contents_is_selected)                                                     
 919     contents->SetInitialFocus();
"""

Doing some archaeology...

"""
Fix focus for in-tab navigations: (--enable-instant-extended-api)
https://chromiumcodereview.appspot.com/12390042
"""

this CL added the call to SetInitialFocus(). The previous call was to SetFocus(). instant-extended breaks the expected interaction focus model for all other web pages, so I'm not surprised that it needed to make some changes. The point of this is to fix bugs involving omnibox focus when navigating to/from NTP.

Going back further...

"""
Add support for SINGLETON_TAB disposition to BrowserNavigator.
https://codereview.chromium.org/3734003
"""
This was from five years ago. It's hard to tell, but I think the initial purpose of this call was to make sure that the appropriate content widget was "focused" after a navigation. 

Now let's look at the implementation of contents->SetInitialFocus();.

"""
216 void WebContentsViewMac::Focus() {                                              
217   gfx::NativeView native_view = GetNativeViewForFocus();                        
218   NSWindow* window = [native_view window];                                      
219   [window makeFirstResponder:native_view];                                      
220   if (![window isVisible])                                                      
221     return;                                                                                                                                                                                                                                                        
222   [window makeKeyAndOrderFront:nil];                                            
223 }                                                                               
224                                                                                 
225 void WebContentsViewMac::SetInitialFocus() {                                    
226   if (web_contents_->FocusLocationBarByDefault())                               
227     web_contents_->SetFocusToLocationBar(false);                                
228   else                                                                          
229     Focus();                                                                    
230 }     
"""

Either [focus the location bar], or [focus some other widget AND activate the Window]. Huh? Tracing the history, we see:

"""
When you clicked Get Themes in Mac prefs, the browser window containing the themes gallery wasn't coming to the front.

There was an implicit assumption on Windows that TabContentsViewWin::Focus caused the window containing the TabContents to be foregrounded. This is because on Windows a HWND is focused with a call to SetFocus, which activates the containing top level window. On Mac, TabContentsViewMac::Focus needs to explicitly activate the containing window.
https://codereview.chromium.org/165492
"""

Ok, makes sense. It doesn't make much sense to focus a widget without also activating a Window.

So it seems like when the foreground tab of a backgrounded window loads, it shouldn't cause the window to activate.

Looking at this CL:
"""
Fixing activation problem: Pages which refresh themselves should not get focus
https://chromiumcodereview.appspot.com/10409034
"""

"""
 2700   // Focus on the content if the content is active and it is user initated
 2701   // or if the window is active as well as the tab - or in other words:
 2702   // Don't focus when the user did not initate the navigation or the window
 2703   // and tab are not active.
 2704   if (contents_is_selected && (user_initiated || window()->IsActive()))
 2705     contents->web_contents()->Focus();
"""

Looks like I'm not the first person with this idea. Unfortunately, the CL was reverted and the bug was still marked as fixed:
https://chromiumcodereview.appspot.com/10425004
https://bugs.chromium.org/p/chromium/issues/detail?id=122392

It is true that I can no longer reproduce the bug, so it was presumably fixed in another way.
Cc: rjkroege@chromium.org sky@chromium.org e...@chromium.org fsam...@chromium.org
+ sky, who will be the eventual reviewer.

Here's the first solution I'm going to try: 
"""
Don't allow navigations in inactive windows to change window focus.
https://codereview.chromium.org/2284573004/#
"""

When this solution inevitably breaks...I guess I'll have to do more digging.

+ fsamuel, erg, rjkroege: I know that you've started looking into focus for MUS. Is there a consistent set of terminology and abstractions we've started to use for focus? This is pretty clearly missing from Chrome right now.

* We lack the ability to set the widget to be focused in a Window, when it is next activated. [right now we focus & activate immediately]. 

Cc: shrike@chromium.org
Project Member

Comment 11 by bugdroid1@chromium.org, Aug 30 2016

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

commit 0de48e14a7008fa85d9d74439a7dd63b413abb13
Author: erikchen <erikchen@chromium.org>
Date: Tue Aug 30 22:46:32 2016

Fix a bug where inactive windows would inappropriately take focus.

On Windows, it's not possible to focus a widget without activating the window.
As a result, the semantics for RenderWidgetHostView::Focus include window
activation on all platforms.

The previous implementation for UpdateUIForNavigationInTab would focus the web
contents, even if the window is backgrounded. This would activate the window.
This manifested itself most clearly during session restore for two windows, when
activation would bounce between the two windows, sometimes as many as 10 times!.

The new implementation only focuses the contents if the window is activated, or
about to be activated. If the window is backgrounded, then the appropriate
content will be focused when the user chooses the activate the window.

BUG= 634248 

Review-Url: https://codereview.chromium.org/2284573004
Cr-Commit-Position: refs/heads/master@{#415472}

[modify] https://crrev.com/0de48e14a7008fa85d9d74439a7dd63b413abb13/chrome/browser/ui/browser.cc
[modify] https://crrev.com/0de48e14a7008fa85d9d74439a7dd63b413abb13/chrome/browser/ui/browser.h
[modify] https://crrev.com/0de48e14a7008fa85d9d74439a7dd63b413abb13/chrome/browser/ui/browser_navigator.cc
[modify] https://crrev.com/0de48e14a7008fa85d9d74439a7dd63b413abb13/chrome/test/base/web_ui_browser_test.cc

Status: Fixed (was: Started)
grt: This fixed the problem for me. Let me know if you're still experiencing this - I'm willing to believe that there are other similar issues lurking in the ocdebase.

Comment 14 by grt@chromium.org, Sep 1 2016

Thanks. I'll keep an eye out.

Comment 15 by grt@chromium.org, Sep 9 2016

Hi Erik. It just happened again in 55.0.2854.2 (Official Build) canary (64-bit) after restarting. The browser window hosting my corp inbox popped in front of a browser for a different profile I'd just opened.
Status: Assigned (was: Fixed)
grt: Can you try the steps in c#4 and c#5, and see if the problem reproduces with either? 
Cc: abodenha@chromium.org warx@chromium.org erikc...@chromium.org skuhne@chromium.org kuscher@chromium.org jamescook@chromium.org osh...@chromium.org
 Issue 641853  has been merged into this issue.
Labels: Merge-Request-54
My fix in #11 has fixed the issue for some (but not all) users, it appears. https://bugs.chromium.org/p/chromium/issues/detail?id=639943#c23

Comment 19 by dimu@chromium.org, Sep 12 2016

Labels: -Merge-Request-54 Merge-Approved-54 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M54 (branch: 2840)
Project Member

Comment 20 by bugdroid1@chromium.org, Sep 12 2016

Labels: -merge-approved-54 merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c6d4e388aea582cb1cf12a95c3366186d0842b6f

commit c6d4e388aea582cb1cf12a95c3366186d0842b6f
Author: Erik Chen <erikchen@chromium.org>
Date: Mon Sep 12 20:49:24 2016

[Merge to 2840] Fix a bug where inactive windows would inappropriately take focus.

> On Windows, it's not possible to focus a widget without activating the window.
> As a result, the semantics for RenderWidgetHostView::Focus include window
> activation on all platforms.
>
> The previous implementation for UpdateUIForNavigationInTab would focus the web
> contents, even if the window is backgrounded. This would activate the window.
> This manifested itself most clearly during session restore for two windows, when
> activation would bounce between the two windows, sometimes as many as 10 times!.
>
> The new implementation only focuses the contents if the window is activated, or
> about to be activated. If the window is backgrounded, then the appropriate
> content will be focused when the user chooses the activate the window.
>
> BUG= 634248 
>
> Review-Url: https://codereview.chromium.org/2284573004
> Cr-Commit-Position: refs/heads/master@{#415472}
> (cherry picked from commit 0de48e14a7008fa85d9d74439a7dd63b413abb13)

Review URL: https://codereview.chromium.org/2331153002 .

Cr-Commit-Position: refs/branch-heads/2840@{#311}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/c6d4e388aea582cb1cf12a95c3366186d0842b6f/chrome/browser/ui/browser.cc
[modify] https://crrev.com/c6d4e388aea582cb1cf12a95c3366186d0842b6f/chrome/browser/ui/browser.h
[modify] https://crrev.com/c6d4e388aea582cb1cf12a95c3366186d0842b6f/chrome/browser/ui/browser_navigator.cc
[modify] https://crrev.com/c6d4e388aea582cb1cf12a95c3366186d0842b6f/chrome/test/base/web_ui_browser_test.cc

Cc: kavvaru@chromium.org
Labels: Needs-Feedback
Tested the issue on Mac 10.11.6 using chrome version 54.0.2840.27 with the steps mentioned in comment $5.

1) Open three profiles. Have gmail open in each of them. Have each profile set to "Continue where you left off"
2) Quit Chrome. 
3) Relaunch Chrome.
4) Observed only the last profile window opened.

Also tested the below steps #7

1) Create two windows. In the first window, navigate to a new page. Immediately switch focus to the second window using Cmd + `.
2) Wait 1 second. Observed the focus is not going back to first window.

Unable to reproduce the issue on reported version 54.0.2817.0  as well.

erikchen@ could you please check and confirm anything missed here? Please provide us any test steps to verify the issue from test team end.

Thanks,

Comment 22 by grt@chromium.org, Sep 14 2016

I can still repro with 55.0.2860.0 (Official Build) canary (64-bit).
grt: Can you provide more information about the nature of the problem? Does it happen during session restore? What do the symptoms look like? [Does the minimize issue still occur?]
grt: The closest problem I can find the is the following:

On macOS, windows get restored sequentially during session restore. Once all windows are restored, Chrome will activate the last used profile. If you've been interacting with another Window in the interim, your window will lose focus.
https://cs.chromium.org/chromium/src/chrome/browser/ui/startup/startup_browser_creator.cc?q=chrome/browser/ui/startup/startup_browser_creator.cc&sq=package:chromium&l=771

Does this sound like the problem you're experiencing?
 Issue 650756  has been merged into this issue.
Labels: OS-Chrome
Users are still reporting this on 54.0.2840.43
Labels: -OS-Mac
Owner: ----
Status: Untriaged (was: Assigned)
Someone else should investigate the ChromeOS issue. Removing myself as OWNER to get this into the ChromeOS triage queue.
Components: -UI UI>Browser>Sessions
Labels: ReleaseBlock-Stable
Owner: warx@chromium.org
Status: Assigned (was: Untriaged)
warx@ can you take a look?

Comment 29 by warx@chromium.org, Oct 4 2016

Status: Started (was: Assigned)
Sure
 Issue 652837  has been merged into this issue.
In case you need another way to repro on 54 on ChromeOS (from  issue 652837 ):

(0) Open lots of website tabs
(1) Crash your browser (easy to do on 54)
(2) Click the restore button
(3) While tabs are loading, attempt to chat in the hangouts window, or type into a feedback report

Comment 32 by warx@chromium.org, Oct 10 2016

Cc: ligim...@chromium.org creis@chromium.org pucchakayala@chromium.org songsuk@chromium.org krajshree@chromium.org nasko@chromium.org
 Issue 639943  has been merged into this issue.

Comment 33 by warx@chromium.org, Oct 11 2016

Please help verify if the issue still happens after 54.0.2840.47. https://chromium.googlesource.com/chromium/src/+/c8ca51e469c96ab4a3e1b0cd9772687077b38984 might be the fix.

That could explain why I cannot repro on tot, starting from Oct 4.

I can repro it on 54.0.2840.43, not 54.0.2840.47.

Comment 34 by warx@chromium.org, Oct 11 2016

Status: Fixed (was: Started)
per comment 33
Status: Verified (was: Fixed)
ChromeOS m55 dev | minnie

Tried one user with 40+ tabs 
chrome://inducebrowsercrashforrealz
hit Restore
Select on the tabs in the middle
It stayed in the foreground the whole time 

Tried multiprofile with above steps and it worked fine too

Tried one user with multiple browser windows with 40+ tabs
Always stayed on the active windows active tab 
Project Member

Comment 36 by bugdroid1@chromium.org, Oct 27 2016

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

commit c6d4e388aea582cb1cf12a95c3366186d0842b6f
Author: Erik Chen <erikchen@chromium.org>
Date: Mon Sep 12 20:49:24 2016

[Merge to 2840] Fix a bug where inactive windows would inappropriately take focus.

> On Windows, it's not possible to focus a widget without activating the window.
> As a result, the semantics for RenderWidgetHostView::Focus include window
> activation on all platforms.
>
> The previous implementation for UpdateUIForNavigationInTab would focus the web
> contents, even if the window is backgrounded. This would activate the window.
> This manifested itself most clearly during session restore for two windows, when
> activation would bounce between the two windows, sometimes as many as 10 times!.
>
> The new implementation only focuses the contents if the window is activated, or
> about to be activated. If the window is backgrounded, then the appropriate
> content will be focused when the user chooses the activate the window.
>
> BUG= 634248 
>
> Review-Url: https://codereview.chromium.org/2284573004
> Cr-Commit-Position: refs/heads/master@{#415472}
> (cherry picked from commit 0de48e14a7008fa85d9d74439a7dd63b413abb13)

Review URL: https://codereview.chromium.org/2331153002 .

Cr-Commit-Position: refs/branch-heads/2840@{#311}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/c6d4e388aea582cb1cf12a95c3366186d0842b6f/chrome/browser/ui/browser.cc
[modify] https://crrev.com/c6d4e388aea582cb1cf12a95c3366186d0842b6f/chrome/browser/ui/browser.h
[modify] https://crrev.com/c6d4e388aea582cb1cf12a95c3366186d0842b6f/chrome/browser/ui/browser_navigator.cc
[modify] https://crrev.com/c6d4e388aea582cb1cf12a95c3366186d0842b6f/chrome/test/base/web_ui_browser_test.cc

Sign in to add a comment