123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208209210211212213214215216217218219220221222223224225226227228229230231232233234235236237238239240241242243244245246247248249250251252253254255256257258259260261262263264265266267 |
- From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
- From: David Sanders <[email protected]>
- Date: Wed, 8 Jan 2025 23:53:27 -0800
- Subject: Revert "Code Health: Clean up stale MacWebContentsOcclusion"
- Chrome has removed this WebContentsOcclusion feature flag upstream,
- which is now causing our visibility tests to break. This patch
- restores the legacy occlusion behavior to ensure the roll can continue
- while we debug the issue.
- This patch can be removed when the root cause because the visibility
- specs failing on MacOS only is debugged and fixed. It should be removed
- before Electron 35's stable date.
- Refs: https://chromium-review.googlesource.com/c/chromium/src/+/6078344
- This partially (leaves the removal of the feature flag) reverts
- ef865130abd5539e7bce12308659b19980368f12.
- diff --git a/content/app_shim_remote_cocoa/web_contents_occlusion_checker_mac.h b/content/app_shim_remote_cocoa/web_contents_occlusion_checker_mac.h
- index 428902f90950b2e9434c8a624a313268ebb80cd4..afcc3bc481be6a16119f7e2af647276cb0dafa1e 100644
- --- a/content/app_shim_remote_cocoa/web_contents_occlusion_checker_mac.h
- +++ b/content/app_shim_remote_cocoa/web_contents_occlusion_checker_mac.h
- @@ -12,6 +12,8 @@
- #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;
-
- diff --git a/content/app_shim_remote_cocoa/web_contents_occlusion_checker_mac.mm b/content/app_shim_remote_cocoa/web_contents_occlusion_checker_mac.mm
- index 32523e6a6f800de3917d18825f94c66ef4ea4388..634d68b9a2840d7c9e7c3b5e23d8b1b8ac02456b 100644
- --- a/content/app_shim_remote_cocoa/web_contents_occlusion_checker_mac.mm
- +++ b/content/app_shim_remote_cocoa/web_contents_occlusion_checker_mac.mm
- @@ -19,6 +19,12 @@
- #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 =
- @@ -127,7 +133,8 @@ - (void)dealloc {
-
- - (BOOL)isManualOcclusionDetectionEnabled {
- return [WebContentsOcclusionCheckerMac
- - manualOcclusionDetectionSupportedForCurrentMacOSVersion];
- + manualOcclusionDetectionSupportedForCurrentMacOSVersion] &&
- + kEnhancedWindowOcclusionDetection.Get();
- }
-
- // Alternative implementation of orderWindow:relativeTo:. Replaces
- diff --git a/content/app_shim_remote_cocoa/web_contents_view_cocoa.mm b/content/app_shim_remote_cocoa/web_contents_view_cocoa.mm
- index 2991489fae8a4eecad97b1ecb2271f096d9a9229..93b7aa620ad1da250ac06e3383ca689732de12cd 100644
- --- a/content/app_shim_remote_cocoa/web_contents_view_cocoa.mm
- +++ b/content/app_shim_remote_cocoa/web_contents_view_cocoa.mm
- @@ -29,6 +29,7 @@
- #include "ui/resources/grit/ui_resources.h"
-
- using content::DropData;
- +using features::kMacWebContentsOcclusion;
- using remote_cocoa::mojom::DraggingInfo;
- using remote_cocoa::mojom::SelectionDirection;
-
- @@ -126,12 +127,15 @@ @implementation WebContentsViewCocoa {
-
- gfx::Rect _windowControlsOverlayRect;
-
- + BOOL _inFullScreenTransition;
- BOOL _willSetWebContentsOccludedAfterDelay;
- }
-
- + (void)initialize {
- - // Create the WebContentsOcclusionCheckerMac shared instance.
- - [WebContentsOcclusionCheckerMac sharedInstance];
- + if (base::FeatureList::IsEnabled(kMacWebContentsOcclusion)) {
- + // Create the WebContentsOcclusionCheckerMac shared instance.
- + [WebContentsOcclusionCheckerMac sharedInstance];
- + }
- }
-
- - (instancetype)initWithViewsHostableView:(ui::ViewsHostableView*)v {
- @@ -442,6 +446,7 @@ - (void)updateWebContentsVisibility:
- (remote_cocoa::mojom::Visibility)visibility {
- using remote_cocoa::mojom::Visibility;
-
- + DCHECK(base::FeatureList::IsEnabled(kMacWebContentsOcclusion));
- if (!_host)
- return;
-
- @@ -487,6 +492,20 @@ - (void)updateWebContentsVisibility {
- [self updateWebContentsVisibility:visibility];
- }
-
- +- (void)legacyUpdateWebContentsVisibility {
- + using remote_cocoa::mojom::Visibility;
- + 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
- @@ -509,11 +528,39 @@ - (void)viewWillMoveToWindow:(NSWindow*)newWindow {
-
- 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) {
- - [notificationCenter
- - removeObserver:self
- - name:NSWindowDidChangeOcclusionStateNotification
- - object:oldWindow];
- + NSArray* notificationsToRemove = @[
- + NSWindowDidChangeOcclusionStateNotification,
- + NSWindowWillEnterFullScreenNotification,
- + NSWindowDidEnterFullScreenNotification,
- + NSWindowWillExitFullScreenNotification,
- + NSWindowDidExitFullScreenNotification
- + ];
- + for (NSString* notificationName in notificationsToRemove) {
- + [notificationCenter removeObserver:self
- + name:notificationName
- + object:oldWindow];
- + }
- }
-
- if (newWindow) {
- @@ -521,26 +568,66 @@ - (void)viewWillMoveToWindow:(NSWindow*)newWindow {
- 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 {
- - // Only respond to occlusion notifications sent by the occlusion checker.
- - NSDictionary* userInfo = [aNotification userInfo];
- - NSString* occlusionCheckerKey = [WebContentsOcclusionCheckerMac className];
- - if (userInfo[occlusionCheckerKey] != nil)
- - [self updateWebContentsVisibility];
- + if (!base::FeatureList::IsEnabled(kMacWebContentsOcclusion)) {
- + [self legacyUpdateWebContentsVisibility];
- + return;
- + }
- +}
- +
- +- (void)fullscreenTransitionStarted:(NSNotification*)notification {
- + _inFullScreenTransition = YES;
- +}
- +
- +- (void)fullscreenTransitionComplete:(NSNotification*)notification {
- + _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];
- }
-
- diff --git a/content/common/features.cc b/content/common/features.cc
- index b55390ed12152c1d5ab01415ac2b64422b59ad8b..51d76c126025f422def2a46ebb5d6c427f5ae3cb 100644
- --- a/content/common/features.cc
- +++ b/content/common/features.cc
- @@ -262,6 +262,14 @@ 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.
- diff --git a/content/common/features.h b/content/common/features.h
- index 7a34bc2fafd421a3c63cb11706e1dac84ef02454..09f4a60267ea2ecb426edf3314274d6806a25bec 100644
- --- a/content/common/features.h
- +++ b/content/common/features.h
- @@ -68,6 +68,9 @@ 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);
|