fix: pageVisibility state when backgroundThrottling disabled (#39299

)

fix: pageVisibility state when backgroundThrottling disabled

patches/chromium/allow_disabling_blink_scheduler_throttling_per_renderview.patch

@@ -33,6 +33,21 @@ index 180abdc9f983887c83fd9d4a596472222e9ab472..00842717a7570561ee9e3eca11190ab5
    void SendWebPreferencesToRenderer();
    void SendRendererPreferencesToRenderer(
        const blink::RendererPreferences& preferences);
+diff --git a/content/browser/renderer_host/render_widget_host_view_aura.cc b/content/browser/renderer_host/render_widget_host_view_aura.cc
+index b69ec986a4d8a96677d785cb6768f0c0d154ff2a..047d1c96a2603fccc3adf645b08575230060d5f7 100644
+--- a/content/browser/renderer_host/render_widget_host_view_aura.cc
++++ b/content/browser/renderer_host/render_widget_host_view_aura.cc
+@@ -555,8 +555,8 @@ void RenderWidgetHostViewAura::ShowImpl(PageVisibilityState page_visibility) {
+   // OnShowWithPageVisibility will not call NotifyHostAndDelegateOnWasShown,
+   // which updates `visibility_`, unless the host is hidden. Make sure no update
+   // is needed.
+-  DCHECK(host_->is_hidden() || visibility_ == Visibility::VISIBLE);
+-  OnShowWithPageVisibility(page_visibility);
++  if (host_->is_hidden() || visibility_ == Visibility::VISIBLE)
++    OnShowWithPageVisibility(page_visibility);
+ }
+
+ void RenderWidgetHostViewAura::NotifyHostAndDelegateOnWasShown(
 diff --git a/content/public/browser/render_view_host.h b/content/public/browser/render_view_host.h
 index 9979c25ecd57e68331b628a518368635db5c2027..32733bf951af3eff7da5fd5758bbcbaa49ff0e3c 100644
 --- a/content/public/browser/render_view_host.h
@@ -72,10 +87,20 @@ index 2c3930e849719dce3871c12b073966ca370e5e43..990f88a20320a2f6f58cf2e0b4d37e39
    // Visibility -----------------------------------------------------------
 
 diff --git a/third_party/blink/renderer/core/exported/web_view_impl.cc b/third_party/blink/renderer/core/exported/web_view_impl.cc
-index 4f99cf1e984cb7411703e3e586203834bf218afe..86e0c9a457b6a43441183f7d95a400cbfd0de1e3 100644
+index 4f99cf1e984cb7411703e3e586203834bf218afe..a7d788610a3611877afe8d0e8b3f3d7507c71aab 100644
 --- a/third_party/blink/renderer/core/exported/web_view_impl.cc
 +++ b/third_party/blink/renderer/core/exported/web_view_impl.cc
-@@ -3895,13 +3895,21 @@ PageScheduler* WebViewImpl::Scheduler() const {
+@@ -2446,6 +2446,9 @@ void WebViewImpl::SetPageLifecycleStateInternal(
+   TRACE_EVENT2("navigation", "WebViewImpl::SetPageLifecycleStateInternal",
+                "old_state", old_state, "new_state", new_state);
+
++  if (!scheduler_throttling_allowed_)
++      new_state->visibility = mojom::blink::PageVisibilityState::kVisible;
++
+   bool storing_in_bfcache = new_state->is_in_back_forward_cache &&
+                             !old_state->is_in_back_forward_cache;
+   bool restoring_from_bfcache = !new_state->is_in_back_forward_cache &&
+@@ -3895,17 +3898,30 @@ PageScheduler* WebViewImpl::Scheduler() const {
    return GetPage()->GetPageScheduler();
  }
 
@@ -90,14 +115,29 @@ index 4f99cf1e984cb7411703e3e586203834bf218afe..86e0c9a457b6a43441183f7d95a400cb
      mojom::blink::PageVisibilityState visibility_state,
      bool is_initial_state) {
    DCHECK(GetPage());
-   GetPage()->SetVisibilityState(visibility_state, is_initial_state);
-   GetPage()->GetPageScheduler()->SetPageVisible(
+-  GetPage()->SetVisibilityState(visibility_state, is_initial_state);
+-  GetPage()->GetPageScheduler()->SetPageVisible(
 -      visibility_state == mojom::blink::PageVisibilityState::kVisible);
-+      scheduler_throttling_allowed_ ?
-+          (visibility_state == mojom::blink::PageVisibilityState::kVisible) : true);
-   // Notify observers of the change.
-   if (!is_initial_state) {
-     for (auto& observer : observers_)
+-  // Notify observers of the change.
+-  if (!is_initial_state) {
+-    for (auto& observer : observers_)
+-      observer.OnPageVisibilityChanged(visibility_state);
++  // If backgroundThrottling is disabled, the page is always visible.
++  if (!scheduler_throttling_allowed_) {
++    GetPage()->SetVisibilityState(mojom::blink::PageVisibilityState::kVisible, is_initial_state);
++    GetPage()->GetPageScheduler()->SetPageVisible(true);
++  } else {
++    bool is_visible = visibility_state == mojom::blink::PageVisibilityState::kVisible;
++    GetPage()->SetVisibilityState(visibility_state, is_initial_state);
++    GetPage()->GetPageScheduler()->SetPageVisible(is_visible);
++    // Notify observers of the change.
++    if (!is_initial_state) {
++      for (auto& observer : observers_)
++        observer.OnPageVisibilityChanged(visibility_state);
++    }
+   }
+ }
+
 diff --git a/third_party/blink/renderer/core/exported/web_view_impl.h b/third_party/blink/renderer/core/exported/web_view_impl.h
 index 421ca0b15eea5958d18e52118613c388aeef7dce..c3751889cc1289f237f9f8e0e22f321e8e793778 100644
 --- a/third_party/blink/renderer/core/exported/web_view_impl.h

patches/chromium/extend_apply_webpreferences.patch

@@ -12,7 +12,7 @@ Ideally we could add an embedder observer pattern here but that can be
 done in future work.
 
 diff --git a/third_party/blink/renderer/core/exported/web_view_impl.cc b/third_party/blink/renderer/core/exported/web_view_impl.cc
-index 86e0c9a457b6a43441183f7d95a400cbfd0de1e3..246744eff96e05d7c14bebd79e2591803d2e4ecf 100644
+index a7d788610a3611877afe8d0e8b3f3d7507c71aab..a94cb272bfdda37206e72f21e711479942662036 100644
 --- a/third_party/blink/renderer/core/exported/web_view_impl.cc
 +++ b/third_party/blink/renderer/core/exported/web_view_impl.cc
 @@ -165,6 +165,7 @@

spec/api-browser-window-spec.ts

@@ -4066,7 +4066,8 @@ describe('BrowserWindow module', () => {
     });
   });
 
-  describe('document.visibilityState/hidden', () => {
+  // TODO(codebytere): figure out how to make these pass in CI on Windows.
+  ifdescribe(process.platform !== 'win32')('document.visibilityState/hidden', () => {
     afterEach(closeAllWindows);
 
     it('visibilityState is initially visible despite window being hidden', async () => {
@@ -4094,8 +4095,7 @@ describe('BrowserWindow module', () => {
       expect(hidden).to.be.false('hidden');
     });
 
-    // TODO(nornagon): figure out why this is failing on windows
-    ifit(process.platform !== 'win32')('visibilityState changes when window is hidden', async () => {
+    it('visibilityState changes when window is hidden', async () => {
       const w = new BrowserWindow({
         width: 100,
         height: 100,
@@ -4122,8 +4122,7 @@ describe('BrowserWindow module', () => {
       }
     });
 
-    // TODO(nornagon): figure out why this is failing on windows
-    ifit(process.platform !== 'win32')('visibilityState changes when window is shown', async () => {
+    it('visibilityState changes when window is shown', async () => {
       const w = new BrowserWindow({
         width: 100,
         height: 100,
@@ -4144,7 +4143,7 @@ describe('BrowserWindow module', () => {
       expect(visibilityState).to.equal('visible');
     });
 
-    ifit(process.platform !== 'win32')('visibilityState changes when window is shown inactive', async () => {
+    it('visibilityState changes when window is shown inactive', async () => {
       const w = new BrowserWindow({
         width: 100,
         height: 100,
@@ -4164,7 +4163,6 @@ describe('BrowserWindow module', () => {
       expect(visibilityState).to.equal('visible');
     });
 
-    // TODO(nornagon): figure out why this is failing on windows
     ifit(process.platform === 'darwin')('visibilityState changes when window is minimized', async () => {
       const w = new BrowserWindow({
         width: 100,
@@ -4191,19 +4189,20 @@ describe('BrowserWindow module', () => {
       }
     });
 
-    // DISABLED-FIXME(MarshallOfSound): This test fails locally 100% of the time, on CI it started failing
-    // when we introduced the compositor recycling patch.  Should figure out how to fix this
     it('visibilityState remains visible if backgroundThrottling is disabled', async () => {
       const w = new BrowserWindow({
         show: false,
         width: 100,
         height: 100,
         webPreferences: {
           backgroundThrottling: false,
-          nodeIntegration: true
+          nodeIntegration: true,
+          contextIsolation: false
         }
       });
+
       w.loadFile(path.join(fixtures, 'pages', 'visibilitychange.html'));
+
       {
         const [, visibilityState, hidden] = await once(ipcMain, 'pong');
         expect(visibilityState).to.equal('visible');

spec/disabled-tests.json

@@ -1,7 +1,6 @@
 [
   "// NOTE: this file is used to disable tests in our test suite by their full title.",
   "BrowserWindow module BrowserWindow.loadURL(url) should emit did-fail-load event for files that do not exist",
-  "BrowserWindow module document.visibilityState/hidden visibilityState remains visible if backgroundThrottling is disabled",
   "Menu module Menu.setApplicationMenu unsets a menu with null",
   "process module main process process.takeHeapSnapshot() returns true on success",
   "protocol module protocol.registerFileProtocol throws an error when custom headers are invalid",

spec/fixtures/pages/visibilitychange.html

@@ -3,10 +3,8 @@
 <script type="text/javascript" charset="utf-8">
   const {ipcRenderer} = require('electron')
   ipcRenderer.send('pong', document.visibilityState, document.hidden)
-  document.addEventListener('visibilitychange', function () {
-    setTimeout(() => {
-      ipcRenderer.send('pong', document.visibilityState, document.hidden)
-    }, 500);
+  document.addEventListener('visibilitychange', () => {
+    ipcRenderer.send('pong', document.visibilityState, document.hidden)
   })
 </script>
 </body>