Commit 4fd5babd authored by Peter Marshall's avatar Peter Marshall Committed by Commit Bot

Revert "Mac: don't use a custom tracking area in BridgedContentView"

This reverts commit 7e18556b.

Reason for revert: Breaks devtools overlay: crbug.com/1090280
and link hovering: https://bugs.chromium.org/p/chromium/issues/detail?id=841792#c24

Original change's description:
> Mac: don't use a custom tracking area in BridgedContentView
> 
> Currently, in BridgedContentView we "opt out"[1] of BaseView's tracking area to install a different one which:
> - Specifies `NSTrackingCursorUpdate`
> - As a consequence, only tracks while app is active, since the docs say that always tracking is incompatible with cursor updates
> 
> However:
> - Empirically, cursor updates work fine without this (some test cases: move mouse between Omnibox and other parts of topchrome, move from webcontent links/inputs to topchrome and vice-versa).
> - Only tracking while app is active means we don't get entered/exited events from app switching. This means:
>   - Switching away from Chrome while showing a topchrome hover tooltip keeps the hover on and makes the tooltip stuck
>   - Under some circumstances, hover stops working on switching back to Chrome (see bug)
>   - Even though we allow mouse clicks to function when Chrome is inactive (that is, we return `YES` from `acceptsFirstMouse:`, we don't process hovers in this case, contrary to other apps on the system.
> 
> This change removes BridgedContentView's tracking area in favor of the default one and gives BaseView some explicit enable/disables, mostly to make tests happy.
> 
> [1] By overriding a private method with a no-op!
> 
> Bug: 841792
> 
> Change-Id: Ifb5d65e22299959b8cac25a9e0a0407650a40746
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2216530
> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Commit-Queue: Leonard Grey <lgrey@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#773848}

TBR=ellyjones@chromium.org,avi@chromium.org,lgrey@chromium.org

Change-Id: Ic1344adb7cdf271636026a2b1e6def014852416f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 841792, 1090280
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2223949Reviewed-by: default avatarPeter Marshall <petermarshall@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#774088}
parent c62a83d8
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "base/strings/string16.h" #include "base/strings/string16.h"
#include "components/remote_cocoa/app_shim/remote_cocoa_app_shim_export.h" #include "components/remote_cocoa/app_shim/remote_cocoa_app_shim_export.h"
#import "ui/base/cocoa/tool_tip_base_view.h" #import "ui/base/cocoa/tool_tip_base_view.h"
#import "ui/base/cocoa/tracking_area.h"
namespace remote_cocoa { namespace remote_cocoa {
class NativeWidgetNSWindowBridge; class NativeWidgetNSWindowBridge;
...@@ -31,6 +32,9 @@ REMOTE_COCOA_APP_SHIM_EXPORT ...@@ -31,6 +32,9 @@ REMOTE_COCOA_APP_SHIM_EXPORT
// Weak, reset by clearView. // Weak, reset by clearView.
remote_cocoa::NativeWidgetNSWindowBridge* _bridge; remote_cocoa::NativeWidgetNSWindowBridge* _bridge;
// A tracking area installed to enable mouseMoved events.
ui::ScopedCrTrackingArea _cursorTrackingArea;
// The keyDown event currently being handled, nil otherwise. // The keyDown event currently being handled, nil otherwise.
NSEvent* _keyDownEvent; NSEvent* _keyDownEvent;
......
...@@ -172,6 +172,17 @@ ui::TextEditCommand GetTextEditCommandForMenuAction(SEL action) { ...@@ -172,6 +172,17 @@ ui::TextEditCommand GetTextEditCommandForMenuAction(SEL action) {
if ((self = [super initWithFrame:initialFrame])) { if ((self = [super initWithFrame:initialFrame])) {
_bridge = bridge; _bridge = bridge;
// Apple's documentation says that NSTrackingActiveAlways is incompatible
// with NSTrackingCursorUpdate, so use NSTrackingActiveInActiveApp.
_cursorTrackingArea.reset([[CrTrackingArea alloc]
initWithRect:NSZeroRect
options:NSTrackingMouseMoved | NSTrackingCursorUpdate |
NSTrackingActiveInActiveApp | NSTrackingInVisibleRect |
NSTrackingMouseEnteredAndExited
owner:self
userInfo:nil]);
[self addTrackingArea:_cursorTrackingArea.get()];
// Get notified whenever Full Keyboard Access mode is changed. // Get notified whenever Full Keyboard Access mode is changed.
[[NSDistributedNotificationCenter defaultCenter] [[NSDistributedNotificationCenter defaultCenter]
addObserver:self addObserver:self
...@@ -216,6 +227,8 @@ ui::TextEditCommand GetTextEditCommandForMenuAction(SEL action) { ...@@ -216,6 +227,8 @@ ui::TextEditCommand GetTextEditCommandForMenuAction(SEL action) {
- (void)clearView { - (void)clearView {
_bridge = nullptr; _bridge = nullptr;
[[NSDistributedNotificationCenter defaultCenter] removeObserver:self]; [[NSDistributedNotificationCenter defaultCenter] removeObserver:self];
[_cursorTrackingArea.get() clearOwner];
[self removeTrackingArea:_cursorTrackingArea.get()];
} }
- (bool)needsUpdateWindows { - (bool)needsUpdateWindows {
...@@ -532,6 +545,11 @@ ui::TextEditCommand GetTextEditCommandForMenuAction(SEL action) { ...@@ -532,6 +545,11 @@ ui::TextEditCommand GetTextEditCommandForMenuAction(SEL action) {
// BaseView implementation. // BaseView implementation.
// Don't use tracking areas from BaseView. BridgedContentView's tracks
// NSTrackingCursorUpdate and Apple's documentation suggests it's incompatible.
- (void)enableTracking {
}
// Translates the location of |theEvent| to toolkit-views coordinates and passes // Translates the location of |theEvent| to toolkit-views coordinates and passes
// the event to NativeWidgetMac for handling. // the event to NativeWidgetMac for handling.
- (void)mouseEvent:(NSEvent*)theEvent { - (void)mouseEvent:(NSEvent*)theEvent {
......
...@@ -12,23 +12,29 @@ NSString* kSelectionDirection = @"Chromium.kSelectionDirection"; ...@@ -12,23 +12,29 @@ NSString* kSelectionDirection = @"Chromium.kSelectionDirection";
@implementation BaseView @implementation BaseView
- (instancetype)initWithFrame:(NSRect)frame {
- (void)dealloc { if ((self = [super initWithFrame:frame])) {
[self disableTracking]; [self enableTracking];
[super dealloc]; }
return self;
} }
- (void)viewWillMoveToWindow:(NSWindow*)window { - (instancetype)initWithCoder:(NSCoder*)decoder {
if (window) { if ((self = [super initWithCoder:decoder])) {
[self enableTracking]; [self enableTracking];
} else {
[self disableTracking];
} }
return self;
}
- (void)dealloc {
[self disableTracking];
[super dealloc];
} }
- (void)enableTracking { - (void)enableTracking {
if (_trackingArea.get()) if (_trackingArea.get())
return; return;
NSTrackingAreaOptions trackingOptions = NSTrackingMouseEnteredAndExited | NSTrackingAreaOptions trackingOptions = NSTrackingMouseEnteredAndExited |
NSTrackingMouseMoved | NSTrackingMouseMoved |
NSTrackingActiveAlways | NSTrackingActiveAlways |
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment