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

Issue 690160 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Last visit > 30 days ago
Closed: Jun 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Blocking:
issue 608840



Sign in to add a comment

Broken connection between ChromeNewWindowClient and MultiUserWindowManager's IsUserEvent

Project Member Reported by warx@chromium.org, Feb 8 2017

Issue description

After CL: https://codereview.chromium.org/2434463004/,

https://cs.chromium.org/chromium/src/chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos.cc?type=cs&q=isprocessinguserevent&l=322 is broken because inside IsProcessingUserEvent, root window event dispatcher's current_event() is null.

Here is the broken point: https://cs.chromium.org/chromium/src/chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos.cc?type=cs&l=96

I don't have a strong opinion here right now. Some ideas?

Thanks!
 

Comment 1 by e...@chromium.org, Feb 8 2017

Cc: xiy...@chromium.org
Starting out, assuming that you can intuit whether you're handling a user event by inspecting the event dispatcher is very optimistic, and the comment above that block is a good example about timing problems this approach can have even before we start carving ash out of chrome, which makes things even more asynchronous. I suspect that an approach that assumes that an event is handled in one pass through the message loop is wrong.
What's the end-user-visible bug this causes? "Open link in new window" opens in wrong user session?

CL introduced the check: https://codereview.chromium.org/55303003
The issue it addresses is to ensure that dragging a tab off a visiting browse window of user B in user A's desktop does not end up in an invisible browser window. We would need a different solution.

Right now, the window to which user's desktop info is stored inside MultiUserWindowManagerChromeOS. This would not work for mash for sure. Maybe we should store it as part of the window, e.g. as a window property. This would make the code more mash ready. And for the original tab dragging problem, we can say the new browser window should inherit the property of where the tab is detached from.

Cc: -e...@chromium.org sky@chromium.org warx@chromium.org
Owner: e...@chromium.org
erg, can you take this? It's our mustash refactoring that broke it; it seems like we should fix it. I'm not sure that warx@ is any more familiar with this code than we are. If you're too busy I can take it.

warx@, if you've made progress on this feel free to take back the bug.

Project Member

Comment 5 by bugdroid1@chromium.org, Feb 22 2017

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

commit 7b01d69adba07ea22bc68412c023f9d27fa397e0
Author: erg <erg@chromium.org>
Date: Wed Feb 22 21:57:35 2017

ash: fix regression where ctrl+n put new window on wrong desktop

In multi-user mode on chromeos, we check whether a new window was
created by a user gesture by looking at the state of the MessageLoop and
the EventDispatcher. This doesn't work in an asynchronous world.

This patch replaces this check by manually annotating windows which were
created by user gesture.

BUG= 690160 
TBR=stevenjb@chromium.org

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

[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/app_controller_mac.mm
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/captive_portal/captive_portal_browsertest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/chromeos/first_run/goodies_displayer_browsertest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/download/download_crx_util.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/extensions/api/browser/browser_api.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/extensions/api/debugger/debugger_apitest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/extensions/api/developer_private/developer_private_api_unittest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/extensions/api/management/management_api_unittest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/extensions/api/permissions/permissions_api_unittest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/extensions/api/runtime/chrome_runtime_api_delegate.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/extensions/api/sessions/sessions_apitest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/extensions/api/tabs/tabs_api.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/extensions/api/tabs/tabs_api_unittest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/extensions/api/tabs/tabs_test.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/extensions/extension_context_menu_model_unittest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/extensions/extension_tab_util.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/extensions/extension_tab_util.h
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/media/webrtc/tab_desktop_media_list_unittest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/net/sdch_browsertest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/prefetch/prefetch_browsertest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/profiles/avatar_menu_actions_desktop.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/profiles/profile_manager_unittest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/safe_browsing/srt_fetcher_win.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/sessions/better_session_restore_browsertest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/sessions/session_restore.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/sessions/session_restore_browsertest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/sessions/session_restore_browsertest_chromeos.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ssl/security_state_tab_helper_browser_tests.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/sync/test/integration/sync_test.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/ash/accelerator_commands_browsertest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/ash/chrome_shell_delegate.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl_browsertest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl_unittest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos_unittest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/ash/window_positioner_unittest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/bookmarks/bookmark_bubble_sign_in_delegate.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/browser.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/browser.h
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/browser_browsertest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/browser_close_unittest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/browser_command_controller_unittest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/browser_commands.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/browser_finder_chromeos_unittest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/browser_focus_uitest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/browser_instant_controller_unittest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/browser_live_tab_context.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/browser_mac.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/browser_navigator.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/browser_navigator_browsertest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/browser_tab_strip_model_delegate.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/browser_window.h
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/chrome_pages.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/cocoa/applescript/browsercrapplication+applescript_test.mm
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/cocoa/applescript/window_applescript.mm
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/cocoa/apps/app_shim_menu_controller_mac_interactive_uitest.mm
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/cocoa/bookmarks/bookmark_editor_base_controller_unittest.mm
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_cocoa_controller.mm
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/cocoa/browser_window_controller_unittest.mm
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/cocoa/browser_window_factory_cocoa.mm
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/cocoa/profiles/avatar_icon_controller_unittest.mm
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/cocoa/profiles/profile_menu_controller_unittest.mm
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/cocoa/test/cocoa_profile_test.mm
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/extensions/application_launch.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/extensions/extension_message_bubble_bridge_unittest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/extensions/extension_message_bubble_browsertest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/find_bar/find_bar_host_browsertest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/scoped_tabbed_browser_displayer.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/startup/session_crashed_infobar_delegate_unittest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/startup/startup_browser_creator_browsertest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/startup/startup_browser_creator_impl.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/sync/one_click_signin_sync_starter.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/tab_contents/tab_contents_iterator_unittest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/tabs/pinned_tab_service_unittest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/toolbar/back_forward_menu_model_unittest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/toolbar/browser_actions_bar_browsertest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/views/bookmarks/bookmark_bar_view_test.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/views/frame/browser_frame_ash_browsertest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/views/frame/browser_view_browsertest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/views/frame/browser_view_focus_uitest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/views/frame/browser_view_interactive_uitest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/views/frame/browser_window_factory.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/views/tabs/tab_drag_controller.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/views/toolbar/browser_actions_container_browsertest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/webui/chrome_web_contents_handler.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/webui/options/sync_setup_handler.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/webui/settings/people_handler.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/webui/signin/signin_supervised_user_import_handler.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/browser/ui/window_sizer/window_sizer_ash_unittest.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/test/base/browser_with_test_window_test.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/test/base/in_process_browser_test.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/chrome/test/base/in_process_browser_test_mac.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/ui/aura/client/aura_constants.cc
[modify] https://crrev.com/7b01d69adba07ea22bc68412c023f9d27fa397e0/ui/aura/client/aura_constants.h

Comment 6 by e...@chromium.org, Jun 16 2017

Status: Fixed (was: Assigned)
(I appear to have never closed this one.)

Comment 7 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment