New issue
Advanced search Search tips

Issue 891489 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 23
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 897058



Sign in to add a comment

ash Accelerator is invoked even when the event is processed / handled

Project Member Reported by mukai@chromium.org, Oct 2

Issue description

- start chrome with --enable-features=SingleProcessMash
- open a file browser, set the keyboard focus on it
- hit ctrl-n

expected:
- a new file-browser window opens

what I see:
- a new browser window and a new file-browser windwo open at the same time


It seems this is happening because the key event is processed by the file browser window (its web-contents), however the key event isn't marked as stopped_propagation(), therefore InputMethodDelegate invokes ack_callback with false (https://cs.chromium.org/chromium/src/ui/base/ime/input_method_delegate.cc?q=ui/base/ime/input_method_delegate&sq=package:chromium&g=0&l=18), which causes WindowServiceDelegateImpl::OnUnhandledKeyEvent() which invokes Ash's accelerator (https://cs.chromium.org/chromium/src/ash/ws/window_service_delegate_impl.cc?q=ash/ws/window_service_delegate&sq=package:chromium&g=0&l=96).


Changing InputMethodDelegate to use event->handled() instead of stopped_propagation() isn't a good idea since some other accelerators (like Ctrl-t) will be stopped unexpectedly.


This symptom is also highly related to BrowserCommandControllerInteractiveTest failures. Once this gets fixed, BrowserCommandControllerInteractiveTest will pass again.
 
Status: Started (was: Assigned)
Cc: mukai@chromium.org sky@chromium.org
 Issue 891473  has been merged into this issue.
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 11

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

commit b1729c64a26c645ed1c69b24f9512c7be77fae6a
Author: Jun Mukai <mukai@chromium.org>
Date: Thu Oct 11 23:25:04 2018

suppress a test for single_process_mash_interactive_ui_tests

KeyboardLockInteractiveBrowserTest.CancelActiveKeyboardLockBeforeFullscreen
has been flaky for a while. Debugging this, I've realized that
this is essentially due to  crbug.com/891489  -- a Ctrl-T creates
tab twice, which exceeds the expected tab count and waiting
doesn't stop.

BUG= 891489 
TEST=none

Change-Id: I3e655cfab9e1797149a76ab06d1f5bfc8d8b2118
Reviewed-on: https://chromium-review.googlesource.com/c/1278265
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Jun Mukai <mukai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599004}
[modify] https://crrev.com/b1729c64a26c645ed1c69b24f9512c7be77fae6a/testing/buildbot/filters/chromeos.single_process_mash.interactive_ui_tests.filter

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 16

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

commit d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1
Author: Scott Violet <sky@chromium.org>
Date: Tue Oct 16 22:00:51 2018

content: makes WebContentsDelegate::HandleKeyboardEvent() return status

This will be used to ensure events are properly updated based on whether
the delegate handles the event or not.

BUG= 891489 
TEST=none

Change-Id: I94fd4f8b781dbf725b7c2c6ac0d540986c24c24f
Reviewed-on: https://chromium-review.googlesource.com/c/1262404
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600143}
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/chrome/browser/chromeos/accessibility/spoken_feedback_event_rewriter_delegate.cc
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/chrome/browser/chromeos/accessibility/spoken_feedback_event_rewriter_delegate.h
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/chrome/browser/chromeos/login/ui/oobe_ui_dialog_delegate.cc
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/chrome/browser/chromeos/login/ui/oobe_ui_dialog_delegate.h
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/chrome/browser/chromeos/login/ui/webui_login_view.cc
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/chrome/browser/chromeos/login/ui/webui_login_view.h
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/chrome/browser/devtools/devtools_window.cc
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/chrome/browser/devtools/devtools_window.h
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/chrome/browser/extensions/extension_view.h
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/chrome/browser/extensions/extension_view_host.cc
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/chrome/browser/extensions/extension_view_host.h
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/chrome/browser/ui/browser.cc
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/chrome/browser/ui/browser.h
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/chrome/browser/ui/browser_window.h
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/chrome/browser/ui/signin_view_controller_delegate.cc
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/chrome/browser/ui/signin_view_controller_delegate.h
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/chrome/browser/ui/views/constrained_web_dialog_delegate_views.cc
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/chrome/browser/ui/views/extensions/extension_view_views.cc
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/chrome/browser/ui/views/extensions/extension_view_views.h
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/chrome/browser/ui/views/frame/browser_view.cc
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/chrome/browser/ui/views/frame/browser_view.h
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/chrome/browser/ui/views/profiles/signin_view_controller_delegate_views.cc
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/chrome/browser/ui/views/profiles/signin_view_controller_delegate_views.h
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/chrome/browser/ui/webui/constrained_web_dialog_delegate_base.cc
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/chrome/browser/ui/webui/constrained_web_dialog_delegate_base.h
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/chrome/test/base/test_browser_window.cc
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/chrome/test/base/test_browser_window.h
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/components/embedder_support/android/delegate/web_contents_delegate_android.cc
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/components/embedder_support/android/delegate/web_contents_delegate_android.h
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/components/guest_view/browser/guest_view_base.cc
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/components/guest_view/browser/guest_view_base.h
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/content/browser/frame_host/interstitial_page_impl.cc
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/content/browser/frame_host/interstitial_page_impl.h
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/content/browser/renderer_host/render_widget_host_delegate.cc
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/content/browser/renderer_host/render_widget_host_delegate.h
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/content/browser/renderer_host/render_widget_host_unittest.cc
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/content/browser/web_contents/web_contents_impl.h
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/content/public/browser/web_contents_delegate.cc
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/content/public/browser/web_contents_delegate.h
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/content/shell/browser/shell.h
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/content/shell/browser/shell_mac.mm
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/extensions/browser/app_window/app_window.cc
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/extensions/browser/app_window/app_window.h
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/extensions/browser/app_window/native_app_window.h
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/extensions/browser/guest_view/web_view/web_view_guest.cc
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/extensions/browser/guest_view/web_view/web_view_guest.h
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/extensions/components/native_app_window/native_app_window_views.cc
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/extensions/components/native_app_window/native_app_window_views.h
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/extensions/shell/browser/shell_native_app_window.cc
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/extensions/shell/browser/shell_native_app_window.h
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/ui/views/controls/webview/unhandled_keyboard_event_handler.cc
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/ui/views/controls/webview/unhandled_keyboard_event_handler.h
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/ui/views/controls/webview/unhandled_keyboard_event_handler_default.cc
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/ui/views/controls/webview/unhandled_keyboard_event_handler_mac.mm
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/ui/views/controls/webview/unhandled_keyboard_event_handler_win.cc
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/ui/views/controls/webview/web_dialog_view.cc
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/ui/views/controls/webview/web_dialog_view.h
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/ui/views/examples/webview_example.cc
[modify] https://crrev.com/d5caa87ad320cc8ceb307f9cc525b7d4f9604dd1/ui/views/examples/webview_example.h

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 17

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

commit 1616d16a7c635ec8dc5c1946122cd6edc124a0ed
Author: Scott Violet <sky@chromium.org>
Date: Wed Oct 17 15:16:54 2018

content: converts InputRouter to use callbacks rather than interface

This makes it easier to attach state to an individual event being processed.

BUG= 891489 
TEST=covered by tests

Change-Id: I50f70567d195c60187c9c8e8a2a68255c4c47f08
Reviewed-on: https://chromium-review.googlesource.com/c/1282031
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600394}
[modify] https://crrev.com/1616d16a7c635ec8dc5c1946122cd6edc124a0ed/content/browser/renderer_host/input/input_disposition_handler.h
[modify] https://crrev.com/1616d16a7c635ec8dc5c1946122cd6edc124a0ed/content/browser/renderer_host/input/input_router.h
[modify] https://crrev.com/1616d16a7c635ec8dc5c1946122cd6edc124a0ed/content/browser/renderer_host/input/input_router_impl.cc
[modify] https://crrev.com/1616d16a7c635ec8dc5c1946122cd6edc124a0ed/content/browser/renderer_host/input/input_router_impl.h
[modify] https://crrev.com/1616d16a7c635ec8dc5c1946122cd6edc124a0ed/content/browser/renderer_host/input/input_router_impl_unittest.cc
[modify] https://crrev.com/1616d16a7c635ec8dc5c1946122cd6edc124a0ed/content/browser/renderer_host/input/mock_input_disposition_handler.cc
[modify] https://crrev.com/1616d16a7c635ec8dc5c1946122cd6edc124a0ed/content/browser/renderer_host/input/mock_input_disposition_handler.h
[modify] https://crrev.com/1616d16a7c635ec8dc5c1946122cd6edc124a0ed/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/1616d16a7c635ec8dc5c1946122cd6edc124a0ed/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/1616d16a7c635ec8dc5c1946122cd6edc124a0ed/content/browser/renderer_host/render_widget_host_unittest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Oct 18

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

commit 297fdf47955d0f16708fa85d4f548a7cf5c0776c
Author: Scott Violet <sky@chromium.org>
Date: Thu Oct 18 21:15:37 2018

adds ability to track when keyevent processed asynchronously

This adds the ability to register a callback on a KeyEvent that is called
when async processing completes. This is important for content as key events
may be handed to the renderer, which means the event is asynchronously
processed.

BUG= 891489 
TEST=none

Change-Id: Ifaad1b88b0256ea2c69128663c24b1803e7f7321
Reviewed-on: https://chromium-review.googlesource.com/c/1262405
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600910}
[modify] https://crrev.com/297fdf47955d0f16708fa85d4f548a7cf5c0776c/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/297fdf47955d0f16708fa85d4f548a7cf5c0776c/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/297fdf47955d0f16708fa85d4f548a7cf5c0776c/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/297fdf47955d0f16708fa85d4f548a7cf5c0776c/content/browser/renderer_host/render_widget_host_view_aura.h
[modify] https://crrev.com/297fdf47955d0f16708fa85d4f548a7cf5c0776c/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/297fdf47955d0f16708fa85d4f548a7cf5c0776c/content/browser/renderer_host/render_widget_host_view_event_handler.cc
[modify] https://crrev.com/297fdf47955d0f16708fa85d4f548a7cf5c0776c/content/browser/renderer_host/render_widget_host_view_event_handler.h
[modify] https://crrev.com/297fdf47955d0f16708fa85d4f548a7cf5c0776c/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/297fdf47955d0f16708fa85d4f548a7cf5c0776c/ui/aura/mus/text_input_client_impl.cc
[modify] https://crrev.com/297fdf47955d0f16708fa85d4f548a7cf5c0776c/ui/events/event.cc
[modify] https://crrev.com/297fdf47955d0f16708fa85d4f548a7cf5c0776c/ui/events/event.h

Project Member

Comment 7 by bugdroid1@chromium.org, Oct 18

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

commit feea1cacd77254f922dd9683b5dbd41f3421bf8a
Author: Scott Violet <sky@chromium.org>
Date: Thu Oct 18 22:40:25 2018

Revert "adds ability to track when keyevent processed asynchronously"

This reverts commit 297fdf47955d0f16708fa85d4f548a7cf5c0776c.

Reason for revert: Causes failures single process mash interactive ui tests.

Original change's description:
> adds ability to track when keyevent processed asynchronously
> 
> This adds the ability to register a callback on a KeyEvent that is called
> when async processing completes. This is important for content as key events
> may be handed to the renderer, which means the event is asynchronously
> processed.
> 
> BUG= 891489 
> TEST=none
> 
> Change-Id: Ifaad1b88b0256ea2c69128663c24b1803e7f7321
> Reviewed-on: https://chromium-review.googlesource.com/c/1262405
> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
> Commit-Queue: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#600910}

TBR=sadrul@chromium.org,sky@chromium.org

Change-Id: I0dd537e67aff1ffbc59a89afc0707db0db4ccc3c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  891489 
Reviewed-on: https://chromium-review.googlesource.com/c/1289993
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600948}
[modify] https://crrev.com/feea1cacd77254f922dd9683b5dbd41f3421bf8a/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/feea1cacd77254f922dd9683b5dbd41f3421bf8a/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/feea1cacd77254f922dd9683b5dbd41f3421bf8a/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/feea1cacd77254f922dd9683b5dbd41f3421bf8a/content/browser/renderer_host/render_widget_host_view_aura.h
[modify] https://crrev.com/feea1cacd77254f922dd9683b5dbd41f3421bf8a/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/feea1cacd77254f922dd9683b5dbd41f3421bf8a/content/browser/renderer_host/render_widget_host_view_event_handler.cc
[modify] https://crrev.com/feea1cacd77254f922dd9683b5dbd41f3421bf8a/content/browser/renderer_host/render_widget_host_view_event_handler.h
[modify] https://crrev.com/feea1cacd77254f922dd9683b5dbd41f3421bf8a/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/feea1cacd77254f922dd9683b5dbd41f3421bf8a/ui/aura/mus/text_input_client_impl.cc
[modify] https://crrev.com/feea1cacd77254f922dd9683b5dbd41f3421bf8a/ui/events/event.cc
[modify] https://crrev.com/feea1cacd77254f922dd9683b5dbd41f3421bf8a/ui/events/event.h

Project Member

Comment 8 by bugdroid1@chromium.org, Oct 19

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

commit 9808da3ecdd1ca94f29e2d5e8c93f40092afb674
Author: Scott Violet <sky@chromium.org>
Date: Fri Oct 19 16:20:47 2018

reland: adds ability to track when keyevent processed asynchronously

This adds the ability to register a callback on a KeyEvent that is called
when async processing completes. This is important for content as key events
may be handed to the renderer, which means the event is asynchronously
processed.

BUG= 891489 
TEST=none
TBR=sadrul@chromium.org

Change-Id: I1dc0ae3768c61eac7bd5edadf0be8562dae8b1d3
Reviewed-on: https://chromium-review.googlesource.com/c/1290069
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601175}
[modify] https://crrev.com/9808da3ecdd1ca94f29e2d5e8c93f40092afb674/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/9808da3ecdd1ca94f29e2d5e8c93f40092afb674/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/9808da3ecdd1ca94f29e2d5e8c93f40092afb674/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/9808da3ecdd1ca94f29e2d5e8c93f40092afb674/content/browser/renderer_host/render_widget_host_view_aura.h
[modify] https://crrev.com/9808da3ecdd1ca94f29e2d5e8c93f40092afb674/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/9808da3ecdd1ca94f29e2d5e8c93f40092afb674/content/browser/renderer_host/render_widget_host_view_event_handler.cc
[modify] https://crrev.com/9808da3ecdd1ca94f29e2d5e8c93f40092afb674/content/browser/renderer_host/render_widget_host_view_event_handler.h
[modify] https://crrev.com/9808da3ecdd1ca94f29e2d5e8c93f40092afb674/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/9808da3ecdd1ca94f29e2d5e8c93f40092afb674/ui/events/event.cc
[modify] https://crrev.com/9808da3ecdd1ca94f29e2d5e8c93f40092afb674/ui/events/event.h

Project Member

Comment 9 by bugdroid1@chromium.org, Oct 19

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

commit 6c77e2f67d62360e78c1e47d38cd786014425069
Author: Scott Violet <sky@chromium.org>
Date: Fri Oct 19 16:38:42 2018

chromeos: makes sure events aren't sent to IME twice

Currently clients of window-service forward KeyEvents to IME. Ash was doing
the same, resulting in KeyEvents destined for remote clients going to IME
twice.

BUG= 891489 
TEST=none

Change-Id: I315c84399cfb9d8adf39ae9f3eb6c50d222f22c4
Reviewed-on: https://chromium-review.googlesource.com/c/1290194
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601182}
[modify] https://crrev.com/6c77e2f67d62360e78c1e47d38cd786014425069/ash/host/ash_window_tree_host_platform.cc
[modify] https://crrev.com/6c77e2f67d62360e78c1e47d38cd786014425069/ash/host/ash_window_tree_host_platform.h
[modify] https://crrev.com/6c77e2f67d62360e78c1e47d38cd786014425069/ui/aura/window_event_dispatcher.cc
[modify] https://crrev.com/6c77e2f67d62360e78c1e47d38cd786014425069/ui/aura/window_targeter.cc
[modify] https://crrev.com/6c77e2f67d62360e78c1e47d38cd786014425069/ui/aura/window_targeter.h
[modify] https://crrev.com/6c77e2f67d62360e78c1e47d38cd786014425069/ui/aura/window_tree_host.cc
[modify] https://crrev.com/6c77e2f67d62360e78c1e47d38cd786014425069/ui/aura/window_tree_host.h

Blocking: 897058
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 23

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

commit 5e5003737d4c9261435072316d4892e8df2ed8da
Author: Scott Violet <sky@chromium.org>
Date: Tue Oct 23 20:00:13 2018

chromeos: changes window-service to dispatch keyevents one at a time

Prior to this patch the window-service would send keyevents to clients without
waiting for an ack. This is problematic for a number of reasons,
primarily that it can lead to out of order keyevents. Out of order happens
because some events are acked earlier than others from IME. This patch also
wires up making the window service wait until the renderer respond to events
as well, which is crucial to ensure ash's post-processing accelerators work.

BUG= 891489 
TEST=covered by tests

Change-Id: Ifbc852633e996cf9175c2796c6e61ebda027d00a
Reviewed-on: https://chromium-review.googlesource.com/c/1292001
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602066}
[modify] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/ash/host/DEPS
[modify] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/ash/host/ash_window_tree_host_platform.cc
[modify] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/ash/host/ash_window_tree_host_platform.h
[modify] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/ash/shell.cc
[modify] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/services/ws/BUILD.gn
[modify] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/services/ws/event_injector.cc
[modify] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/services/ws/event_injector.h
[add] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/services/ws/event_queue.cc
[add] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/services/ws/event_queue.h
[add] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/services/ws/event_queue_test_helper.cc
[add] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/services/ws/event_queue_test_helper.h
[add] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/services/ws/event_queue_unittest.cc
[add] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/services/ws/host_event_dispatcher.h
[add] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/services/ws/host_event_queue.cc
[add] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/services/ws/host_event_queue.h
[modify] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/services/ws/injected_event_handler.cc
[modify] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/services/ws/injected_event_handler.h
[modify] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/services/ws/injected_event_handler_unittest.cc
[add] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/services/ws/test_host_event_dispatcher.cc
[add] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/services/ws/test_host_event_dispatcher.h
[modify] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/services/ws/test_ws/BUILD.gn
[modify] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/services/ws/test_ws/test_window_service.cc
[modify] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/services/ws/test_ws/test_window_service.h
[modify] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/services/ws/window_service.cc
[modify] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/services/ws/window_service.h
[modify] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/services/ws/window_service_observer.h
[modify] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/services/ws/window_service_observer_unittest.cc
[modify] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/services/ws/window_service_test_setup.cc
[modify] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/services/ws/window_service_test_setup.h
[modify] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/services/ws/window_tree.cc
[modify] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/services/ws/window_tree_unittest.cc
[modify] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/testing/buildbot/filters/chromeos.single_process_mash.interactive_ui_tests.filter
[modify] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/ui/aura/mus/text_input_client_impl.cc
[modify] https://crrev.com/5e5003737d4c9261435072316d4892e8df2ed8da/ui/aura/window_event_dispatcher.h

Status: Fixed (was: Started)
This should hopefully be fixed. Will enable tests next.
Project Member

Comment 13 by bugdroid1@chromium.org, Oct 23

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

commit bb75953cbb8f1d313968ab8038f695efca0e3b46
Author: Scott Violet <sky@chromium.org>
Date: Tue Oct 23 21:29:40 2018

chromeos: update test expectations for single-process-mash

All of these tests now pass.

BUG= 889101 , 891489 
TEST=test only change

Change-Id: I3f71af680c57c6e4c584156d098fad07f351b27b
Reviewed-on: https://chromium-review.googlesource.com/c/1297059
Reviewed-by: Jun Mukai <mukai@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602106}
[modify] https://crrev.com/bb75953cbb8f1d313968ab8038f695efca0e3b46/testing/buildbot/filters/chromeos.single_process_mash.interactive_ui_tests.filter

Project Member

Comment 14 by bugdroid1@chromium.org, Oct 23

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

commit ca4b9572075f299fc3d67cc8a3f003a82ee25b96
Author: Scott Violet <sky@chromium.org>
Date: Tue Oct 23 21:58:05 2018

content: removes debugging code

Forgot to remove before landing.

BUG= 891489 
TEST=none
TBR=xiyuan@chromium.org

Change-Id: Icc5df4d872a9fe5a08a275bf3a15bc0b7f9b8781
Reviewed-on: https://chromium-review.googlesource.com/c/1297056
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602120}
[modify] https://crrev.com/ca4b9572075f299fc3d67cc8a3f003a82ee25b96/content/browser/renderer_host/render_widget_host_view_aura.cc

Sign in to add a comment