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

Issue 828864 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature

Blocking:
issue 803102
issue 836125


Participants' hotlists:
Audio-Service


Sign in to add a comment

Move UserInputMonitor to audio service

Project Member Reported by marinaciocea@chromium.org, Apr 4 2018

Issue description

Figure out thread requirements and move implementation to audio service.

http://drawings/1_ZIKj6lihGKRjq4Mflduitmkn_REqpHFeqVNelBGHHk
 
Labels: -Type-Bug Type-Feature
Description: Show this description
Status: Assigned (was: Untriaged)
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 12 2018

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

commit c37646df101b84397a1513ed4339bcf33bc04136
Author: Marina Ciocea <marinaciocea@chromium.org>
Date: Thu Apr 12 05:53:08 2018

Remove second StopMonitor in UserInputMonitorLinux.

Cleanup double StopMonitor and unnecessary condition, leftover from mouse listener removal in https://crrev.com/2577573002.

Bug:  828864 
Change-Id: Ia19c5bf4f81897f49b1c0eaf0f6d85a9cfff4984
Reviewed-on: https://chromium-review.googlesource.com/1007194
Reviewed-by: Max Morin <maxmorin@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Marina Ciocea <marinaciocea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550032}
[modify] https://crrev.com/c37646df101b84397a1513ed4339bcf33bc04136/media/base/user_input_monitor_linux.cc

Status: Started (was: Assigned)
Labels: -Pri-3 Pri-2
Blocking: 836125
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 24 2018

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

commit fa3e776038d21d6c2296d21e2bf35f67eb8943b3
Author: Marina Ciocea <marinaciocea@chromium.org>
Date: Tue Apr 24 23:32:29 2018

Use uint32_t instead of size_t in KeyboardEventCounter.

Needed for sending media::UserInputMonitor::GetKeyPressCount() over IPC.

Bug:  https://crbug.com/828864 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: Ifa391e3f6b9c75971616eb7ed6596f73bc9ddec8
Reviewed-on: https://chromium-review.googlesource.com/1025814
Commit-Queue: Marina Ciocea <marinaciocea@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553360}
[modify] https://crrev.com/fa3e776038d21d6c2296d21e2bf35f67eb8943b3/content/browser/renderer_host/media/audio_input_delegate_impl_unittest.cc
[modify] https://crrev.com/fa3e776038d21d6c2296d21e2bf35f67eb8943b3/media/audio/audio_input_controller_unittest.cc
[modify] https://crrev.com/fa3e776038d21d6c2296d21e2bf35f67eb8943b3/media/base/keyboard_event_counter.cc
[modify] https://crrev.com/fa3e776038d21d6c2296d21e2bf35f67eb8943b3/media/base/keyboard_event_counter.h
[modify] https://crrev.com/fa3e776038d21d6c2296d21e2bf35f67eb8943b3/media/base/user_input_monitor.h
[modify] https://crrev.com/fa3e776038d21d6c2296d21e2bf35f67eb8943b3/media/base/user_input_monitor_linux.cc
[modify] https://crrev.com/fa3e776038d21d6c2296d21e2bf35f67eb8943b3/media/base/user_input_monitor_mac.cc
[modify] https://crrev.com/fa3e776038d21d6c2296d21e2bf35f67eb8943b3/media/base/user_input_monitor_win.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Apr 26 2018

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

commit a62ef37c0f828a8f85db123feefbc49f373b8c41
Author: Marina Ciocea <marinaciocea@chromium.org>
Date: Thu Apr 26 09:52:28 2018

Add UserInputMonitor in audio service.

audio::UserInputMonitor overrides audio::UserInputMonitor::GetKeyPressCount() to read key press
count (computed in browser process) from shared memory. Read only handle to key press count shmem
is passed over IPC on audio service input stream creation.

Design doc: https://docs.google.com/document/d/1GHL4uMlIFox2eAqsjUQVA8eJ7HYU3g2AoyAqyx3pxMA/edit?usp=sharing

Notes:
- Browser side implementation of key press count share memory, and  wrapping of memory handle
  into a Mojo handle will be added in dependent CL https://crrev.com/c/1025754.
- UserInputMonitor is enabled in MojoAudioInputStreamObserver constructor, when a stream is created,
  and disabled on connection error, called when the observed stream is destroyed.
- audio:: and media:: UserInputMonitor implementations inherit from media::UserInputMonitorBase in
  order to keep media::AudioInputController unchanged for now. Inheritance will be removed when
  switching to audio service input streams.

Streams design diagram: http://drawings/1_ZIKj6lihGKRjq4Mflduitmkn_REqpHFeqVNelBGHHk

Bug:  828864 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I0219494d73df2fd275d2b7ccb5bf01f81a1ff11c
Reviewed-on: https://chromium-review.googlesource.com/1011605
Commit-Queue: Marina Ciocea <marinaciocea@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Olga Sharonova <olka@chromium.org>
Reviewed-by: Max Morin <maxmorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553967}
[modify] https://crrev.com/a62ef37c0f828a8f85db123feefbc49f373b8c41/content/browser/renderer_host/media/audio_input_delegate_impl_unittest.cc
[modify] https://crrev.com/a62ef37c0f828a8f85db123feefbc49f373b8c41/media/audio/audio_input_controller_unittest.cc
[modify] https://crrev.com/a62ef37c0f828a8f85db123feefbc49f373b8c41/media/base/user_input_monitor.cc
[modify] https://crrev.com/a62ef37c0f828a8f85db123feefbc49f373b8c41/media/base/user_input_monitor.h
[modify] https://crrev.com/a62ef37c0f828a8f85db123feefbc49f373b8c41/media/base/user_input_monitor_linux.cc
[modify] https://crrev.com/a62ef37c0f828a8f85db123feefbc49f373b8c41/media/base/user_input_monitor_mac.cc
[modify] https://crrev.com/a62ef37c0f828a8f85db123feefbc49f373b8c41/media/base/user_input_monitor_unittest.cc
[modify] https://crrev.com/a62ef37c0f828a8f85db123feefbc49f373b8c41/media/base/user_input_monitor_win.cc
[modify] https://crrev.com/a62ef37c0f828a8f85db123feefbc49f373b8c41/media/mojo/services/mojo_audio_input_stream_observer.cc
[modify] https://crrev.com/a62ef37c0f828a8f85db123feefbc49f373b8c41/media/mojo/services/mojo_audio_input_stream_observer.h
[modify] https://crrev.com/a62ef37c0f828a8f85db123feefbc49f373b8c41/media/mojo/services/mojo_audio_input_stream_observer_unittest.cc
[modify] https://crrev.com/a62ef37c0f828a8f85db123feefbc49f373b8c41/services/audio/BUILD.gn
[modify] https://crrev.com/a62ef37c0f828a8f85db123feefbc49f373b8c41/services/audio/input_stream.cc
[modify] https://crrev.com/a62ef37c0f828a8f85db123feefbc49f373b8c41/services/audio/input_stream.h
[modify] https://crrev.com/a62ef37c0f828a8f85db123feefbc49f373b8c41/services/audio/input_stream_unittest.cc
[modify] https://crrev.com/a62ef37c0f828a8f85db123feefbc49f373b8c41/services/audio/public/mojom/stream_factory.mojom
[modify] https://crrev.com/a62ef37c0f828a8f85db123feefbc49f373b8c41/services/audio/stream_factory.cc
[modify] https://crrev.com/a62ef37c0f828a8f85db123feefbc49f373b8c41/services/audio/stream_factory.h
[add] https://crrev.com/a62ef37c0f828a8f85db123feefbc49f373b8c41/services/audio/user_input_monitor.cc
[add] https://crrev.com/a62ef37c0f828a8f85db123feefbc49f373b8c41/services/audio/user_input_monitor.h
[add] https://crrev.com/a62ef37c0f828a8f85db123feefbc49f373b8c41/services/audio/user_input_monitor_unittest.cc

Status: Fixed (was: Started)
[bulk-edit: disregard if N/A] Can the owner please set milestone to this bug if applicable?
Project Member

Comment 12 by bugdroid1@chromium.org, Jun 25 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-go.git/+/17909a5ed08ccf3d8b55f37c33cfa7b376c273d8

commit 17909a5ed08ccf3d8b55f37c33cfa7b376c273d8
Author: David Benjamin <davidben@google.com>
Date: Mon Jun 25 22:49:00 2018

[milo] Fix interaction of linkifying rules with each other and escaping.

formatCommitDesc ran regexes on the HTML escaped input. This results in
a number of incorrect conversions. E.g., rewrote:

    https://crbug.com/618365

to:

    <a href="https://<a href="https://crbug.com/618365">crbug.com/618365</a>">https://<a href="https://crbug.com/618365">crbug.com/618365</a></a>

because it would first apply the URL-linking rule on the full URL, then
it would find several instances of crbug.com/618365, which it dutifully
linked as well. This particular case was even enshrined in a test
expectation.

For a truly entertaining case, see
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/GPU%20FYI%20Linux%20Builder/5847.
Here it rewrites:

    Bug:  https://crbug.com/828864 

as:

    Bug: <a href="https://<a href="https://crbug.com/<a href=" https://crbug.com/828864 ">828864</a>">crbug.com/<a href=" https://crbug.com/828864 ">828864</a></a>">https://<a href="https://crbug.com/<a href=" https://crbug.com/828864 ">828864</a>">crbug.com/<a href=" https://crbug.com/828864 ">828864</a></a></a>

There are seven hrefs in there. :-) First, it sees
 https://crbug.com/828864  and linkifies that (1). That results in two
 crbug.com/828864  instances, which matches the next rule (2). That
results in four 828864 instances, which matches the "Bug: ..." rule,
each of which gets linkified (4). 1 + 2 + 4 = 7.

Instead, use a slightly richer intermediate representation that prevents
already-processed chunks of text from being processed a second time.

Bug:  855719 
Change-Id: Ica1aec4ed10ca2547680fca8639a996cd0d529f2
Reviewed-on: https://chromium-review.googlesource.com/1112575
Reviewed-by: Nodir Turakulov <nodir@chromium.org>
Commit-Queue: David Benjamin <davidben@chromium.org>

[modify] https://crrev.com/17909a5ed08ccf3d8b55f37c33cfa7b376c273d8/milo/frontend/expectations/buildbot.build-Debug_page-_CrWinGoma_30608.html
[modify] https://crrev.com/17909a5ed08ccf3d8b55f37c33cfa7b376c273d8/milo/frontend/middleware.go
[modify] https://crrev.com/17909a5ed08ccf3d8b55f37c33cfa7b376c273d8/milo/frontend/middleware_test.go

Sign in to add a comment