Increase times for the merge request widget polling

Changes the polling to also stop when the window looses focus
The hidden polling is also now behind a feature flag to allow for
customers to turn it off if they wish to

Closes https://gitlab.com/gitlab-org/gitlab/-/issues/210377

app/assets/javascripts/smart_interval.js

@@ -33,7 +33,7 @@ export default class SmartInterval {
     this.state = {
       intervalId: null,
       currentInterval: this.cfg.startingInterval,
-      pageVisibility: 'visible',
+      pagevisibile: true,
     };
 
     this.initInterval();
@@ -91,8 +91,10 @@ export default class SmartInterval {
   }
 
   destroy() {
+    document.removeEventListener('visibilitychange', this.onVisibilityChange);
+    window.removeEventListener('blur', this.onWindowVisibilityChange);
+    window.removeEventListener('focus', this.onWindowVisibilityChange);
     this.cancel();
-    document.removeEventListener('visibilitychange', this.handleVisibilityChange);
     $(document)
       .off('visibilitychange')
       .off('beforeunload');
@@ -124,9 +126,21 @@ export default class SmartInterval {
       });
   }
 
+  onWindowVisibilityChange(e) {
+    this.state.pagevisibile = e.type === 'focus';
+    this.handleVisibilityChange();
+  }
+
+  onVisibilityChange(e) {
+    this.state.pagevisibile = e.target.visibilityState === 'visible';
+    this.handleVisibilityChange();
+  }
+
   initVisibilityChangeHandling() {
-    // cancel interval when tab no longer shown (prevents cached pages from polling)
-    document.addEventListener('visibilitychange', this.handleVisibilityChange.bind(this));
+    // cancel interval when tab or window is no longer shown (prevents cached pages from polling)
+    document.addEventListener('visibilitychange', this.onVisibilityChange.bind(this));
+    window.addEventListener('blur', this.onWindowVisibilityChange.bind(this));
+    window.addEventListener('focus', this.onWindowVisibilityChange.bind(this));
   }
 
   initPageUnloadHandling() {
@@ -135,8 +149,7 @@ export default class SmartInterval {
     $(document).on('beforeunload', () => this.cancel());
   }
 
-  handleVisibilityChange(e) {
-    this.state.pageVisibility = e.target.visibilityState;
+  handleVisibilityChange() {
     const intervalAction = this.isPageVisible()
       ? this.onVisibilityVisible
       : this.onVisibilityHidden;
@@ -166,7 +179,7 @@ export default class SmartInterval {
   }
 
   isPageVisible() {
-    return this.state.pageVisibility === 'visible';
+    return this.state.pagevisibile;
   }
 
   stopTimer() {

app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue

@@ -214,8 +214,6 @@ export default {
       return new MRWidgetService(this.getServiceEndpoints(store));
     },
     checkStatus(cb, isRebased) {
-      if (document.visibilityState !== 'visible') return Promise.resolve();
-
       return this.service
         .checkStatus()
         .then(({ data }) => {
@@ -238,10 +236,10 @@ export default {
     initPolling() {
       this.pollingInterval = new SmartInterval({
         callback: this.checkStatus,
-        startingInterval: 10000,
-        maxInterval: 30000,
-        hiddenInterval: 120000,
-        incrementByFactorOf: 5000,
+        startingInterval: 10 * 1000,
+        maxInterval: 240 * 1000,
+        hiddenInterval: window.gon?.features?.widgetVisibilityPolling && 360 * 1000,
+        incrementByFactorOf: 2,
       });
     },
     initDeploymentsPolling() {
@@ -253,10 +251,9 @@ export default {
     deploymentsPoll(callback) {
       return new SmartInterval({
         callback,
-        startingInterval: 30000,
-        maxInterval: 120000,
-        hiddenInterval: 240000,
-        incrementByFactorOf: 15000,
+        startingInterval: 30 * 1000,
+        maxInterval: 240 * 1000,
+        incrementByFactorOf: 4,
         immediateExecution: true,
       });
     },

app/controllers/projects/merge_requests_controller.rb

@@ -24,6 +24,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
     push_frontend_feature_flag(:single_mr_diff_view, @project, default_enabled: true)
     push_frontend_feature_flag(:suggest_pipeline) if experiment_enabled?(:suggest_pipeline)
     push_frontend_feature_flag(:code_navigation, @project)
+    push_frontend_feature_flag(:widget_visibility_polling, @project, default_enabled: true)
   end
 
   before_action do

changelogs/unreleased/ph-210377-increaseMrPollTimes.yml

+---
+title: Increase the timing of polling for the merge request widget
+merge_request:
+author:
+type: changed

spec/frontend/smart_interval_spec.js

+import $ from 'jquery';
+import { assignIn } from 'lodash';
+import waitForPromises from 'helpers/wait_for_promises';
+import SmartInterval from '~/smart_interval';
+
+jest.useFakeTimers();
+
+let interval;
+
+describe('SmartInterval', () => {
+  const DEFAULT_MAX_INTERVAL = 100;
+  const DEFAULT_STARTING_INTERVAL = 5;
+  const DEFAULT_INCREMENT_FACTOR = 2;
+
+  function createDefaultSmartInterval(config) {
+    const defaultParams = {
+      callback: () => Promise.resolve(),
+      startingInterval: DEFAULT_STARTING_INTERVAL,
+      maxInterval: DEFAULT_MAX_INTERVAL,
+      incrementByFactorOf: DEFAULT_INCREMENT_FACTOR,
+      lazyStart: false,
+      immediateExecution: false,
+      hiddenInterval: null,
+    };
+
+    if (config) {
+      assignIn(defaultParams, config);
+    }
+
+    return new SmartInterval(defaultParams);
+  }
+
+  afterEach(() => {
+    interval.destroy();
+  });
+
+  describe('Increment Interval', () => {
+    it('should increment the interval delay', () => {
+      interval = createDefaultSmartInterval();
+
+      jest.runOnlyPendingTimers();
+
+      return waitForPromises().then(() => {
+        const intervalConfig = interval.cfg;
+        const iterationCount = 4;
+        const maxIntervalAfterIterations =
+          intervalConfig.startingInterval * intervalConfig.incrementByFactorOf ** iterationCount;
+        const currentInterval = interval.getCurrentInterval();
+
+        // Provide some flexibility for performance of testing environment
+        expect(currentInterval).toBeGreaterThan(intervalConfig.startingInterval);
+        expect(currentInterval).toBeLessThanOrEqual(maxIntervalAfterIterations);
+      });
+    });
+
+    it('should not increment past maxInterval', () => {
+      interval = createDefaultSmartInterval({ maxInterval: DEFAULT_STARTING_INTERVAL });
+
+      jest.runOnlyPendingTimers();
+
+      return waitForPromises().then(() => {
+        const currentInterval = interval.getCurrentInterval();
+
+        expect(currentInterval).toBe(interval.cfg.maxInterval);
+      });
+    });
+
+    it('does not increment while waiting for callback', () => {
+      interval = createDefaultSmartInterval({
+        callback: () => new Promise($.noop),
+      });
+
+      jest.runOnlyPendingTimers();
+
+      return waitForPromises().then(() => {
+        const oneInterval = interval.cfg.startingInterval * DEFAULT_INCREMENT_FACTOR;
+
+        expect(interval.getCurrentInterval()).toEqual(oneInterval);
+      });
+    });
+  });
+
+  describe('Public methods', () => {
+    beforeEach(() => {
+      interval = createDefaultSmartInterval();
+    });
+
+    it('should cancel an interval', () => {
+      jest.runOnlyPendingTimers();
+
+      interval.cancel();
+
+      return waitForPromises().then(() => {
+        const { intervalId } = interval.state;
+        const currentInterval = interval.getCurrentInterval();
+        const intervalLowerLimit = interval.cfg.startingInterval;
+
+        expect(intervalId).toBeUndefined();
+        expect(currentInterval).toBe(intervalLowerLimit);
+      });
+    });
+
+    it('should resume an interval', () => {
+      jest.runOnlyPendingTimers();
+
+      interval.cancel();
+
+      interval.resume();
+
+      return waitForPromises().then(() => {
+        const { intervalId } = interval.state;
+
+        expect(intervalId).toBeTruthy();
+      });
+    });
+  });
+
+  describe('DOM Events', () => {
+    beforeEach(() => {
+      // This ensures DOM and DOM events are initialized for these specs.
+      setFixtures('<div></div>');
+
+      interval = createDefaultSmartInterval();
+    });
+
+    it('should pause when page is not visible', () => {
+      jest.runOnlyPendingTimers();
+
+      return waitForPromises().then(() => {
+        expect(interval.state.intervalId).toBeTruthy();
+
+        // simulates triggering of visibilitychange event
+        interval.onVisibilityChange({ target: { visibilityState: 'hidden' } });
+
+        expect(interval.state.intervalId).toBeUndefined();
+      });
+    });
+
+    it('should change to the hidden interval when page is not visible', () => {
+      interval.destroy();
+
+      const HIDDEN_INTERVAL = 1500;
+      interval = createDefaultSmartInterval({ hiddenInterval: HIDDEN_INTERVAL });
+
+      jest.runOnlyPendingTimers();
+
+      return waitForPromises().then(() => {
+        expect(interval.state.intervalId).toBeTruthy();
+        expect(
+          interval.getCurrentInterval() >= DEFAULT_STARTING_INTERVAL &&
+            interval.getCurrentInterval() <= DEFAULT_MAX_INTERVAL,
+        ).toBeTruthy();
+
+        // simulates triggering of visibilitychange event
+        interval.onVisibilityChange({ target: { visibilityState: 'hidden' } });
+
+        expect(interval.state.intervalId).toBeTruthy();
+        expect(interval.getCurrentInterval()).toBe(HIDDEN_INTERVAL);
+      });
+    });
+
+    it('should resume when page is becomes visible at the previous interval', () => {
+      jest.runOnlyPendingTimers();
+
+      return waitForPromises().then(() => {
+        expect(interval.state.intervalId).toBeTruthy();
+
+        // simulates triggering of visibilitychange event
+        interval.onVisibilityChange({ target: { visibilityState: 'hidden' } });
+
+        expect(interval.state.intervalId).toBeUndefined();
+
+        // simulates triggering of visibilitychange event
+        interval.onVisibilityChange({ target: { visibilityState: 'visible' } });
+
+        expect(interval.state.intervalId).toBeTruthy();
+      });
+    });
+
+    it('should cancel on page unload', () => {
+      jest.runOnlyPendingTimers();
+
+      return waitForPromises().then(() => {
+        $(document).triggerHandler('beforeunload');
+
+        expect(interval.state.intervalId).toBeUndefined();
+        expect(interval.getCurrentInterval()).toBe(interval.cfg.startingInterval);
+      });
+    });
+
+    it('should execute callback before first interval', () => {
+      interval = createDefaultSmartInterval({ immediateExecution: true });
+
+      expect(interval.cfg.immediateExecution).toBeFalsy();
+    });
+  });
+});

spec/frontend/vue_mr_widget/mr_widget_options_spec.js

@@ -273,25 +273,6 @@ describe('mrWidgetOptions', () => {
           };
         });
 
-        it('should not tell service to check status if document is not visible', () => {
-          Object.defineProperty(document, 'visibilityState', {
-            value: 'hidden',
-            configurable: true,
-          });
-          vm.checkStatus(cb);
-
-          return vm.$nextTick().then(() => {
-            expect(vm.service.checkStatus).not.toHaveBeenCalled();
-            expect(vm.mr.setData).not.toHaveBeenCalled();
-            expect(vm.handleNotification).not.toHaveBeenCalled();
-            expect(isCbExecuted).toBeFalsy();
-            Object.defineProperty(document, 'visibilityState', {
-              value: 'visible',
-              configurable: true,
-            });
-          });
-        });
-
         it('should tell service to check status if document is visible', () => {
           vm.checkStatus(cb);
 

spec/javascripts/smart_interval_spec.js

-import $ from 'jquery';
-import { assignIn } from 'lodash';
-import waitForPromises from 'spec/helpers/wait_for_promises';
-import SmartInterval from '~/smart_interval';
-
-describe('SmartInterval', function() {
-  const DEFAULT_MAX_INTERVAL = 100;
-  const DEFAULT_STARTING_INTERVAL = 5;
-  const DEFAULT_SHORT_TIMEOUT = 75;
-  const DEFAULT_INCREMENT_FACTOR = 2;
-
-  function createDefaultSmartInterval(config) {
-    const defaultParams = {
-      callback: () => Promise.resolve(),
-      startingInterval: DEFAULT_STARTING_INTERVAL,
-      maxInterval: DEFAULT_MAX_INTERVAL,
-      incrementByFactorOf: DEFAULT_INCREMENT_FACTOR,
-      lazyStart: false,
-      immediateExecution: false,
-      hiddenInterval: null,
-    };
-
-    if (config) {
-      assignIn(defaultParams, config);
-    }
-
-    return new SmartInterval(defaultParams);
-  }
-
-  beforeEach(() => {
-    jasmine.clock().install();
-  });
-
-  afterEach(() => {
-    jasmine.clock().uninstall();
-  });
-
-  describe('Increment Interval', function() {
-    it('should increment the interval delay', done => {
-      const smartInterval = createDefaultSmartInterval();
-
-      jasmine.clock().tick(DEFAULT_SHORT_TIMEOUT);
-
-      waitForPromises()
-        .then(() => {
-          const intervalConfig = smartInterval.cfg;
-          const iterationCount = 4;
-          const maxIntervalAfterIterations =
-            intervalConfig.startingInterval * intervalConfig.incrementByFactorOf ** iterationCount;
-          const currentInterval = smartInterval.getCurrentInterval();
-
-          // Provide some flexibility for performance of testing environment
-          expect(currentInterval).toBeGreaterThan(intervalConfig.startingInterval);
-          expect(currentInterval).toBeLessThanOrEqual(maxIntervalAfterIterations);
-        })
-        .then(done)
-        .catch(done.fail);
-    });
-
-    it('should not increment past maxInterval', done => {
-      const smartInterval = createDefaultSmartInterval({ maxInterval: DEFAULT_STARTING_INTERVAL });
-
-      jasmine.clock().tick(DEFAULT_STARTING_INTERVAL);
-      jasmine.clock().tick(DEFAULT_STARTING_INTERVAL * DEFAULT_INCREMENT_FACTOR);
-
-      waitForPromises()
-        .then(() => {
-          const currentInterval = smartInterval.getCurrentInterval();
-
-          expect(currentInterval).toBe(smartInterval.cfg.maxInterval);
-        })
-        .then(done)
-        .catch(done.fail);
-    });
-
-    it('does not increment while waiting for callback', done => {
-      const smartInterval = createDefaultSmartInterval({
-        callback: () => new Promise($.noop),
-      });
-
-      jasmine.clock().tick(DEFAULT_SHORT_TIMEOUT);
-
-      waitForPromises()
-        .then(() => {
-          const oneInterval = smartInterval.cfg.startingInterval * DEFAULT_INCREMENT_FACTOR;
-
-          expect(smartInterval.getCurrentInterval()).toEqual(oneInterval);
-        })
-        .then(done)
-        .catch(done.fail);
-    });
-  });
-
-  describe('Public methods', function() {
-    beforeEach(function() {
-      this.smartInterval = createDefaultSmartInterval();
-    });
-
-    it('should cancel an interval', function(done) {
-      const interval = this.smartInterval;
-
-      jasmine.clock().tick(DEFAULT_SHORT_TIMEOUT);
-
-      interval.cancel();
-
-      waitForPromises()
-        .then(() => {
-          const { intervalId } = interval.state;
-          const currentInterval = interval.getCurrentInterval();
-          const intervalLowerLimit = interval.cfg.startingInterval;
-
-          expect(intervalId).toBeUndefined();
-          expect(currentInterval).toBe(intervalLowerLimit);
-        })
-        .then(done)
-        .catch(done.fail);
-    });
-
-    it('should resume an interval', function(done) {
-      const interval = this.smartInterval;
-
-      jasmine.clock().tick(DEFAULT_SHORT_TIMEOUT);
-
-      interval.cancel();
-
-      interval.resume();
-
-      waitForPromises()
-        .then(() => {
-          const { intervalId } = interval.state;
-
-          expect(intervalId).toBeTruthy();
-        })
-        .then(done)
-        .catch(done.fail);
-    });
-  });
-
-  describe('DOM Events', function() {
-    beforeEach(function() {
-      // This ensures DOM and DOM events are initialized for these specs.
-      setFixtures('<div></div>');
-
-      this.smartInterval = createDefaultSmartInterval();
-    });
-
-    it('should pause when page is not visible', function(done) {
-      const interval = this.smartInterval;
-
-      jasmine.clock().tick(DEFAULT_SHORT_TIMEOUT);
-
-      waitForPromises()
-        .then(() => {
-          expect(interval.state.intervalId).toBeTruthy();
-
-          // simulates triggering of visibilitychange event
-          interval.handleVisibilityChange({ target: { visibilityState: 'hidden' } });
-
-          expect(interval.state.intervalId).toBeUndefined();
-        })
-        .then(done)
-        .catch(done.fail);
-    });
-
-    it('should change to the hidden interval when page is not visible', done => {
-      const HIDDEN_INTERVAL = 1500;
-      const interval = createDefaultSmartInterval({ hiddenInterval: HIDDEN_INTERVAL });
-
-      jasmine.clock().tick(DEFAULT_SHORT_TIMEOUT);
-
-      waitForPromises()
-        .then(() => {
-          expect(interval.state.intervalId).toBeTruthy();
-          expect(
-            interval.getCurrentInterval() >= DEFAULT_STARTING_INTERVAL &&
-              interval.getCurrentInterval() <= DEFAULT_MAX_INTERVAL,
-          ).toBeTruthy();
-
-          // simulates triggering of visibilitychange event
-          interval.handleVisibilityChange({ target: { visibilityState: 'hidden' } });
-
-          expect(interval.state.intervalId).toBeTruthy();
-          expect(interval.getCurrentInterval()).toBe(HIDDEN_INTERVAL);
-        })
-        .then(done)
-        .catch(done.fail);
-    });
-
-    it('should resume when page is becomes visible at the previous interval', function(done) {
-      const interval = this.smartInterval;
-
-      jasmine.clock().tick(DEFAULT_SHORT_TIMEOUT);
-
-      waitForPromises()
-        .then(() => {
-          expect(interval.state.intervalId).toBeTruthy();
-
-          // simulates triggering of visibilitychange event
-          interval.handleVisibilityChange({ target: { visibilityState: 'hidden' } });
-
-          expect(interval.state.intervalId).toBeUndefined();
-
-          // simulates triggering of visibilitychange event
-          interval.handleVisibilityChange({ target: { visibilityState: 'visible' } });
-
-          expect(interval.state.intervalId).toBeTruthy();
-        })
-        .then(done)
-        .catch(done.fail);
-    });
-
-    it('should cancel on page unload', function(done) {
-      const interval = this.smartInterval;
-
-      jasmine.clock().tick(DEFAULT_SHORT_TIMEOUT);
-
-      waitForPromises()
-        .then(() => {
-          $(document).triggerHandler('beforeunload');
-
-          expect(interval.state.intervalId).toBeUndefined();
-          expect(interval.getCurrentInterval()).toBe(interval.cfg.startingInterval);
-        })
-        .then(done)
-        .catch(done.fail);
-    });
-
-    it('should execute callback before first interval', function() {
-      const interval = createDefaultSmartInterval({ immediateExecution: true });
-
-      expect(interval.cfg.immediateExecution).toBeFalsy();
-    });
-  });
-});