Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Clipboard Ozone implementation #506

Merged
merged 4 commits into from
Nov 30, 2018
Merged

Clipboard Ozone implementation #506

merged 4 commits into from
Nov 30, 2018

Conversation

msisov
Copy link
Member

@msisov msisov commented Nov 28, 2018 •
edited
Loading

This is a first set of patches, which implement clipboard
support for the Ozone platforms.

The flow is asynchronous and based on Requests. That is,
whenever a ClipboardOzone receives a Read/Write/GetMime
call, it forwards it to the AsyncClipboardOzone, which
then creates a request (used for internal usage and holding
data filled by the ClipboardDelegate), calls to the delegate
and start an abort timer to make sure the request is not stalled.

What is more the clipboard data is cached and removed only when another
chunk of data iss written to a system clipboard. And whenever the chromium browser is the
selection owner, the cached data is used.

@msisov msisov self-assigned this Nov 28, 2018
tonikitoo
tonikitoo reviewed Nov 28, 2018
View reviewed changes
Copy link
Member

@tonikitoo tonikitoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpicking a bit, maybe clipboard_aura.cc or clipboard_auralinux.cc (rather than _linux)?
but lets try to upstream this and hopefully hear from them.

Should this commit include any associated include adaptations?

tonikitoo
tonikitoo reviewed Nov 28, 2018
View reviewed changes
ui/base/BUILD.gn
]
if (is_desktop_linux) {
if (use_x11 || use_ozone) {
sources += [ "clipboard/clipboard_linux.cc" ]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will this file be needed for say ozone/win (something google was upstreaming at some point)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, this is not platform specific and can be used with mac or win in theory.

tonikitoo
tonikitoo approved these changes Nov 28, 2018
View reviewed changes
Copy link
Member

@tonikitoo tonikitoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good thing is that it does not change the ozone's clipboard impl, so I can apply my local patches for middle-click paste as a follow up.

r+

jkim-julie
jkim-julie reviewed Nov 29, 2018
View reviewed changes
ui/base/clipboard/clipboard.h
@@ -31,6 +31,10 @@
#include <objidl.h>
#endif

#if defined(USE_OZONE)
#include "ui/ozone/public/clipboard_delegate.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: It would be great if we could use the forward declaration here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it will need another set of if defs, which I don't want to add.

@jkim-julie
Copy link
Member

lgtm!

@msisov
Clipboard: move various predefined FormatTypes to common file.
1a948b9 
Many clipboard implementations use the predefined FormatTypes
from the clipboard_aurax11. Thus, to make clipboard ozone use
them as well, move those into a separate clipboard_linux
file and include it when necessary.

Change-Id: Iec6b6aacecc567a35534a170fb50afff7e978140
@msisov
[ozone] Implement Clipboard for Ozone platforms.
5da1a21 
This is a first patch, which implements basic text
support for the Ozone platforms.

The flow is asynchronous and based on Requests. That is,
whenever a ClipboardOzone receives a Read/Write/GetMime
call, it forwards it to the AsyncClipboardOzone, which
then creates a request (used for internal usage and holding
data filled by the ClipboardDelegate), calls to the delegate
and start an abort timer to make sure the request is not stalled.

What is more the clipboard data is cached and removed only when another
chunk of data iss written to a system clipboard. And whenever the chromium browser is the
selection owner, the cached data is used.

Change-Id: I0101aebe47cf2cac666f0434b6be2a9a11e2418c
@msisov
[ozone/wayland] Clipboard: integrate Ozone/Wayland and ClipboardOzone
d8af43b 
Change-Id: I3b1253f38681e8555c762eb4560df9fbe34e106f
@msisov
[ozone] Add clipboard testing support.
4ee6ebb 
Mock out clipboard delegate and provide a simple mocked
system clipboard implementation to test clipboard_ozone

Change-Id: I4d65e1496930cb14a32396be974e8f84353288d0
@msisov msisov merged commit 8bdadf8 into Igalia:ozone-wayland-dev Nov 30, 2018
@msisov msisov mentioned this pull request Nov 30, 2018
Closed
msisov pushed a commit that referenced this pull request Dec 23, 2018
[Merge to M71] Speculative fix for the unusable touchpad issue.
10d430c 
There might be a race condition between calling
InputControllerEvdev::set_has_touchpad() and
InputControllerEvdev::SetInternalTouchpadEnabled() functions. So it's
possible that we call SetInternalTouchpadEnabled(false) to disable a
touchpad, and then set_has_touchpad(false) is called because the touchpad
is removed, and then later when the touchpad is re-added to the device,
SetInternalTouchpadEnabled(true) is called before set_has_touchpad(true)
is called. In this case, SetInternalTouchpadEnabled(true) does nothing.
See b/118567558comment#11 for more details.

This CL ensures after a touchpad is added or readded to the device, it's
properly updated.

Bug: b/118317363, b/118567558, b/118633774
TBR=xiyuan@chromium.org

Change-Id: I1da3c9c30ad843a761e947ef4687b6ca30b98b49
Reviewed-on: https://chromium-review.googlesource.com/c/1312116
Commit-Queue: Xiaoqian Dai <xdai@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#604666}(cherry picked from commit 09618ae)
Reviewed-on: https://chromium-review.googlesource.com/c/1318180
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#506}
Cr-Branched-From: 4226ddf-refs/heads/master@{#599034}
msisov pushed a commit that referenced this pull request Jan 31, 2019
Updating XTBs based on .GRDs from branch 3626
324ea90 
R=abdulsyed@chromium.org

Change-Id: I5eee956b6b7deaf57d4950e348bd12409b1d96e3
Reviewed-on: https://chromium-review.googlesource.com/c/1388749
Reviewed-by: Krishna Govind <govind@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#506}
Cr-Branched-From: d897fb1-refs/heads/master@{#612437}
msisov pushed a commit that referenced this pull request Mar 14, 2019
[Autofill] Add dark mode Google Pay logo for Autofill suggestions
76dffb7 
TBR=jochen
(cherry picked from commit 5f6002a)

Bug: 933406
Change-Id: If18e074621398ac7795523311c024821c7fb6ff1
Reviewed-on: https://chromium-review.googlesource.com/c/1468784
Reviewed-by: Fabio Tirelo <ftirelo@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: Jared Saul <jsaul@google.com>
Cr-Original-Commit-Position: refs/heads/master@{#632769}
Reviewed-on: https://chromium-review.googlesource.com/c/1477999
Reviewed-by: Jared Saul <jsaul@google.com>
Cr-Commit-Position: refs/branch-heads/3683@{#506}
Cr-Branched-From: e510299-refs/heads/master@{#625896}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jkim-julie jkim-julie jkim-julie left review comments

@tonikitoo tonikitoo tonikitoo approved these changes

Assignees

@msisov msisov

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@msisov @jkim-julie @tonikitoo