Browser windows pop to the foreground too aggressively |
||||||||||||||||||||
Issue description54.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.
,
Aug 19 2016
Assigning to grt@ to get their attention. Pop it back to Untriaged status when you're done.
,
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.
,
Aug 24 2016
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
,
Aug 24 2016
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!
,
Aug 25 2016
I'm not insane! Thanks for digging in.
,
Aug 26 2016
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.
,
Aug 26 2016
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.
,
Aug 27 2016
+ 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].
,
Aug 29 2016
,
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
,
Aug 31 2016
,
Aug 31 2016
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.
,
Sep 1 2016
Thanks. I'll keep an eye out.
,
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.
,
Sep 9 2016
grt: Can you try the steps in c#4 and c#5, and see if the problem reproduces with either?
,
Sep 12 2016
Issue 641853 has been merged into this issue.
,
Sep 12 2016
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
,
Sep 12 2016
Your change meets the bar and is auto-approved for M54 (branch: 2840)
,
Sep 12 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
,
Sep 14 2016
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,
,
Sep 14 2016
I can still repro with 55.0.2860.0 (Official Build) canary (64-bit).
,
Sep 14 2016
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?]
,
Sep 14 2016
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?
,
Sep 27 2016
Issue 650756 has been merged into this issue.
,
Oct 3 2016
Users are still reporting this on 54.0.2840.43
,
Oct 3 2016
Someone else should investigate the ChromeOS issue. Removing myself as OWNER to get this into the ChromeOS triage queue.
,
Oct 4 2016
warx@ can you take a look?
,
Oct 4 2016
Sure
,
Oct 4 2016
Issue 652837 has been merged into this issue.
,
Oct 4 2016
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
,
Oct 10 2016
Issue 639943 has been merged into this issue.
,
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.
,
Oct 11 2016
per comment 33
,
Oct 19 2016
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
,
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 |
||||||||||||||||||||
Comment 1 by erikc...@chromium.org
, Aug 16 2016