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

Issue 2779373002: Remove Nested Message Loop from MenuController (Closed)

Created:
3 years, 8 months ago by jonross
Modified:
3 years, 8 months ago
Reviewers:
sky
CC:
chromium-reviews, tfarina, mac-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

All menus on aura platforms no longer use the nested message loop code path of MenuController. On Mac non-native menus also no longer use the nested message loop. While native menus are handled vis menu_runner_impl_cocoa, which does block. This change removes the Nested Message Loop code paths from MenuController. Additionally some tests which specifically were for nested message loops below asynchronous menus, have been either transitioned, or removed if redundant. TEST=views_unittests, manual testing. BUG=557130 Review-Url: https://codereview.chromium.org/2779373002 Cr-Commit-Position: refs/heads/master@{#460818} Committed: https://chromium.googlesource.com/chromium/src/+/b31c887a2d14761b078f329629f1d3469106b891

Patch Set 1 : unneeded runloop #

Unified diffs Side-by-side diffs Delta from patch set Stats (+126 lines, -713 lines) Patch
M ui/views/BUILD.gn View 2 chunks +0 lines, -5 lines 0 comments Download
M ui/views/controls/menu/menu_controller.h View 7 chunks +6 lines, -28 lines 0 comments Download
M ui/views/controls/menu/menu_controller.cc View 17 chunks +64 lines, -110 lines 0 comments Download
M ui/views/controls/menu/menu_controller_unittest.cc View 33 chunks +31 lines, -275 lines 0 comments Download
D ui/views/controls/menu/menu_message_loop.h View 1 chunk +0 lines, -46 lines 0 comments Download
D ui/views/controls/menu/menu_message_loop_aura.h View 1 chunk +0 lines, -34 lines 0 comments Download
D ui/views/controls/menu/menu_message_loop_aura.cc View 1 chunk +0 lines, -87 lines 0 comments Download
D ui/views/controls/menu/menu_message_loop_mac.h View 1 chunk +0 lines, -35 lines 0 comments Download
D ui/views/controls/menu/menu_message_loop_mac.cc View 1 chunk +0 lines, -44 lines 0 comments Download
M ui/views/controls/menu/menu_runner_impl.h View 1 chunk +0 lines, -3 lines 0 comments Download
M ui/views/controls/menu/menu_runner_impl.cc View 6 chunks +25 lines, -46 lines 0 comments Download

Messages

Total messages: 18 (12 generated)
jonross
Hey sky@, In this change I removed the nested message loop code path within MenuController. ...
3 years, 8 months ago (2017-03-29 23:02:15 UTC) #6
sky
Yay! LGTM
3 years, 8 months ago (2017-03-29 23:52:26 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/2779373002/20001
3 years, 8 months ago (2017-03-30 13:36:12 UTC) #11
commit-bot: I haz the power
Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/builds/183398)
3 years, 8 months ago (2017-03-30 16:00:52 UTC) #13
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/2779373002/20001
3 years, 8 months ago (2017-03-30 16:05:41 UTC) #15
commit-bot: I haz the power
3 years, 8 months ago (2017-03-30 17:53:37 UTC) #18
Message was sent while issue was closed.
Committed patchset #1 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/b31c887a2d14761b078f329629f1...

Powered by Google App Engine
This is Rietveld 408576698