revert_code_health_clean_up_stale_macwebcontentsocclusion.patch 10 KB

123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208209210211212213214215216217218219220221222223224225226227228229230231232233234235236237238239240241242243244245246247248249250251252253254255256257258259260261262263264265266267
  1. From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
  2. From: David Sanders <[email protected]>
  3. Date: Wed, 8 Jan 2025 23:53:27 -0800
  4. Subject: Revert "Code Health: Clean up stale MacWebContentsOcclusion"
  5. Chrome has removed this WebContentsOcclusion feature flag upstream,
  6. which is now causing our visibility tests to break. This patch
  7. restores the legacy occlusion behavior to ensure the roll can continue
  8. while we debug the issue.
  9. This patch can be removed when the root cause because the visibility
  10. specs failing on MacOS only is debugged and fixed. It should be removed
  11. before Electron 35's stable date.
  12. Refs: https://chromium-review.googlesource.com/c/chromium/src/+/6078344
  13. This partially (leaves the removal of the feature flag) reverts
  14. ef865130abd5539e7bce12308659b19980368f12.
  15. 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
  16. index 428902f90950b2e9434c8a624a313268ebb80cd4..afcc3bc481be6a16119f7e2af647276cb0dafa1e 100644
  17. --- a/content/app_shim_remote_cocoa/web_contents_occlusion_checker_mac.h
  18. +++ b/content/app_shim_remote_cocoa/web_contents_occlusion_checker_mac.h
  19. @@ -12,6 +12,8 @@
  20. #import "content/app_shim_remote_cocoa/web_contents_view_cocoa.h"
  21. #include "content/common/web_contents_ns_view_bridge.mojom.h"
  22. +extern CONTENT_EXPORT const base::FeatureParam<bool>
  23. + kEnhancedWindowOcclusionDetection;
  24. extern CONTENT_EXPORT const base::FeatureParam<bool>
  25. kDisplaySleepAndAppHideDetection;
  26. 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
  27. index 32523e6a6f800de3917d18825f94c66ef4ea4388..634d68b9a2840d7c9e7c3b5e23d8b1b8ac02456b 100644
  28. --- a/content/app_shim_remote_cocoa/web_contents_occlusion_checker_mac.mm
  29. +++ b/content/app_shim_remote_cocoa/web_contents_occlusion_checker_mac.mm
  30. @@ -19,6 +19,12 @@
  31. #include "content/public/browser/content_browser_client.h"
  32. #include "content/public/common/content_client.h"
  33. +using features::kMacWebContentsOcclusion;
  34. +
  35. +// Experiment features.
  36. +const base::FeatureParam<bool> kEnhancedWindowOcclusionDetection{
  37. + &kMacWebContentsOcclusion, "EnhancedWindowOcclusionDetection", false};
  38. +
  39. namespace {
  40. NSString* const kWindowDidChangePositionInWindowList =
  41. @@ -127,7 +133,8 @@ - (void)dealloc {
  42. - (BOOL)isManualOcclusionDetectionEnabled {
  43. return [WebContentsOcclusionCheckerMac
  44. - manualOcclusionDetectionSupportedForCurrentMacOSVersion];
  45. + manualOcclusionDetectionSupportedForCurrentMacOSVersion] &&
  46. + kEnhancedWindowOcclusionDetection.Get();
  47. }
  48. // Alternative implementation of orderWindow:relativeTo:. Replaces
  49. diff --git a/content/app_shim_remote_cocoa/web_contents_view_cocoa.mm b/content/app_shim_remote_cocoa/web_contents_view_cocoa.mm
  50. index 2991489fae8a4eecad97b1ecb2271f096d9a9229..93b7aa620ad1da250ac06e3383ca689732de12cd 100644
  51. --- a/content/app_shim_remote_cocoa/web_contents_view_cocoa.mm
  52. +++ b/content/app_shim_remote_cocoa/web_contents_view_cocoa.mm
  53. @@ -29,6 +29,7 @@
  54. #include "ui/resources/grit/ui_resources.h"
  55. using content::DropData;
  56. +using features::kMacWebContentsOcclusion;
  57. using remote_cocoa::mojom::DraggingInfo;
  58. using remote_cocoa::mojom::SelectionDirection;
  59. @@ -126,12 +127,15 @@ @implementation WebContentsViewCocoa {
  60. gfx::Rect _windowControlsOverlayRect;
  61. + BOOL _inFullScreenTransition;
  62. BOOL _willSetWebContentsOccludedAfterDelay;
  63. }
  64. + (void)initialize {
  65. - // Create the WebContentsOcclusionCheckerMac shared instance.
  66. - [WebContentsOcclusionCheckerMac sharedInstance];
  67. + if (base::FeatureList::IsEnabled(kMacWebContentsOcclusion)) {
  68. + // Create the WebContentsOcclusionCheckerMac shared instance.
  69. + [WebContentsOcclusionCheckerMac sharedInstance];
  70. + }
  71. }
  72. - (instancetype)initWithViewsHostableView:(ui::ViewsHostableView*)v {
  73. @@ -442,6 +446,7 @@ - (void)updateWebContentsVisibility:
  74. (remote_cocoa::mojom::Visibility)visibility {
  75. using remote_cocoa::mojom::Visibility;
  76. + DCHECK(base::FeatureList::IsEnabled(kMacWebContentsOcclusion));
  77. if (!_host)
  78. return;
  79. @@ -487,6 +492,20 @@ - (void)updateWebContentsVisibility {
  80. [self updateWebContentsVisibility:visibility];
  81. }
  82. +- (void)legacyUpdateWebContentsVisibility {
  83. + using remote_cocoa::mojom::Visibility;
  84. + if (!_host || _inFullScreenTransition)
  85. + return;
  86. + Visibility visibility = Visibility::kVisible;
  87. + if ([self isHiddenOrHasHiddenAncestor] || ![self window])
  88. + visibility = Visibility::kHidden;
  89. + else if ([[self window] occlusionState] & NSWindowOcclusionStateVisible)
  90. + visibility = Visibility::kVisible;
  91. + else
  92. + visibility = Visibility::kOccluded;
  93. + _host->OnWindowVisibilityChanged(visibility);
  94. +}
  95. +
  96. - (void)resizeSubviewsWithOldSize:(NSSize)oldBoundsSize {
  97. // Subviews do not participate in auto layout unless the the size this view
  98. // changes. This allows RenderWidgetHostViewMac::SetBounds(..) to select a
  99. @@ -509,11 +528,39 @@ - (void)viewWillMoveToWindow:(NSWindow*)newWindow {
  100. NSWindow* oldWindow = [self window];
  101. + if (base::FeatureList::IsEnabled(kMacWebContentsOcclusion)) {
  102. + if (oldWindow) {
  103. + [notificationCenter
  104. + removeObserver:self
  105. + name:NSWindowDidChangeOcclusionStateNotification
  106. + object:oldWindow];
  107. + }
  108. +
  109. + if (newWindow) {
  110. + [notificationCenter
  111. + addObserver:self
  112. + selector:@selector(windowChangedOcclusionState:)
  113. + name:NSWindowDidChangeOcclusionStateNotification
  114. + object:newWindow];
  115. + }
  116. +
  117. + return;
  118. + }
  119. +
  120. + _inFullScreenTransition = NO;
  121. if (oldWindow) {
  122. - [notificationCenter
  123. - removeObserver:self
  124. - name:NSWindowDidChangeOcclusionStateNotification
  125. - object:oldWindow];
  126. + NSArray* notificationsToRemove = @[
  127. + NSWindowDidChangeOcclusionStateNotification,
  128. + NSWindowWillEnterFullScreenNotification,
  129. + NSWindowDidEnterFullScreenNotification,
  130. + NSWindowWillExitFullScreenNotification,
  131. + NSWindowDidExitFullScreenNotification
  132. + ];
  133. + for (NSString* notificationName in notificationsToRemove) {
  134. + [notificationCenter removeObserver:self
  135. + name:notificationName
  136. + object:oldWindow];
  137. + }
  138. }
  139. if (newWindow) {
  140. @@ -521,26 +568,66 @@ - (void)viewWillMoveToWindow:(NSWindow*)newWindow {
  141. selector:@selector(windowChangedOcclusionState:)
  142. name:NSWindowDidChangeOcclusionStateNotification
  143. object:newWindow];
  144. + // The fullscreen transition causes spurious occlusion notifications.
  145. + // See https://crbug.com/1081229
  146. + [notificationCenter addObserver:self
  147. + selector:@selector(fullscreenTransitionStarted:)
  148. + name:NSWindowWillEnterFullScreenNotification
  149. + object:newWindow];
  150. + [notificationCenter addObserver:self
  151. + selector:@selector(fullscreenTransitionComplete:)
  152. + name:NSWindowDidEnterFullScreenNotification
  153. + object:newWindow];
  154. + [notificationCenter addObserver:self
  155. + selector:@selector(fullscreenTransitionStarted:)
  156. + name:NSWindowWillExitFullScreenNotification
  157. + object:newWindow];
  158. + [notificationCenter addObserver:self
  159. + selector:@selector(fullscreenTransitionComplete:)
  160. + name:NSWindowDidExitFullScreenNotification
  161. + object:newWindow];
  162. }
  163. }
  164. - (void)windowChangedOcclusionState:(NSNotification*)aNotification {
  165. - // Only respond to occlusion notifications sent by the occlusion checker.
  166. - NSDictionary* userInfo = [aNotification userInfo];
  167. - NSString* occlusionCheckerKey = [WebContentsOcclusionCheckerMac className];
  168. - if (userInfo[occlusionCheckerKey] != nil)
  169. - [self updateWebContentsVisibility];
  170. + if (!base::FeatureList::IsEnabled(kMacWebContentsOcclusion)) {
  171. + [self legacyUpdateWebContentsVisibility];
  172. + return;
  173. + }
  174. +}
  175. +
  176. +- (void)fullscreenTransitionStarted:(NSNotification*)notification {
  177. + _inFullScreenTransition = YES;
  178. +}
  179. +
  180. +- (void)fullscreenTransitionComplete:(NSNotification*)notification {
  181. + _inFullScreenTransition = NO;
  182. }
  183. - (void)viewDidMoveToWindow {
  184. + if (!base::FeatureList::IsEnabled(kMacWebContentsOcclusion)) {
  185. + [self legacyUpdateWebContentsVisibility];
  186. + return;
  187. + }
  188. +
  189. [self updateWebContentsVisibility];
  190. }
  191. - (void)viewDidHide {
  192. + if (!base::FeatureList::IsEnabled(kMacWebContentsOcclusion)) {
  193. + [self legacyUpdateWebContentsVisibility];
  194. + return;
  195. + }
  196. +
  197. [self updateWebContentsVisibility];
  198. }
  199. - (void)viewDidUnhide {
  200. + if (!base::FeatureList::IsEnabled(kMacWebContentsOcclusion)) {
  201. + [self legacyUpdateWebContentsVisibility];
  202. + return;
  203. + }
  204. +
  205. [self updateWebContentsVisibility];
  206. }
  207. diff --git a/content/common/features.cc b/content/common/features.cc
  208. index 31484e57bd0989af7a2e9584bbd430cdfa713346..fdb5e8f5f395b128c1c5300b94a50f693d6b52e1 100644
  209. --- a/content/common/features.cc
  210. +++ b/content/common/features.cc
  211. @@ -261,6 +261,14 @@ BASE_FEATURE(kIOSurfaceCapturer,
  212. base::FEATURE_ENABLED_BY_DEFAULT);
  213. #endif
  214. +// Feature that controls whether WebContentsOcclusionChecker should handle
  215. +// occlusion notifications.
  216. +#if BUILDFLAG(IS_MAC)
  217. +BASE_FEATURE(kMacWebContentsOcclusion,
  218. + "MacWebContentsOcclusion",
  219. + base::FEATURE_ENABLED_BY_DEFAULT);
  220. +#endif
  221. +
  222. // If this feature is enabled, media-device enumerations use a cache that is
  223. // invalidated upon notifications sent by base::SystemMonitor. If disabled, the
  224. // cache is considered invalid on every enumeration request.
  225. diff --git a/content/common/features.h b/content/common/features.h
  226. index 906c0da9313ac0272f5e5a79ef797de596804cfb..94b63dfe1235b7643e29926edd2c27c447302b35 100644
  227. --- a/content/common/features.h
  228. +++ b/content/common/features.h
  229. @@ -68,6 +68,9 @@ CONTENT_EXPORT BASE_DECLARE_FEATURE(kInterestGroupUpdateIfOlderThan);
  230. #if BUILDFLAG(IS_MAC)
  231. CONTENT_EXPORT BASE_DECLARE_FEATURE(kIOSurfaceCapturer);
  232. #endif
  233. +#if BUILDFLAG(IS_MAC)
  234. +CONTENT_EXPORT BASE_DECLARE_FEATURE(kMacWebContentsOcclusion);
  235. +#endif
  236. CONTENT_EXPORT BASE_DECLARE_FEATURE(kMediaDevicesSystemMonitorCache);
  237. CONTENT_EXPORT BASE_DECLARE_FEATURE(kMediaStreamTrackTransfer);
  238. CONTENT_EXPORT BASE_DECLARE_FEATURE(kMojoDedicatedThread);