0

Code Health: Clean up stale MacWebContentsOcclusion

Fixed: 362247468
Change-Id: I1ac6f1427cfc4aa1d2b9de60c52b7b7849a45f1a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6078344
Commit-Queue: Nathan Memmott <memmott@chromium.org>
Reviewed-by: Jayson Adams <shrike@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1395120}
This commit is contained in:
Nathan Memmott
2024-12-11 14:40:57 -08:00
committed by Chromium LUCI CQ
parent 86451cddab
commit ef865130ab
7 changed files with 36 additions and 272 deletions

@ -12,8 +12,6 @@
#import "content/app_shim_remote_cocoa/web_contents_view_cocoa.h"
#include "content/common/web_contents_ns_view_bridge.mojom.h"
extern CONTENT_EXPORT const base::FeatureParam<bool>
kEnhancedWindowOcclusionDetection;
extern CONTENT_EXPORT const base::FeatureParam<bool>
kDisplaySleepAndAppHideDetection;

@ -19,12 +19,6 @@
#include "content/public/browser/content_browser_client.h"
#include "content/public/common/content_client.h"
using features::kMacWebContentsOcclusion;
// Experiment features.
const base::FeatureParam<bool> kEnhancedWindowOcclusionDetection{
&kMacWebContentsOcclusion, "EnhancedWindowOcclusionDetection", false};
namespace {
NSString* const kWindowDidChangePositionInWindowList =
@ -98,7 +92,6 @@ bool IsBrowserProcess() {
- (instancetype)init {
self = [super init];
DCHECK(base::FeatureList::IsEnabled(kMacWebContentsOcclusion));
DCHECK(IsBrowserProcess());
if (!IsBrowserProcess()) {
static auto* const crash_key = base::debug::AllocateCrashKeyString(
@ -134,8 +127,7 @@ bool IsBrowserProcess() {
- (BOOL)isManualOcclusionDetectionEnabled {
return [WebContentsOcclusionCheckerMac
manualOcclusionDetectionSupportedForCurrentMacOSVersion] &&
kEnhancedWindowOcclusionDetection.Get();
manualOcclusionDetectionSupportedForCurrentMacOSVersion];
}
// Alternative implementation of orderWindow:relativeTo:. Replaces

@ -29,7 +29,6 @@
#include "ui/resources/grit/ui_resources.h"
using content::DropData;
using features::kMacWebContentsOcclusion;
using remote_cocoa::mojom::DraggingInfo;
using remote_cocoa::mojom::SelectionDirection;
@ -127,17 +126,12 @@ STATIC_ASSERT_ENUM(NSDragOperationMove, ui::DragDropTypes::DRAG_MOVE);
gfx::Rect _windowControlsOverlayRect;
// TODO(crbug.com/40593221): Remove this when kMacWebContentsOcclusion
// is enabled by default.
BOOL _inFullScreenTransition;
BOOL _willSetWebContentsOccludedAfterDelay;
}
+ (void)initialize {
if (base::FeatureList::IsEnabled(kMacWebContentsOcclusion)) {
// Create the WebContentsOcclusionCheckerMac shared instance.
[WebContentsOcclusionCheckerMac sharedInstance];
}
// Create the WebContentsOcclusionCheckerMac shared instance.
[WebContentsOcclusionCheckerMac sharedInstance];
}
- (instancetype)initWithViewsHostableView:(ui::ViewsHostableView*)v {
@ -440,7 +434,6 @@ STATIC_ASSERT_ENUM(NSDragOperationMove, ui::DragDropTypes::DRAG_MOVE);
(remote_cocoa::mojom::Visibility)visibility {
using remote_cocoa::mojom::Visibility;
DCHECK(base::FeatureList::IsEnabled(kMacWebContentsOcclusion));
if (!_host)
return;
@ -486,21 +479,6 @@ STATIC_ASSERT_ENUM(NSDragOperationMove, ui::DragDropTypes::DRAG_MOVE);
[self updateWebContentsVisibility:visibility];
}
- (void)legacyUpdateWebContentsVisibility {
using remote_cocoa::mojom::Visibility;
DCHECK(!base::FeatureList::IsEnabled(kMacWebContentsOcclusion));
if (!_host || _inFullScreenTransition)
return;
Visibility visibility = Visibility::kVisible;
if ([self isHiddenOrHasHiddenAncestor] || ![self window])
visibility = Visibility::kHidden;
else if ([[self window] occlusionState] & NSWindowOcclusionStateVisible)
visibility = Visibility::kVisible;
else
visibility = Visibility::kOccluded;
_host->OnWindowVisibilityChanged(visibility);
}
- (void)resizeSubviewsWithOldSize:(NSSize)oldBoundsSize {
// Subviews do not participate in auto layout unless the the size this view
// changes. This allows RenderWidgetHostViewMac::SetBounds(..) to select a
@ -523,72 +501,22 @@ STATIC_ASSERT_ENUM(NSDragOperationMove, ui::DragDropTypes::DRAG_MOVE);
NSWindow* oldWindow = [self window];
if (base::FeatureList::IsEnabled(kMacWebContentsOcclusion)) {
if (oldWindow) {
[notificationCenter
removeObserver:self
name:NSWindowDidChangeOcclusionStateNotification
object:oldWindow];
}
if (newWindow) {
[notificationCenter
addObserver:self
selector:@selector(windowChangedOcclusionState:)
name:NSWindowDidChangeOcclusionStateNotification
object:newWindow];
}
return;
}
_inFullScreenTransition = NO;
if (oldWindow) {
NSArray* notificationsToRemove = @[
NSWindowDidChangeOcclusionStateNotification,
NSWindowWillEnterFullScreenNotification,
NSWindowDidEnterFullScreenNotification,
NSWindowWillExitFullScreenNotification,
NSWindowDidExitFullScreenNotification
];
for (NSString* notificationName in notificationsToRemove) {
[notificationCenter removeObserver:self
name:notificationName
object:oldWindow];
}
[notificationCenter
removeObserver:self
name:NSWindowDidChangeOcclusionStateNotification
object:oldWindow];
}
if (newWindow) {
[notificationCenter addObserver:self
selector:@selector(windowChangedOcclusionState:)
name:NSWindowDidChangeOcclusionStateNotification
object:newWindow];
// The fullscreen transition causes spurious occlusion notifications.
// See https://crbug.com/1081229
[notificationCenter addObserver:self
selector:@selector(fullscreenTransitionStarted:)
name:NSWindowWillEnterFullScreenNotification
object:newWindow];
[notificationCenter addObserver:self
selector:@selector(fullscreenTransitionComplete:)
name:NSWindowDidEnterFullScreenNotification
object:newWindow];
[notificationCenter addObserver:self
selector:@selector(fullscreenTransitionStarted:)
name:NSWindowWillExitFullScreenNotification
object:newWindow];
[notificationCenter addObserver:self
selector:@selector(fullscreenTransitionComplete:)
name:NSWindowDidExitFullScreenNotification
object:newWindow];
}
}
- (void)windowChangedOcclusionState:(NSNotification*)aNotification {
if (!base::FeatureList::IsEnabled(kMacWebContentsOcclusion)) {
[self legacyUpdateWebContentsVisibility];
return;
}
// Only respond to occlusion notifications sent by the occlusion checker.
NSDictionary* userInfo = [aNotification userInfo];
NSString* occlusionCheckerKey = [WebContentsOcclusionCheckerMac className];
@ -596,40 +524,15 @@ STATIC_ASSERT_ENUM(NSDragOperationMove, ui::DragDropTypes::DRAG_MOVE);
[self updateWebContentsVisibility];
}
- (void)fullscreenTransitionStarted:(NSNotification*)notification {
DCHECK(!base::FeatureList::IsEnabled(kMacWebContentsOcclusion));
_inFullScreenTransition = YES;
}
- (void)fullscreenTransitionComplete:(NSNotification*)notification {
DCHECK(!base::FeatureList::IsEnabled(kMacWebContentsOcclusion));
_inFullScreenTransition = NO;
}
- (void)viewDidMoveToWindow {
if (!base::FeatureList::IsEnabled(kMacWebContentsOcclusion)) {
[self legacyUpdateWebContentsVisibility];
return;
}
[self updateWebContentsVisibility];
}
- (void)viewDidHide {
if (!base::FeatureList::IsEnabled(kMacWebContentsOcclusion)) {
[self legacyUpdateWebContentsVisibility];
return;
}
[self updateWebContentsVisibility];
}
- (void)viewDidUnhide {
if (!base::FeatureList::IsEnabled(kMacWebContentsOcclusion)) {
[self legacyUpdateWebContentsVisibility];
return;
}
[self updateWebContentsVisibility];
}

@ -16,19 +16,15 @@
#include "content/public/test/browser_test.h"
#include "content/public/test/content_browser_test.h"
using content::DropData;
using remote_cocoa::mojom::DraggingInfo;
using remote_cocoa::mojom::DraggingInfoPtr;
using remote_cocoa::mojom::SelectionDirection;
using content::DropData;
namespace {
const int kNeverCalled = -100;
struct FeatureState {
bool enhanced_occlusion_detection_enabled = false;
};
struct Version {
int packed_version;
bool supported;
@ -333,21 +329,8 @@ class WebContentsNSViewHostStub
};
// Sets up occlusion tests.
class WindowOcclusionBrowserTestMac
: public ::testing::WithParamInterface<FeatureState>,
public ContentBrowserTest {
class WindowOcclusionBrowserTestMac : public ContentBrowserTest {
public:
WindowOcclusionBrowserTestMac() {
if (GetParam().enhanced_occlusion_detection_enabled) {
base::FieldTrialParams params;
params["EnhancedWindowOcclusionDetection"] = "true";
features_.InitAndEnableFeatureWithParameters(
features::kMacWebContentsOcclusion, params);
} else {
features_.InitAndDisableFeature(features::kMacWebContentsOcclusion);
}
}
void SetUp() override {
if (![NSClassFromString(@"WebContentsOcclusionCheckerMac")
manualOcclusionDetectionSupportedForCurrentMacOSVersion]) {
@ -397,9 +380,6 @@ class WindowOcclusionBrowserTestMac
}
void WaitForOcclusionUpdate() {
if (!base::FeatureList::IsEnabled(features::kMacWebContentsOcclusion))
return;
while ([[NSClassFromString(@"WebContentsOcclusionCheckerMac")
sharedInstance] occlusionStateUpdatesAreScheduledForTesting] ||
WebContentsAwaitingUpdates()) {
@ -480,16 +460,10 @@ class WindowOcclusionBrowserTestMac
void OrderWindowFront(NSWindow* window) {
[[maybe_unused]] WebContentVisibilityUpdateCounter* watcher;
if (!kEnhancedWindowOcclusionDetection.Get()) {
watcher = [[WebContentVisibilityUpdateCounter alloc] init];
}
[window orderFront:nil];
ASSERT_TRUE([window isVisible]);
if (kEnhancedWindowOcclusionDetection.Get()) {
WaitForOcclusionUpdate();
}
WaitForOcclusionUpdate();
}
void OrderWindowOut(NSWindow* window) {
@ -575,34 +549,8 @@ class WindowOcclusionBrowserTestMac
WebContentsNSViewHostStub host_a_;
};
using WindowOcclusionBrowserTestMacWithoutOcclusionFeature =
WindowOcclusionBrowserTestMac;
using WindowOcclusionBrowserTestMacWithOcclusionDetectionFeature =
WindowOcclusionBrowserTestMac;
// Tests that should only work without the occlusion detection feature.
INSTANTIATE_TEST_SUITE_P(NoFeature,
WindowOcclusionBrowserTestMacWithoutOcclusionFeature,
::testing::Values(FeatureState{
.enhanced_occlusion_detection_enabled = false}));
// Tests that should work with or without the occlusion detection feature.
INSTANTIATE_TEST_SUITE_P(
Common,
WindowOcclusionBrowserTestMac,
::testing::Values(
FeatureState{.enhanced_occlusion_detection_enabled = false},
FeatureState{.enhanced_occlusion_detection_enabled = true}));
// Tests that require enhanced window occlusion detection.
INSTANTIATE_TEST_SUITE_P(
EnhancedWindowOcclusionDetection,
WindowOcclusionBrowserTestMacWithOcclusionDetectionFeature,
::testing::Values(FeatureState{
.enhanced_occlusion_detection_enabled = true}));
// Tests that we correctly disallow unsupported macOS versions.
IN_PROC_BROWSER_TEST_P(WindowOcclusionBrowserTestMac, MacOSVersionChecking) {
IN_PROC_BROWSER_TEST_F(WindowOcclusionBrowserTestMac, MacOSVersionChecking) {
Class WebContentsOcclusionCheckerMac =
NSClassFromString(@"WebContentsOcclusionCheckerMac");
std::vector<Version> versions = {
@ -617,45 +565,8 @@ IN_PROC_BROWSER_TEST_P(WindowOcclusionBrowserTestMac, MacOSVersionChecking) {
}
}
// Tests that enhanced occlusion detection isn't triggered if the feature's
// not enabled.
IN_PROC_BROWSER_TEST_P(WindowOcclusionBrowserTestMacWithoutOcclusionFeature,
ManualOcclusionDetectionDisabled) {
InitWindowA();
// Create a second window and place it exactly over window_a. The window
// should still be considered visible.
InitWindowB([window_a_ frame]);
EXPECT_EQ(WindowAWebContentsVisibility(),
remote_cocoa::mojom::Visibility::kVisible);
}
// Test that display sleep and app hide detection don't work if the feature's
// not enabled.
IN_PROC_BROWSER_TEST_P(WindowOcclusionBrowserTestMacWithoutOcclusionFeature,
OcclusionDetectionOnDisplaySleepDisabled) {
InitWindowA();
EXPECT_EQ(WindowAWebContentsVisibility(),
remote_cocoa::mojom::Visibility::kVisible);
// Fake a display sleep notification.
ASSERT_TRUE(NSWorkspace.sharedWorkspace.notificationCenter);
[[maybe_unused]] WebContentVisibilityUpdateCounter* watcher =
[[WebContentVisibilityUpdateCounter alloc] init];
[NSWorkspace.sharedWorkspace.notificationCenter
postNotificationName:NSWorkspaceScreensDidSleepNotification
object:nil
userInfo:nil];
EXPECT_TRUE([WebContentVisibilityUpdateCounter methodNeverCalled]);
EXPECT_EQ(WindowAWebContentsVisibility(),
remote_cocoa::mojom::Visibility::kVisible);
}
// Test that we properly handle occlusion notifications from macOS.
IN_PROC_BROWSER_TEST_P(WindowOcclusionBrowserTestMac,
IN_PROC_BROWSER_TEST_F(WindowOcclusionBrowserTestMac,
MacOSOcclusionNotifications) {
InitWindowA();
@ -672,9 +583,8 @@ IN_PROC_BROWSER_TEST_P(WindowOcclusionBrowserTestMac,
remote_cocoa::mojom::Visibility::kVisible);
}
IN_PROC_BROWSER_TEST_P(
WindowOcclusionBrowserTestMacWithOcclusionDetectionFeature,
ManualOcclusionDetection) {
IN_PROC_BROWSER_TEST_F(WindowOcclusionBrowserTestMac,
ManualOcclusionDetection) {
InitWindowA();
// Create a second window and place it exactly over window_a. Unlike macOS,
@ -710,9 +620,8 @@ IN_PROC_BROWSER_TEST_P(
}
// Checks manual occlusion detection as windows change display order.
IN_PROC_BROWSER_TEST_P(
WindowOcclusionBrowserTestMacWithOcclusionDetectionFeature,
ManualOcclusionDetectionOnWindowOrderChange) {
IN_PROC_BROWSER_TEST_F(WindowOcclusionBrowserTestMac,
ManualOcclusionDetectionOnWindowOrderChange) {
InitWindowA();
// Size and position the second window so that it exactly covers the
@ -734,9 +643,8 @@ IN_PROC_BROWSER_TEST_P(
// Checks that window_a, occluded by window_b, transitions to kVisible while the
// user resizes window_b.
IN_PROC_BROWSER_TEST_P(
WindowOcclusionBrowserTestMacWithOcclusionDetectionFeature,
ManualOcclusionDetectionOnWindowLiveResize) {
IN_PROC_BROWSER_TEST_F(WindowOcclusionBrowserTestMac,
ManualOcclusionDetectionOnWindowLiveResize) {
InitWindowA();
// Size and position the second window so that it exactly covers the
@ -765,9 +673,8 @@ IN_PROC_BROWSER_TEST_P(
// Checks that window_a, occluded by window_b, transitions to kVisible when
// window_b is set to close.
IN_PROC_BROWSER_TEST_P(
WindowOcclusionBrowserTestMacWithOcclusionDetectionFeature,
ManualOcclusionDetectionOnWindowClose) {
IN_PROC_BROWSER_TEST_F(WindowOcclusionBrowserTestMac,
ManualOcclusionDetectionOnWindowClose) {
InitWindowA();
// Size and position the second window so that it exactly covers the
@ -788,9 +695,8 @@ IN_PROC_BROWSER_TEST_P(
// Checks that window_a, occluded by window_b and window_c, remains kOccluded
// when window_b is set to close.
IN_PROC_BROWSER_TEST_P(
WindowOcclusionBrowserTestMacWithOcclusionDetectionFeature,
ManualOcclusionDetectionOnMiddleWindowClose) {
IN_PROC_BROWSER_TEST_F(WindowOcclusionBrowserTestMac,
ManualOcclusionDetectionOnMiddleWindowClose) {
InitWindowA();
// Size and position the second window so that it exactly covers the
@ -821,9 +727,8 @@ IN_PROC_BROWSER_TEST_P(
}
// Checks that web contents are marked kHidden on display sleep.
IN_PROC_BROWSER_TEST_P(
WindowOcclusionBrowserTestMacWithOcclusionDetectionFeature,
OcclusionDetectionOnDisplaySleep) {
IN_PROC_BROWSER_TEST_F(WindowOcclusionBrowserTestMac,
OcclusionDetectionOnDisplaySleep) {
InitWindowA();
EXPECT_EQ(WindowAWebContentsVisibility(),
@ -844,9 +749,8 @@ IN_PROC_BROWSER_TEST_P(
// Checks that occlusion updates are ignored in between fullscreen transition
// notifications.
IN_PROC_BROWSER_TEST_P(
IN_PROC_BROWSER_TEST_F(
WindowOcclusionBrowserTestMac,
// WindowOcclusionBrowserTestMacWithOcclusionDetectionFeature,
IgnoreOcclusionUpdatesBetweenWindowFullscreenTransitionNotifications) {
InitWindowA();
@ -907,9 +811,8 @@ IN_PROC_BROWSER_TEST_P(
// Tests that each web contents in a window receives an updated occlusion
// state updated.
IN_PROC_BROWSER_TEST_P(
WindowOcclusionBrowserTestMacWithOcclusionDetectionFeature,
OcclusionDetectionForMultipleWebContents) {
IN_PROC_BROWSER_TEST_F(WindowOcclusionBrowserTestMac,
OcclusionDetectionForMultipleWebContents) {
InitWindowA();
EXPECT_EQ(WindowAWebContentsVisibility(),
@ -960,7 +863,7 @@ IN_PROC_BROWSER_TEST_P(
}
// Checks that web contentses are marked kHidden on WebContentsViewCocoa hide.
IN_PROC_BROWSER_TEST_P(WindowOcclusionBrowserTestMac,
IN_PROC_BROWSER_TEST_F(WindowOcclusionBrowserTestMac,
OcclusionDetectionOnWebContentsViewCocoaHide) {
InitWindowA();
@ -987,7 +890,7 @@ IN_PROC_BROWSER_TEST_P(WindowOcclusionBrowserTestMac,
// Checks that web contentses are marked kHidden on WebContentsViewCocoa removal
// from the view hierarchy.
IN_PROC_BROWSER_TEST_P(
IN_PROC_BROWSER_TEST_F(
WindowOcclusionBrowserTestMac,
OcclusionDetectionOnWebContentsViewCocoaRemoveFromSuperview) {
InitWindowA();
@ -1024,9 +927,8 @@ IN_PROC_BROWSER_TEST_P(
}
// Checks that web contentses are marked kHidden on window miniaturize.
IN_PROC_BROWSER_TEST_P(
WindowOcclusionBrowserTestMacWithOcclusionDetectionFeature,
OcclusionDetectionOnWindowMiniaturize) {
IN_PROC_BROWSER_TEST_F(WindowOcclusionBrowserTestMac,
OcclusionDetectionOnWindowMiniaturize) {
InitWindowA();
EXPECT_EQ(WindowAWebContentsVisibility(),
@ -1052,9 +954,8 @@ IN_PROC_BROWSER_TEST_P(
// triggering a visibility update, which causes a visibility watcher to add
// a second child window (while we're still inside AppKit code adding the
// first).
IN_PROC_BROWSER_TEST_P(
WindowOcclusionBrowserTestMacWithOcclusionDetectionFeature,
ChildWindowListMutationDuringManualOcclusionDetection) {
IN_PROC_BROWSER_TEST_F(WindowOcclusionBrowserTestMac,
ChildWindowListMutationDuringManualOcclusionDetection) {
InitWindowA();
const NSRect kContentRect = NSMakeRect(0.0, 0.0, 20.0, 20.0);
@ -1083,9 +984,8 @@ IN_PROC_BROWSER_TEST_P(
// Tests that when a window becomes a child, if the occlusion system
// previously marked it occluded, the window transitions to visible.
IN_PROC_BROWSER_TEST_P(
WindowOcclusionBrowserTestMacWithOcclusionDetectionFeature,
WindowMadeChildForcedVisible) {
IN_PROC_BROWSER_TEST_F(WindowOcclusionBrowserTestMac,
WindowMadeChildForcedVisible) {
InitWindowA();
// Create a second window that occludes window_a.

@ -305,14 +305,6 @@ BASE_FEATURE(kIOSurfaceCapturer,
base::FEATURE_ENABLED_BY_DEFAULT);
#endif
// Feature that controls whether WebContentsOcclusionChecker should handle
// occlusion notifications.
#if BUILDFLAG(IS_MAC)
BASE_FEATURE(kMacWebContentsOcclusion,
"MacWebContentsOcclusion",
base::FEATURE_ENABLED_BY_DEFAULT);
#endif
// If this feature is enabled, media-device enumerations use a cache that is
// invalidated upon notifications sent by base::SystemMonitor. If disabled, the
// cache is considered invalid on every enumeration request.

@ -81,9 +81,6 @@ CONTENT_EXPORT BASE_DECLARE_FEATURE(kInterestGroupUpdateIfOlderThan);
#if BUILDFLAG(IS_MAC)
CONTENT_EXPORT BASE_DECLARE_FEATURE(kIOSurfaceCapturer);
#endif
#if BUILDFLAG(IS_MAC)
CONTENT_EXPORT BASE_DECLARE_FEATURE(kMacWebContentsOcclusion);
#endif
CONTENT_EXPORT BASE_DECLARE_FEATURE(kMediaDevicesSystemMonitorCache);
CONTENT_EXPORT BASE_DECLARE_FEATURE(kMediaStreamTrackTransfer);
CONTENT_EXPORT BASE_DECLARE_FEATURE(kMojoDedicatedThread);

@ -13443,24 +13443,6 @@
]
}
],
"MacWebContentsOcclusionV2": [
{
"platforms": [
"mac"
],
"experiments": [
{
"name": "Enabled",
"params": {
"EnhancedWindowOcclusionDetection": "true"
},
"enable_features": [
"MacWebContentsOcclusion"
]
}
]
}
],
"MagicStackGradientView": [
{
"platforms": [