Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(352)

Issue 2133013002: AcceleratorProvider: Make GetAcceleratorForCommandId const. (Closed)

Created:
4 years, 5 months ago by Matt Giuca
Modified:
4 years, 5 months ago
CC:
chromium-reviews, asanka, msramek+watch_chromium.org, sadrul, yusukes+watch_chromium.org, noyau+watch_chromium.org, markusheintz_, extensions-reviews_chromium.org, nona+watch_chromium.org, raymes+watch_chromium.org, Peter Beverloo, vabr+watchlistautofill_chromium.org, chromium-apps-reviews_chromium.org, rouslan+autofill_chromium.org, mlamouri+watch-notifications_chromium.org, kalyank, Lei Zhang, tfarina, shuchen+watch_chromium.org, jdonnelly+autofillwatch_chromium.org, tommycli, estade+watch_chromium.org, James Su, chrome-apps-syd-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

AcceleratorProvider: Make GetAcceleratorForCommandId const. Same for SimpleMenuModel and ButtonMenuItemModel. There is no reason for it to be non-const. Updated all subclasses. SimpleMenuModel::GetIndexOfCommandID was also made const. TBR=derat@chromium.org,avi@chromium.org,hajimehoshi@chromium.org,mkwst@chromium.org BUG=627344 Committed: https://crrev.com/272470e81fb7c7b47298e32009e2bae2001eb480 Cr-Commit-Position: refs/heads/master@{#405004}

Patch Set 1 #

Patch Set 2 : Rebase. #

Patch Set 3 : Fix tests and remove TODOs. #

Patch Set 4 : Fix more overrides as const. #

Patch Set 5 : Fix more overrides. #

Patch Set 6 : Fix overrides on Mac and Chrome OS. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+137 lines, -119 lines) Patch
M ash/common/shelf/shelf_alignment_menu.h View 1 chunk +1 line, -1 line 0 comments Download
M ash/common/shelf/shelf_alignment_menu.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M ash/common/system/web_notification/web_notification_tray.h View 1 1 chunk +1 line, -1 line 0 comments Download
M ash/common/system/web_notification/web_notification_tray.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M ash/shelf/shelf_view_unittest.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M ash/shell/context_menu.h View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M ash/shell/context_menu.cc View 1 2 3 1 chunk +3 lines, -2 lines 0 comments Download
M ash/shell_unittest.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M ash/sysui/context_menu_mus.h View 1 2 1 chunk +1 line, -1 line 0 comments Download
M ash/sysui/context_menu_mus.cc View 1 2 1 chunk +3 lines, -2 lines 0 comments Download
M ash/sysui/shelf_delegate_mus.cc View 1 2 1 chunk +3 lines, -2 lines 0 comments Download
M chrome/browser/apps/app_browsertest.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/download/download_shelf_context_menu.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/download/download_shelf_context_menu.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/extension_context_menu_model.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/extension_context_menu_model.cc View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/media_galleries/media_gallery_context_menu.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/media_galleries/media_gallery_context_menu.cc View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/profiles/profile_window_browsertest.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/renderer_context_menu/mock_render_view_context_menu.h View 1 2 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/renderer_context_menu/mock_render_view_context_menu.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/renderer_context_menu/open_with_menu_factory_ash.h View 1 2 3 4 5 1 chunk +3 lines, -2 lines 0 comments Download
M chrome/browser/renderer_context_menu/open_with_menu_factory_ash.cc View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/renderer_context_menu/render_view_context_menu_test_util.h View 1 2 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/renderer_context_menu/render_view_context_menu_test_util.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/status_icons/status_icon_menu_model.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/status_icons/status_icon_menu_model.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/app_list/app_context_menu.h View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/app_list/app_context_menu.cc View 1 2 3 4 5 1 chunk +3 lines, -2 lines 0 comments Download
M chrome/browser/ui/ash/launcher/launcher_application_menu_item_model.h View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/ash/launcher/launcher_application_menu_item_model.cc View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/ash/launcher/launcher_context_menu.h View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/ash/launcher/launcher_context_menu.cc View 1 2 3 4 5 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/ash/multi_user/multi_user_context_menu_chromeos.cc View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/autofill/autofill_dialog_models.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/autofill/autofill_dialog_models.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/bookmarks/bookmark_context_menu_controller.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/bookmarks/bookmark_context_menu_controller.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/cocoa/app_menu/app_menu_controller.mm View 1 2 3 4 5 1 chunk +3 lines, -2 lines 0 comments Download
M chrome/browser/ui/cocoa/browser/exclusive_access_controller_views.h View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/cocoa/browser/exclusive_access_controller_views.mm View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/cocoa/renderer_context_menu/render_view_context_menu_mac.h View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/cocoa/renderer_context_menu/render_view_context_menu_mac.mm View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/cocoa/tabs/tab_controller.mm View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/content_settings/content_setting_media_menu_model.h View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/content_settings/content_setting_media_menu_model.cc View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/panels/panel_extension_browsertest.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/toolbar/app_menu_model.h View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/toolbar/app_menu_model.cc View 1 2 chunks +4 lines, -3 lines 0 comments Download
M chrome/browser/ui/toolbar/app_menu_model_unittest.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/toolbar/media_router_contextual_menu.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/toolbar/media_router_contextual_menu.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/toolbar/recent_tabs_sub_menu_model.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/toolbar/recent_tabs_sub_menu_model.cc View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/views/bookmarks/bookmark_editor_view.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/frame/browser_view.h View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/frame/browser_view.cc View 1 1 chunk +3 lines, -2 lines 0 comments Download
M chrome/browser/ui/views/frame/system_menu_model_delegate.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/frame/system_menu_model_delegate.cc View 1 1 chunk +3 lines, -2 lines 0 comments Download
M chrome/browser/ui/views/new_task_manager_view.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/new_task_manager_view.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.h View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/toolbar/reload_button.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/toolbar/reload_button.cc View 1 chunk +3 lines, -2 lines 0 comments Download
M chrome/browser/ui/views/toolbar/toolbar_view.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/toolbar/toolbar_view.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/translate/translate_bubble_view.h View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/translate/translate_bubble_view.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/website_settings/permission_menu_model.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/website_settings/permission_menu_model.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/test/base/menu_model_test.h View 1 2 1 chunk +3 lines, -2 lines 0 comments Download
M chrome/test/base/menu_model_test.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M components/renderer_context_menu/render_view_context_menu_base.h View 1 2 1 chunk +2 lines, -2 lines 0 comments Download
M components/translate/core/browser/options_menu_model.h View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M components/translate/core/browser/options_menu_model.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M content/shell/browser/shell_views.cc View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M ui/app_list/app_list_menu.h View 1 chunk +1 line, -1 line 0 comments Download
M ui/app_list/app_list_menu.cc View 1 chunk +3 lines, -2 lines 0 comments Download
M ui/base/accelerators/accelerator.h View 1 chunk +3 lines, -2 lines 0 comments Download
M ui/base/cocoa/menu_controller_unittest.mm View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M ui/base/models/button_menu_item_model.h View 1 2 1 chunk +3 lines, -2 lines 0 comments Download
M ui/base/models/simple_menu_model.h View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
M ui/base/models/simple_menu_model.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M ui/message_center/message_center_tray.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M ui/message_center/views/notifier_settings_view.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M ui/views/controls/menu/menu_runner_cocoa_unittest.mm View 1 2 3 4 1 chunk +3 lines, -2 lines 0 comments Download
M ui/views/controls/textfield/textfield.h View 1 1 chunk +1 line, -1 line 0 comments Download
M ui/views/controls/textfield/textfield.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M ui/views/examples/menu_example.cc View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M ui/views/examples/tree_view_example.h View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M ui/views/examples/tree_view_example.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M ui/views/touchui/touch_selection_controller_impl_unittest.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download

Dependent Patchsets:

Messages

Total messages: 20 (8 generated)
Matt Giuca
Nico: Please review the main change in ui/base/accelerators/accelerator.h, ui/base/models/simple_menu_model.h and ui/base/models/button_menu_item_model.h. The rest of the ...
4 years, 5 months ago (2016-07-12 09:12:13 UTC) #3
Nico
lgtm To thin that not too long ago a change like this could've silently introduced ...
4 years, 5 months ago (2016-07-12 17:58:38 UTC) #4
Nico
To think, too
4 years, 5 months ago (2016-07-12 17:58:46 UTC) #5
Matt Giuca
Adding TBRs for the rest of the mechanical changes (just adding const): derat@chromium.org: ash/* avi@chromium.org: ...
4 years, 5 months ago (2016-07-13 01:01:52 UTC) #9
Matt Giuca
On 2016/07/12 17:58:38, Nico wrote: > lgtm > > To thin that not too long ...
4 years, 5 months ago (2016-07-13 01:07:57 UTC) #10
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/2133013002/100001
4 years, 5 months ago (2016-07-13 01:11:32 UTC) #12
tapted
On 2016/07/13 01:07:57, Matt Giuca wrote: > On 2016/07/12 17:58:38, Nico wrote: > > lgtm ...
4 years, 5 months ago (2016-07-13 01:26:19 UTC) #13
Daniel Erat
lgtm for ash
4 years, 5 months ago (2016-07-13 01:45:06 UTC) #14
Avi (use Gerrit)
lgtm
4 years, 5 months ago (2016-07-13 02:15:25 UTC) #15
commit-bot: I haz the power
Committed patchset #6 (id:100001)
4 years, 5 months ago (2016-07-13 04:13:50 UTC) #17
commit-bot: I haz the power
CQ bit was unchecked.
4 years, 5 months ago (2016-07-13 04:14:23 UTC) #18
commit-bot: I haz the power
4 years, 5 months ago (2016-07-13 04:16:59 UTC) #20
Message was sent while issue was closed.
Patchset 6 (id:??) landed as
https://crrev.com/272470e81fb7c7b47298e32009e2bae2001eb480
Cr-Commit-Position: refs/heads/master@{#405004}

Powered by Google App Engine
This is Rietveld 408576698