Mac: modernize menu reveal tracking

The fullscreen toolbar controller currently uses undocumented Carbon
events to track the menubar's reveal amount in fullscreen. In recent
macOS versions, the intermediate values are no longer send and we
only receive events when the menubar is fully shown or fully hidden.

The Mac immersive code which is still behind a flag uses a different
technique, observing the reveal amount on a hidden titlebar accessory
view. This *does* provide correct intermediate values.

This change extracts the reveal code from the immersive controller
and uses it in the current fullscreen code rather than the Carbon
event. Unfortunately, despite receiving the correct progress
notifications, the topchrome still does a discontinuous jump, but
we'll cross that bridge later.

Bug: 1063417
Change-Id: I6827cafbcad58eeaabf98c6facaeb85ce3439997
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2314998Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Leonard Grey <lgrey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791273}

chrome/browser/ui/BUILD.gn

@@ -2798,6 +2798,8 @@ static_library("ui") {
       "cocoa/fullscreen/fullscreen_toolbar_controller.mm",
       "cocoa/fullscreen/fullscreen_toolbar_mouse_tracker.h",
       "cocoa/fullscreen/fullscreen_toolbar_mouse_tracker.mm",
+      "cocoa/fullscreen/menu_reveal_monitor.h",
+      "cocoa/fullscreen/menu_reveal_monitor.mm",
       "cocoa/handoff_active_url_observer.cc",
       "cocoa/handoff_active_url_observer.h",
       "cocoa/handoff_active_url_observer_bridge.h",

chrome/browser/ui/cocoa/fullscreen/fullscreen_menubar_tracker.mm

@@ -4,56 +4,19 @@
 
 #import "chrome/browser/ui/cocoa/fullscreen/fullscreen_menubar_tracker.h"
 
-#include <Carbon/Carbon.h>
 #include <QuartzCore/QuartzCore.h>
 
 #include "base/mac/mac_util.h"
+#include "base/mac/scoped_nsobject.h"
 #include "base/stl_util.h"
 #import "chrome/browser/ui/cocoa/fullscreen/fullscreen_toolbar_controller.h"
+#import "chrome/browser/ui/cocoa/fullscreen/menu_reveal_monitor.h"
 #include "ui/base/cocoa/appkit_utils.h"
 
-namespace {
-
-// The event kind value for a undocumented menubar show/hide Carbon event.
-const CGFloat kMenuBarRevealEventKind = 2004;
-
-// TODO(https://crbug.com/1063417): Replace this with something that works
-// on modern macOS versions.
-OSStatus MenuBarRevealHandler(EventHandlerCallRef handler,
-                              EventRef event,
-                              void* context) {
-  FullscreenMenubarTracker* self =
-      static_cast<FullscreenMenubarTracker*>(context);
-
-  // If Chrome has multiple fullscreen windows in their own space, the Handler
-  // becomes flaky and might start receiving kMenuBarRevealEventKind events
-  // from another space. Since the menubar in the another space is in either a
-  // shown or hidden state, it will give us a reveal fraction of 0.0 or 1.0.
-  // As such, we should ignore the kMenuBarRevealEventKind event if it gives
-  // us a fraction of 0.0 or 1.0, and rely on kEventMenuBarShown and
-  // kEventMenuBarHidden to set these values.
-  if (GetEventKind(event) == kMenuBarRevealEventKind) {
-    CGFloat revealFraction = 0;
-    GetEventParameter(event, FOUR_CHAR_CODE('rvlf'), typeCGFloat, NULL,
-                      sizeof(CGFloat), NULL, &revealFraction);
-    if (revealFraction > 0.0 && revealFraction < 1.0)
-      [self setMenubarProgress:revealFraction];
-  } else if (GetEventKind(event) == kEventMenuBarShown) {
-    [self setMenubarProgress:1.0];
-  } else {
-    [self setMenubarProgress:0.0];
-  }
-
-  return CallNextEventHandler(handler, event);
-}
-
-}  // end namespace
-
 @interface FullscreenMenubarTracker () {
   FullscreenToolbarController* _controller;        // weak
 
-  // A Carbon event handler that tracks the revealed fraction of the menubar.
-  EventHandlerRef _menubarTrackingHandler;
+  base::scoped_nsobject<MenuRevealMonitor> menu_reveal_monitor_;
 }
 
 // Returns YES if the mouse is on the same screen as the window.
@@ -72,22 +35,12 @@ OSStatus MenuBarRevealHandler(EventHandlerCallRef handler,
     _controller = controller;
     _state = FullscreenMenubarState::HIDDEN;
 
-    // Install the Carbon event handler for the menubar show, hide and
-    // undocumented reveal event.
-    EventTypeSpec eventSpecs[3];
-
-    eventSpecs[0].eventClass = kEventClassMenu;
-    eventSpecs[0].eventKind = kMenuBarRevealEventKind;
-
-    eventSpecs[1].eventClass = kEventClassMenu;
-    eventSpecs[1].eventKind = kEventMenuBarShown;
-
-    eventSpecs[2].eventClass = kEventClassMenu;
-    eventSpecs[2].eventKind = kEventMenuBarHidden;
-
-    InstallApplicationEventHandler(NewEventHandlerUPP(&MenuBarRevealHandler),
-                                   base::size(eventSpecs), eventSpecs, self,
-                                   &_menubarTrackingHandler);
+    __block FullscreenMenubarTracker* nonCaptureSelf = self;
+    menu_reveal_monitor_.reset([[MenuRevealMonitor alloc]
+        initWithWindow:[controller window]
+         changeHandler:^(double reveal_amount) {
+           [nonCaptureSelf setMenubarProgress:reveal_amount];
+         }]);
 
     // Register for Active Space change notifications.
     [[[NSWorkspace sharedWorkspace] notificationCenter]
@@ -100,9 +53,8 @@ OSStatus MenuBarRevealHandler(EventHandlerCallRef handler,
 }
 
 - (void)dealloc {
-  RemoveEventHandler(_menubarTrackingHandler);
   [[[NSWorkspace sharedWorkspace] notificationCenter] removeObserver:self];
-
+  menu_reveal_monitor_.reset();
   [super dealloc];
 }
 

chrome/browser/ui/cocoa/fullscreen/menu_reveal_monitor.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_UI_COCOA_FULLSCREEN_MENU_REVEAL_MONITOR_H_
+#define CHROME_BROWSER_UI_COCOA_FULLSCREEN_MENU_REVEAL_MONITOR_H_
+
+#import <Cocoa/Cocoa.h>
+
+// MenuRevealMonitor tracks visibility of the menu bar associated with |window|,
+// and calls |handler| when it changes. In fullscreen, when the mouse pointer
+// moves to or away from the top of the screen, |handler| will be called several
+// times with a number between zero and one indicating how much of the menu bar
+// is visible.
+@interface MenuRevealMonitor : NSObject
+- (instancetype)initWithWindow:(NSWindow*)window
+                 changeHandler:(void (^)(double))handler
+    NS_DESIGNATED_INITIALIZER;
+- (instancetype)init NS_UNAVAILABLE;
+@end
+
+#endif  // CHROME_BROWSER_UI_COCOA_FULLSCREEN_MENU_REVEAL_MONITOR_H_

chrome/browser/ui/cocoa/fullscreen/menu_reveal_monitor.mm

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import "chrome/browser/ui/cocoa/fullscreen/menu_reveal_monitor.h"
+
+#include "base/mac/foundation_util.h"
+#include "base/mac/scoped_block.h"
+#include "base/mac/scoped_nsobject.h"
+
+static NSString* const kRevealAmountKeyPath = @"revealAmount";
+
+@implementation MenuRevealMonitor {
+  base::mac::ScopedBlock<void (^)(double)> _change_handler;
+  base::scoped_nsobject<NSTitlebarAccessoryViewController> _accVC;
+}
+
+- (instancetype)initWithWindow:(NSWindow*)window
+                 changeHandler:(void (^)(double))handler {
+  if ((self = [super init])) {
+    _change_handler.reset([handler copy]);
+    _accVC.reset([[NSTitlebarAccessoryViewController alloc] init]);
+    auto* accVC = _accVC.get();
+    accVC.view = [[[NSView alloc] initWithFrame:NSZeroRect] autorelease];
+    [accVC addObserver:self
+            forKeyPath:kRevealAmountKeyPath
+               options:NSKeyValueObservingOptionNew
+               context:nil];
+    [window addTitlebarAccessoryViewController:accVC];
+  }
+  return self;
+}
+
+- (void)dealloc {
+  [_accVC removeObserver:self forKeyPath:kRevealAmountKeyPath];
+  [_accVC removeFromParentViewController];
+  [super dealloc];
+}
+
+- (void)observeValueForKeyPath:(NSString*)keyPath
+                      ofObject:(id)object
+                        change:(NSDictionary<NSKeyValueChangeKey, id>*)change
+                       context:(void*)context {
+  double revealAmount =
+      base::mac::ObjCCastStrict<NSNumber>(change[NSKeyValueChangeNewKey])
+          .doubleValue;
+  _change_handler.get()(revealAmount);
+}
+@end

chrome/browser/ui/views/frame/immersive_mode_controller_mac.mm

@@ -9,6 +9,7 @@
 #include "base/mac/foundation_util.h"
 #include "base/mac/scoped_nsobject.h"
 #include "base/memory/weak_ptr.h"
+#include "chrome/browser/ui/cocoa/fullscreen/menu_reveal_monitor.h"
 #include "chrome/browser/ui/cocoa/scoped_menu_bar_lock.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/frame/immersive_mode_controller.h"
@@ -21,56 +22,6 @@ namespace {
 const CGFloat kMenuBarLockPadding = 50;
 }
 
-// MenuRevealMonitor tracks visibility of the menu bar associated with |window|,
-// and calls |handler| when it changes. In fullscreen, when the mouse pointer
-// moves to or away from the top of the screen, |handler| will be called several
-// times with a number between zero and one indicating how much of the menu bar
-// is visible.
-@interface MenuRevealMonitor : NSObject
-- (instancetype)initWithWindow:(NSWindow*)window
-                 changeHandler:(void (^)(double))handler
-    NS_DESIGNATED_INITIALIZER;
-- (instancetype)init NS_UNAVAILABLE;
-@end
-
-@implementation MenuRevealMonitor {
-  base::mac::ScopedBlock<void (^)(double)> _change_handler;
-  base::scoped_nsobject<NSTitlebarAccessoryViewController> _accVC;
-}
-
-- (instancetype)initWithWindow:(NSWindow*)window
-                 changeHandler:(void (^)(double))handler {
-  if ((self = [super init])) {
-    _change_handler.reset([handler copy]);
-    _accVC.reset([[NSTitlebarAccessoryViewController alloc] init]);
-    auto* accVC = _accVC.get();
-    accVC.view = [[[NSView alloc] initWithFrame:NSZeroRect] autorelease];
-    [accVC addObserver:self
-            forKeyPath:@"revealAmount"
-               options:NSKeyValueObservingOptionNew
-               context:nil];
-    [window addTitlebarAccessoryViewController:accVC];
-  }
-  return self;
-}
-
-- (void)dealloc {
-  [_accVC removeObserver:self forKeyPath:@"revealAmount"];
-  [_accVC removeFromParentViewController];
-  [super dealloc];
-}
-
-- (void)observeValueForKeyPath:(NSString*)keyPath
-                      ofObject:(id)object
-                        change:(NSDictionary<NSKeyValueChangeKey, id>*)change
-                       context:(void*)context {
-  double revealAmount =
-      base::mac::ObjCCastStrict<NSNumber>(change[NSKeyValueChangeNewKey])
-          .doubleValue;
-  _change_handler.get()(revealAmount);
-}
-@end
-
 // ImmersiveToolbarOverlayView performs two functions. First, it hitTests to its
 // superview (BridgedContentView) to block mouse events from hitting siblings
 // which the toolbar might overlap, like RenderWidgetHostView. It also sets up a