Mac[Views]: Make native menus more responsive by pumping private runloop modes.

base/message_loop/message_pump_mac.mm

Index: base/message_loop/message_pump_mac.mm
diff --git a/base/message_loop/message_pump_mac.mm b/base/message_loop/message_pump_mac.mm
index 82d9f1e818b18c07af4b3ec2ac5e785de43af61d..144449fe2cd88d6b6eeb40878bb8c32aed5a600a 100644
--- a/base/message_loop/message_pump_mac.mm
+++ b/base/message_loop/message_pump_mac.mm
@@ -25,38 +25,54 @@ namespace base {
 
 namespace {
 
+// AppKit RunLoop modes observed to potentially run tasks posted to Chrome's
+// main thread task runner. Some are internal to AppKit but must be observed to
+// keep Chrome's UI responsive. Others that may be interesting, but are not
+// watched:
+//  - com.apple.hitoolbox.windows.transitionmode
+//  - com.apple.hitoolbox.windows.flushmode
+const CFStringRef kAllModes[] = {
+    kCFRunLoopCommonModes, kMessageLoopExclusiveRunLoopMode,

[Mark Mentovai, 2017/05/10 14:00:18]

I guess I’m not supposed to complain if clang-format wrote this, but the second
item on this line is kind of hiding.

[Robert Sesek, 2017/05/10 14:02:49]

I complain all the time about this because clang-format is wrong, and I'm not
going to apologize for that :).

I've started using "// clang-format off" around lists of things to compensate.

[tapted, 2017/05/11 00:24:06]

I added a comment, 

// Mode that only sees Chrome work sources.
kMessageLoopExclusiveRunLoopMode,


(Incidentally... and probably orthogonal, do we still need
kMessageLoopExclusiveRunLoopMode?

`git grep` only shows it being passed to CFRunLoopAddObserver(), but then only
in combination with kCFRunLoopCommonModes -- it looks like we only ever spin a
run loop in the kMessageLoopExclusiveRunLoopMode in a couple of unit test
files).

[Robert Sesek, 2017/05/11 16:12:36]

I was wondering the same thing. The Exclusive mode was added a while ago, but I
don't think it was ultimately used as intended. I can't tell if the few cases
where it's currently used are actually required, or if CommonModes are
sufficient.

+
+    // Process work when NSMenus are fading out.
+    CFSTR("com.apple.hitoolbox.windows.windowfadingmode"),
+
+    // Process work when AppKit is highlighting an item on the main menubar.
+    CFSTR("NSUnhighlightMenuRunLoopMode"),
+};
+
 void CFRunLoopAddSourceToAllModes(CFRunLoopRef rl, CFRunLoopSourceRef source) {

[Robert Sesek, 2017/05/10 13:37:27]

One last thought: it would be nice if we could limit this to the
MessagePumpNSApplication, but I don't see a clean way to do that since these are
installed in the MessagePumpCFRunLoopBase and the members are private, not
protected. Maybe Mark has an idea.

[tapted, 2017/05/11 00:24:05]

We could pass a std::vector of scoped_cftyperef<CFStringRef> to the
MessagePumpCFRunLoopBase constructor? Or a vector<size_t> of kAllModes indices.

I don't know enough about the message pump stuff to know how crucial this is..
Leaving as is, but LMK if I should explore it now, or in a follow-up.

[Robert Sesek, 2017/05/11 16:12:36]

I don't think it's critical since the mode should only be used on the AppKit
main thread. Passing a vector of additional modes (in addition to CommonModes)
to the MessagePumpCFRunLoopBase constructor seems reasonable. If this CL sticks,
I think it's worth considering.

-  CFRunLoopAddSource(rl, source, kCFRunLoopCommonModes);
-  CFRunLoopAddSource(rl, source, kMessageLoopExclusiveRunLoopMode);
+  for (const CFStringRef& mode : kAllModes)
+    CFRunLoopAddSource(rl, source, mode);
 }
 
 void CFRunLoopRemoveSourceFromAllModes(CFRunLoopRef rl,
                                        CFRunLoopSourceRef source) {
-  CFRunLoopRemoveSource(rl, source, kCFRunLoopCommonModes);
-  CFRunLoopRemoveSource(rl, source, kMessageLoopExclusiveRunLoopMode);
+  for (const CFStringRef& mode : kAllModes)
+    CFRunLoopRemoveSource(rl, source, mode);
 }
 
 void CFRunLoopAddTimerToAllModes(CFRunLoopRef rl, CFRunLoopTimerRef timer) {
-  CFRunLoopAddTimer(rl, timer, kCFRunLoopCommonModes);
-  CFRunLoopAddTimer(rl, timer, kMessageLoopExclusiveRunLoopMode);
+  for (const CFStringRef& mode : kAllModes)
+    CFRunLoopAddTimer(rl, timer, mode);
 }
 
 void CFRunLoopRemoveTimerFromAllModes(CFRunLoopRef rl,
                                       CFRunLoopTimerRef timer) {
-  CFRunLoopRemoveTimer(rl, timer, kCFRunLoopCommonModes);
-  CFRunLoopRemoveTimer(rl, timer, kMessageLoopExclusiveRunLoopMode);
+  for (const CFStringRef& mode : kAllModes)
+    CFRunLoopRemoveTimer(rl, timer, mode);
 }
 
 void CFRunLoopAddObserverToAllModes(CFRunLoopRef rl,
                                     CFRunLoopObserverRef observer) {
-  CFRunLoopAddObserver(rl, observer, kCFRunLoopCommonModes);
-  CFRunLoopAddObserver(rl, observer, kMessageLoopExclusiveRunLoopMode);
+  for (const CFStringRef& mode : kAllModes)
+    CFRunLoopAddObserver(rl, observer, mode);
 }
 
 void CFRunLoopRemoveObserverFromAllModes(CFRunLoopRef rl,
                                          CFRunLoopObserverRef observer) {
-  CFRunLoopRemoveObserver(rl, observer, kCFRunLoopCommonModes);
-  CFRunLoopRemoveObserver(rl, observer, kMessageLoopExclusiveRunLoopMode);
+  for (const CFStringRef& mode : kAllModes)
+    CFRunLoopRemoveObserver(rl, observer, mode);
 }
 
 void NoOp(void* info) {