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

Issue 2751323002: Converts ui::Accelerator::type to an enum (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 months, 1 week ago by sky
Modified:
2 months, 1 week ago
Reviewers:
msw
CC:
chromium-reviews, sadrul, yusukes+watch_chromium.org, tfarina, shuchen+watch_chromium.org, nona+watch_chromium.org, kalyank, James Su
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Converts ui::Accelerator::type to an enum Accelerator::type_ was an EventType, but this is overkill as the type can only be pressed or released. So, this converts the type to an enum with those two values. I'm doing this as I'm going to write a mojom for Accelerator and it's better to restrict the mojom to the actual values. BUG=701815 TEST=covered by tests R=msw@chromium.org Review-Url: https://codereview.chromium.org/2751323002 Cr-Commit-Position: refs/heads/master@{#457492} Committed: https://chromium.googlesource.com/chromium/src/+/d56e56c7ea299f4b1f565f57022e9341265d3d28

Patch Set 1 #

Patch Set 2 : remove dcheck #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+100 lines, -97 lines) Patch
M ash/accelerators/accelerator_controller_unittest.cc View 13 chunks +25 lines, -28 lines 0 comments Download
M ash/common/accelerators/accelerator_controller.cc View 6 chunks +13 lines, -9 lines 0 comments Download
M ash/mus/accelerators/accelerator_controller_registrar.cc View 1 1 chunk +1 line, -3 lines 0 comments Download
M chrome/browser/ui/ash/chrome_screenshot_grabber_unittest.cc View 1 chunk +0 lines, -1 line 0 comments Download
M ui/base/accelerators/accelerator.h View 4 chunks +13 lines, -11 lines 2 comments Download
M ui/base/accelerators/accelerator.cc View 4 chunks +41 lines, -39 lines 0 comments Download
M ui/base/accelerators/accelerator_manager_unittest.cc View 1 chunk +1 line, -3 lines 0 comments Download
M ui/content_accelerators/accelerator_util.cc View 1 chunk +1 line, -1 line 0 comments Download
M ui/views/controls/textfield/textfield.cc View 1 chunk +5 lines, -2 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 12 (7 generated)
sky
2 months, 1 week ago (2017-03-16 03:51:46 UTC) #1
msw
I like type re-use, but I guess this is okay. lgtm with a nit. https://codereview.chromium.org/2751323002/diff/20001/ui/base/accelerators/accelerator.h ...
2 months, 1 week ago (2017-03-16 16:57:22 UTC) #6
sky
https://codereview.chromium.org/2751323002/diff/20001/ui/base/accelerators/accelerator.h File ui/base/accelerators/accelerator.h (right): https://codereview.chromium.org/2751323002/diff/20001/ui/base/accelerators/accelerator.h#newcode38 ui/base/accelerators/accelerator.h:38: enum class KeyState { On 2017/03/16 16:57:22, msw wrote: ...
2 months, 1 week ago (2017-03-16 17:06:40 UTC) #7
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/2751323002/20001
2 months, 1 week ago (2017-03-16 17:07:34 UTC) #9
commit-bot: I haz the power
2 months, 1 week ago (2017-03-16 18:11:32 UTC) #12
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/d56e56c7ea299f4b1f565f57022e...
Sign in to reply to this message.

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