Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(14)

Issue 2724913002: Move chromeos::EventRewriter to //ui/chromeos/events (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 months, 3 weeks ago by Peng
Modified:
3 months, 1 week ago
Reviewers:
sadrul, Tim Song, oshima, sky, Wez
CC:
chromium-reviews, extensions-reviews_chromium.org, alemate+watch_chromium.org, sadrul, chromoting-reviews_chromium.org, oshima+watch_chromium.org, chromium-apps-reviews_chromium.org, kalyank, davemoore+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Move chromeos::EventRewriter to //ui/chromeos/events chromeos::EventRewriter is responsible for converting F1~F12 to media function keys. Those conversion is not working in mus+ash. This CL moves chromeos::EventRewriter to //ui/chromeos/events from //chrome/browser/chromeos/events. So it can be used by mus as well. BUG=693180 Review-Url: https://codereview.chromium.org/2724913002 Cr-Commit-Position: refs/heads/master@{#457592} Committed: https://chromium.googlesource.com/chromium/src/+/e5df28289ab75276db7941bb8c5c76705ac1ef8f

Patch Set 1 #

Patch Set 2 : WIP #

Patch Set 3 : WIP #

Patch Set 4 : WIP #

Patch Set 5 : Fix unittests #

Patch Set 6 : Fix build errors #

Patch Set 7 : WIP #

Patch Set 8 : Fix build error #

Patch Set 9 : Fix build error #

Total comments: 7

Patch Set 10 : Address review issue #

Patch Set 11 : WIP #

Patch Set 12 : Use ui::EventRewriter #

Patch Set 13 : Use ui::EventRewriter #

Patch Set 14 : Fix build issue #

Total comments: 26

Patch Set 15 : Address review issues #

Total comments: 3

Patch Set 16 : Address review issues #

Total comments: 4

Patch Set 17 : Address review issue #

Patch Set 18 : Fix a typo #

Unified diffs Side-by-side diffs Delta from patch set Stats (+612 lines, -1932 lines) Patch
M ash/sticky_keys/sticky_keys_overlay_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 2 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/chrome_browser_main_chromeos.h View 1 2 3 4 5 6 7 8 9 2 chunks +2 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/chrome_browser_main_chromeos.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 3 chunks +7 lines, -3 lines 0 comments Download
D chrome/browser/chromeos/events/event_rewriter.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +0 lines, -205 lines 0 comments Download
D chrome/browser/chromeos/events/event_rewriter.cc View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +0 lines, -1196 lines 0 comments Download
A chrome/browser/chromeos/events/event_rewriter_delegate_impl.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +42 lines, -0 lines 0 comments Download
A chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +101 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/events/event_rewriter_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 69 chunks +194 lines, -287 lines 0 comments Download
M chrome/browser/chromeos/events/keyboard_driven_event_rewriter.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 3 chunks +5 lines, -7 lines 0 comments Download
M chrome/browser/chromeos/preferences.cc View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/extensions/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/extensions/api/settings_private/prefs_util.cc View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/ui/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/ui/ash/system_tray_delegate_chromeos.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/ui/webui/chromeos/keyboard_overlay_ui.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/pref_names.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +0 lines, -7 lines 0 comments Download
M chrome/common/pref_names.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +0 lines, -20 lines 0 comments Download
M remoting/client/normalizing_input_filter_cros.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +1 line, -2 lines 0 comments Download
A ui/chromeos/events/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +32 lines, -0 lines 0 comments Download
A + ui/chromeos/events/event_rewriter_chromeos.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 9 chunks +52 lines, -38 lines 0 comments Download
A + ui/chromeos/events/event_rewriter_chromeos.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 45 chunks +112 lines, -164 lines 0 comments Download
A ui/chromeos/events/pref_names.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +23 lines, -0 lines 0 comments Download
A ui/chromeos/events/pref_names.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +31 lines, -0 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 89 (64 generated)
Peng
Hi Sadrul, PTAL. Thanks.
3 months, 3 weeks ago (2017-03-02 21:51:13 UTC) #29
sadrul
https://codereview.chromium.org/2724913002/diff/160001/chrome/browser/chromeos/chrome_browser_main_chromeos.h File chrome/browser/chromeos/chrome_browser_main_chromeos.h (right): https://codereview.chromium.org/2724913002/diff/160001/chrome/browser/chromeos/chrome_browser_main_chromeos.h#newcode92 chrome/browser/chromeos/chrome_browser_main_chromeos.h:92: std::unique_ptr<EventRewriterDelegateImpl> event_rewriter_delegate_; This should be destroyed *after* all the ...
3 months, 3 weeks ago (2017-03-03 15:11:42 UTC) #36
Peng
https://codereview.chromium.org/2724913002/diff/160001/chrome/browser/chromeos/chrome_browser_main_chromeos.h File chrome/browser/chromeos/chrome_browser_main_chromeos.h (right): https://codereview.chromium.org/2724913002/diff/160001/chrome/browser/chromeos/chrome_browser_main_chromeos.h#newcode92 chrome/browser/chromeos/chrome_browser_main_chromeos.h:92: std::unique_ptr<EventRewriterDelegateImpl> event_rewriter_delegate_; On 2017/03/03 15:11:41, sadrul wrote: > This ...
3 months, 3 weeks ago (2017-03-03 16:07:05 UTC) #37
Wez
nit: Note that there are some typos in the comments of the new code in ...
3 months, 3 weeks ago (2017-03-03 23:31:19 UTC) #38
Peng
On 2017/03/03 23:31:19, Wez wrote: > nit: Note that there are some typos in the ...
3 months, 2 weeks ago (2017-03-06 15:18:18 UTC) #39
Peng
On 2017/03/03 23:31:19, Wez wrote: > nit: Note that there are some typos in the ...
3 months, 2 weeks ago (2017-03-06 16:14:52 UTC) #40
sadrul
https://codereview.chromium.org/2724913002/diff/160001/ui/chromeos/events/event_rewriter.h File ui/chromeos/events/event_rewriter.h (right): https://codereview.chromium.org/2724913002/diff/160001/ui/chromeos/events/event_rewriter.h#newcode89 ui/chromeos/events/event_rewriter.h:89: int* flags) = 0; On 2017/03/03 16:07:05, Peng wrote: ...
3 months, 2 weeks ago (2017-03-06 16:54:27 UTC) #41
Peng
On 2017/03/06 16:54:27, sadrul wrote: > https://codereview.chromium.org/2724913002/diff/160001/ui/chromeos/events/event_rewriter.h > File ui/chromeos/events/event_rewriter.h (right): > > https://codereview.chromium.org/2724913002/diff/160001/ui/chromeos/events/event_rewriter.h#newcode89 > ...
3 months, 2 weeks ago (2017-03-09 19:07:26 UTC) #46
Peng
On 2017/03/09 19:07:26, Peng wrote: > On 2017/03/06 16:54:27, sadrul wrote: > > > https://codereview.chromium.org/2724913002/diff/160001/ui/chromeos/events/event_rewriter.h ...
3 months, 1 week ago (2017-03-13 17:16:17 UTC) #55
sadrul
Looks pretty close. Some cleanups https://codereview.chromium.org/2724913002/diff/260001/chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc File chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc (right): https://codereview.chromium.org/2724913002/diff/260001/chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc#newcode73 chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc:73: return preference->GetValue()->GetBool(); Just use ...
3 months, 1 week ago (2017-03-14 01:15:41 UTC) #56
Peng
https://codereview.chromium.org/2724913002/diff/260001/chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc File chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc (right): https://codereview.chromium.org/2724913002/diff/260001/chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc#newcode73 chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc:73: return preference->GetValue()->GetBool(); On 2017/03/14 01:15:41, sadrul wrote: > Just ...
3 months, 1 week ago (2017-03-14 16:06:53 UTC) #58
sadrul
lgtm with the suggested changes below https://codereview.chromium.org/2724913002/diff/260001/chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc File chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc (right): https://codereview.chromium.org/2724913002/diff/260001/chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc#newcode73 chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc:73: return preference->GetValue()->GetBool(); On ...
3 months, 1 week ago (2017-03-16 05:02:43 UTC) #62
sadrul
https://codereview.chromium.org/2724913002/diff/280001/chrome/browser/chromeos/events/event_rewriter_unittest.cc File chrome/browser/chromeos/events/event_rewriter_unittest.cc (right): https://codereview.chromium.org/2724913002/diff/280001/chrome/browser/chromeos/events/event_rewriter_unittest.cc#newcode2606 chrome/browser/chromeos/events/event_rewriter_unittest.cc:2606: In a follow-up CL, can you look into moving ...
3 months, 1 week ago (2017-03-16 05:04:00 UTC) #63
Peng
https://codereview.chromium.org/2724913002/diff/260001/chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc File chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc (right): https://codereview.chromium.org/2724913002/diff/260001/chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc#newcode73 chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc:73: return preference->GetValue()->GetBool(); On 2017/03/16 05:02:43, sadrul wrote: > On ...
3 months, 1 week ago (2017-03-16 15:44:58 UTC) #66
Peng
3 months, 1 week ago (2017-03-16 15:45:01 UTC) #67
Peng
tengs@chromium.org: Please review changes in //ash/sticky_keys/ oshima@chromium.org: Please review changes in //chrome/browser/chromeos and //ui/chromeos Hi ...
3 months, 1 week ago (2017-03-16 17:20:39 UTC) #71
oshima
*/chromeos/* lgtm https://codereview.chromium.org/2724913002/diff/300001/chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc File chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc (right): https://codereview.chromium.org/2724913002/diff/300001/chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc#newcode88 chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc:88: ui::EF_ALT_DOWN | ui::EF_COMMAND_DOWN); nit: define mask as ...
3 months, 1 week ago (2017-03-16 18:51:08 UTC) #72
Peng
sky@chromium.org: Please review changes in //chrome/browser/ui/* and //chrome/browser/extensions/* Hi Scott, PTAL. Thanks.
3 months, 1 week ago (2017-03-16 19:06:33 UTC) #74
sky
LGTM
3 months, 1 week ago (2017-03-16 20:30:31 UTC) #75
Peng
https://codereview.chromium.org/2724913002/diff/300001/chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc File chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc (right): https://codereview.chromium.org/2724913002/diff/300001/chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc#newcode88 chrome/browser/chromeos/events/event_rewriter_delegate_impl.cc:88: ui::EF_ALT_DOWN | ui::EF_COMMAND_DOWN); On 2017/03/16 18:51:08, oshima wrote: > ...
3 months, 1 week ago (2017-03-16 20:33:27 UTC) #76
Peng
Hi Wez, Could you please take a look remoting/client/normalizing_input_filter_cros.cc? Thanks. On 2017/03/16 20:33:27, Peng wrote: ...
3 months, 1 week ago (2017-03-16 20:36:57 UTC) #77
Tim Song
lgtm
3 months, 1 week ago (2017-03-16 22:20:26 UTC) #80
Wez
On 2017/03/16 20:36:57, Peng wrote: > Hi Wez, Could you please take a look > ...
3 months, 1 week ago (2017-03-16 22:20:39 UTC) #81
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2724913002/340001
3 months, 1 week ago (2017-03-16 22:48:03 UTC) #86
commit-bot: I haz the power
3 months, 1 week ago (2017-03-16 22:55:32 UTC) #89
Message was sent while issue was closed.
Committed patchset #18 (id:340001) as
https://chromium.googlesource.com/chromium/src/+/e5df28289ab75276db7941bb8c5c...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cb946e318