Browse Source

fix: re-enable MacWebContentsOcclusion feature flag (#45801)

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Keeley Hammond <[email protected]>
trop[bot] 1 month ago
parent
commit
7a0abfd667

+ 141 - 17
patches/chromium/revert_code_health_clean_up_stale_macwebcontentsocclusion.patch

@@ -17,11 +17,59 @@ 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..6c735286bf901fc7ff3872830d83fe119dd3bd33 100644
+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
-@@ -126,13 +126,11 @@ @implementation WebContentsViewCocoa {
+@@ -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;
  
@@ -29,15 +77,25 @@ index 2991489fae8a4eecad97b1ecb2271f096d9a9229..6c735286bf901fc7ff3872830d83fe11
    BOOL _willSetWebContentsOccludedAfterDelay;
  }
  
--+ (void)initialize {
+ + (void)initialize {
 -  // Create the WebContentsOcclusionCheckerMac shared instance.
 -  [WebContentsOcclusionCheckerMac sharedInstance];
--}
-++ (void)initialize { }
++  if (base::FeatureList::IsEnabled(kMacWebContentsOcclusion)) {
++    // Create the WebContentsOcclusionCheckerMac shared instance.
++    [WebContentsOcclusionCheckerMac sharedInstance];
++  }
+ }
  
  - (instancetype)initWithViewsHostableView:(ui::ViewsHostableView*)v {
-   self = [super initWithFrame:NSZeroRect tracking:YES];
-@@ -487,6 +485,20 @@ - (void)updateWebContentsVisibility {
+@@ -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];
  }
  
@@ -58,10 +116,29 @@ index 2991489fae8a4eecad97b1ecb2271f096d9a9229..6c735286bf901fc7ff3872830d83fe11
  - (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 +521,20 @@ - (void)viewWillMoveToWindow:(NSWindow*)newWindow {
+@@ -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
@@ -83,7 +160,7 @@ index 2991489fae8a4eecad97b1ecb2271f096d9a9229..6c735286bf901fc7ff3872830d83fe11
    }
  
    if (newWindow) {
-@@ -521,27 +542,49 @@ - (void)viewWillMoveToWindow:(NSWindow*)newWindow {
+@@ -521,26 +568,66 @@ - (void)viewWillMoveToWindow:(NSWindow*)newWindow {
                             selector:@selector(windowChangedOcclusionState:)
                                 name:NSWindowDidChangeOcclusionStateNotification
                               object:newWindow];
@@ -114,7 +191,10 @@ index 2991489fae8a4eecad97b1ecb2271f096d9a9229..6c735286bf901fc7ff3872830d83fe11
 -  NSString* occlusionCheckerKey = [WebContentsOcclusionCheckerMac className];
 -  if (userInfo[occlusionCheckerKey] != nil)
 -    [self updateWebContentsVisibility];
-+  [self legacyUpdateWebContentsVisibility];
++  if (!base::FeatureList::IsEnabled(kMacWebContentsOcclusion)) {
++    [self legacyUpdateWebContentsVisibility];
++    return;
++  }
 +}
 +
 +- (void)fullscreenTransitionStarted:(NSNotification*)notification {
@@ -126,18 +206,62 @@ index 2991489fae8a4eecad97b1ecb2271f096d9a9229..6c735286bf901fc7ff3872830d83fe11
  }
  
  - (void)viewDidMoveToWindow {
--  [self updateWebContentsVisibility];
-+  [self legacyUpdateWebContentsVisibility];
++  if (!base::FeatureList::IsEnabled(kMacWebContentsOcclusion)) {
++    [self legacyUpdateWebContentsVisibility];
++    return;
++  }
++
+   [self updateWebContentsVisibility];
  }
  
  - (void)viewDidHide {
--  [self updateWebContentsVisibility];
-+  [self legacyUpdateWebContentsVisibility];
++  if (!base::FeatureList::IsEnabled(kMacWebContentsOcclusion)) {
++    [self legacyUpdateWebContentsVisibility];
++    return;
++  }
++
+   [self updateWebContentsVisibility];
  }
  
  - (void)viewDidUnhide {
--  [self updateWebContentsVisibility];
-+  [self legacyUpdateWebContentsVisibility];
++  if (!base::FeatureList::IsEnabled(kMacWebContentsOcclusion)) {
++    [self legacyUpdateWebContentsVisibility];
++    return;
++  }
++
+   [self updateWebContentsVisibility];
  }
  
- // ViewsHostable protocol implementation.
+diff --git a/content/common/features.cc b/content/common/features.cc
+index be5847985950e9136fc975fdbc021308f48f41b5..a5062c8fb5c5d9f1427819131b71ad6896e9d1df 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 06562dd07ac4f1cad1011d99aed6c09958044486..9748070bc26f8314faec99afdb20a5d91bc9dde2 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);

+ 8 - 0
shell/browser/feature_list.cc

@@ -11,6 +11,7 @@
 #include "base/feature_list.h"
 #include "base/metrics/field_trial.h"
 #include "components/spellcheck/common/spellcheck_features.h"
+#include "content/common/features.h"
 #include "content/public/common/content_features.h"
 #include "electron/buildflags/buildflags.h"
 #include "media/base/media_switches.h"
@@ -48,6 +49,13 @@ void InitializeFeatureList() {
       std::string(",") + spellcheck::kWinDelaySpellcheckServiceInit.name;
 #endif
 
+#if BUILDFLAG(IS_MAC)
+  disable_features +=
+      // MacWebContentsOcclusion is causing some odd visibility
+      // issues with multiple web contents
+      std::string(",") + features::kMacWebContentsOcclusion.name;
+#endif
+
 #if BUILDFLAG(ENABLE_PDF_VIEWER)
   // Enable window.showSaveFilePicker api for saving pdf files.
   // Refs https://issues.chromium.org/issues/373852607