Fix instantly sending RRs

Splits UserActivity into a tristate of 'active' (last < 1s), 'passive' (lasts a
couple of mins) and neither. Read receipts are sent when 'active', read markers
are sent while 'passive'.

Also fixed a document / window mix-up on the 'blur' handler.

Also adds a unit test for UserActivity because it's quite complex now
(and changes UserActivity to make it testable by accessing the singleton
via sharedInstance() rather than exporting it directly).

Fixes https://github.com/vector-im/riot-web/issues/9023

src/Lifecycle.js

@@ -439,7 +439,7 @@ async function startMatrixClient() {
     dis.dispatch({action: 'will_start_client'}, true);
 
     Notifier.start();
-    UserActivity.start();
+    UserActivity.sharedInstance().start();
     Presence.start();
     DMRoomMap.makeShared().start();
     ActiveWidgetStore.start();

src/UserActivity.js

@@ -23,16 +23,25 @@ import Timer from './utils/Timer';
 // such as READ_MARKER_INVIEW_THRESHOLD_MS,
 // READ_MARKER_OUTOFVIEW_THRESHOLD_MS,
 // READ_RECEIPT_INTERVAL_MS in TimelinePanel
-const CURRENTLY_ACTIVE_THRESHOLD_MS = 2 * 60 * 1000;
+const CURRENTLY_ACTIVE_THRESHOLD_MS = 700;
+const CURRENTLY_PASSIVE_THRESHOLD_MS = 2 * 60 * 1000;
 
 /**
  * This class watches for user activity (moving the mouse or pressing a key)
  * and starts/stops attached timers while the user is active.
+ *
+ * There are two classes of 'active' a user can be: 'active' and 'passive':
+ * see doc on the userCurrently* functions for what these mean.
  */
-class UserActivity {
+export default class UserActivity {
-    constructor() {
+    constructor(windowObj, documentObj) {
-        this._attachedTimers = [];
+        this._window = windowObj;
-        this._activityTimeout = new Timer(CURRENTLY_ACTIVE_THRESHOLD_MS);
+        this._document = documentObj;
+
+        this._attachedTimersActive = [];
+        this._attachedTimersPassive = [];
+        this._activeTimeout = new Timer(CURRENTLY_ACTIVE_THRESHOLD_MS);
+        this._passiveTimeout = new Timer(CURRENTLY_PASSIVE_THRESHOLD_MS);
         this._onUserActivity = this._onUserActivity.bind(this);
         this._onWindowBlurred = this._onWindowBlurred.bind(this);
         this._onPageVisibilityChanged = this._onPageVisibilityChanged.bind(this);
@@ -40,48 +49,76 @@ class UserActivity {
         this.lastScreenY = 0;
     }
 
+    static sharedInstance() {
+        if (global.mxUserActivity === undefined) {
+            global.mxUserActivity = new UserActivity(window, document);
+        }
+        return global.mxUserActivity;
+    }
+
     /**
-     * Runs the given timer while the user is active, aborting when the user becomes inactive.
+     * Runs the given timer while the user is 'active', aborting when the user becomes 'passive'.
+     * See userCurrentlyActive() for what it means for a user to be 'active'.
      * Can be called multiple times with the same already running timer, which is a NO-OP.
      * Can be called before the user becomes active, in which case it is only started
      * later on when the user does become active.
      * @param {Timer} timer the timer to use
      */
     timeWhileActive(timer) {
+        this._timeWhile(timer, this._attachedTimersActive);
+        if (this.userCurrentlyActive()) {
+            timer.start();
+        }
+    }
+
+    /**
+     * Runs the given timer while the user is 'active' or 'passive', aborting when the user becomes
+     * inactive.
+     * See userCurrentlyPassive() for what it means for a user to be 'passive'.
+     * Can be called multiple times with the same already running timer, which is a NO-OP.
+     * Can be called before the user becomes active, in which case it is only started
+     * later on when the user does become active.
+     * @param {Timer} timer the timer to use
+     */
+    timeWhilePassive(timer) {
+        this._timeWhile(timer, this._attachedTimersPassive);
+        if (this.userCurrentlyPassive()) {
+            timer.start();
+        }
+    }
+
+    _timeWhile(timer, attachedTimers) {
         // important this happens first
-        const index = this._attachedTimers.indexOf(timer);
+        const index = attachedTimers.indexOf(timer);
         if (index === -1) {
-            this._attachedTimers.push(timer);
+            attachedTimers.push(timer);
             // remove when done or aborted
             timer.finished().finally(() => {
-                const index = this._attachedTimers.indexOf(timer);
+                const index = attachedTimers.indexOf(timer);
                 if (index !== -1) { // should never be -1
-                    this._attachedTimers.splice(index, 1);
+                    attachedTimers.splice(index, 1);
                 }
             // as we fork the promise here,
             // avoid unhandled rejection warnings
             }).catch((err) => {});
         }
-        if (this.userCurrentlyActive()) {
-            timer.start();
-        }
     }
 
     /**
      * Start listening to user activity
      */
     start() {
-        document.onmousedown = this._onUserActivity;
+        this._document.onmousedown = this._onUserActivity;
-        document.onmousemove = this._onUserActivity;
+        this._document.onmousemove = this._onUserActivity;
-        document.onkeydown = this._onUserActivity;
+        this._document.onkeydown = this._onUserActivity;
-        document.addEventListener("visibilitychange", this._onPageVisibilityChanged);
+        this._document.addEventListener("visibilitychange", this._onPageVisibilityChanged);
-        window.addEventListener("blur", this._onWindowBlurred);
+        this._window.addEventListener("blur", this._onWindowBlurred);
-        window.addEventListener("focus", this._onUserActivity);
+        this._window.addEventListener("focus", this._onUserActivity);
         // can't use document.scroll here because that's only the document
         // itself being scrolled. Need to use addEventListener's useCapture.
         // also this needs to be the wheel event, not scroll, as scroll is
         // fired when the view scrolls down for a new message.
-        window.addEventListener('wheel', this._onUserActivity,
+        this._window.addEventListener('wheel', this._onUserActivity,
                                 { passive: true, capture: true });
     }
 
@@ -89,39 +126,57 @@ class UserActivity {
      * Stop tracking user activity
      */
     stop() {
-        document.onmousedown = undefined;
+        this._document.onmousedown = undefined;
-        document.onmousemove = undefined;
+        this._document.onmousemove = undefined;
-        document.onkeydown = undefined;
+        this._document.onkeydown = undefined;
-        window.removeEventListener('wheel', this._onUserActivity,
+        this._window.removeEventListener('wheel', this._onUserActivity,
                                    { passive: true, capture: true });
 
-        document.removeEventListener("visibilitychange", this._onPageVisibilityChanged);
+        this._document.removeEventListener("visibilitychange", this._onPageVisibilityChanged);
-        document.removeEventListener("blur", this._onDocumentBlurred);
+        this._window.removeEventListener("blur", this._onWindowBlurred);
-        document.removeEventListener("focus", this._onUserActivity);
+        this._window.removeEventListener("focus", this._onUserActivity);
     }
 
     /**
-     * Return true if there has been user activity very recently
+     * Return true if the user is currently 'active'
-     * (ie. within a few seconds)
+     * A user is 'active' while they are interacting with the app and for a very short (<1s)
-     * @returns {boolean} true if user is currently/very recently active
+     * time after that. This is intended to give a strong indication that the app has the
+     * user's attention at any given moment.
+     * @returns {boolean} true if user is currently 'active'
      */
     userCurrentlyActive() {
-        return this._activityTimeout.isRunning();
+        return this._activeTimeout.isRunning();
+    }
+
+    /**
+     * Return true if the user is currently 'active' or 'passive'
+     * A user is 'passive' for a longer period of time (~2 mins) after they have been 'active' and
+     * while the app still has the focus. This is intended to indicate when the app may still have
+     * the user's attention (or they may have gone to make tea and left the window focused).
+     * @returns {boolean} true if user is currently 'active' or 'passive'
+     */
+    userCurrentlyPassive() {
+        return this._passiveTimeout.isRunning();
     }
 
     _onPageVisibilityChanged(e) {
-        if (document.visibilityState === "hidden") {
+        if (this._document.visibilityState === "hidden") {
-            this._activityTimeout.abort();
+            this._activeTimeout.abort();
+            this._passiveTimeout.abort();
         } else {
             this._onUserActivity(e);
         }
     }
 
     _onWindowBlurred() {
-        this._activityTimeout.abort();
+        this._activeTimeout.abort();
+        this._passiveTimeout.abort();
     }
 
-    async _onUserActivity(event) {
+    _onUserActivity(event) {
+        // ignore anything if the window isn't focused
+        if (!this._document.hasFocus()) return;
+
         if (event.screenX && event.type === "mousemove") {
             if (event.screenX === this.lastScreenX && event.screenY === this.lastScreenY) {
                 // mouse hasn't actually moved
@@ -132,19 +187,29 @@ class UserActivity {
         }
 
         dis.dispatch({action: 'user_activity'});
-        if (!this._activityTimeout.isRunning()) {
+        if (!this._activeTimeout.isRunning()) {
-            this._activityTimeout.start();
+            this._activeTimeout.start();
             dis.dispatch({action: 'user_activity_start'});
-            this._attachedTimers.forEach((t) => t.start());
+
-            try {
+            this._runTimersUntilTimeout(this._attachedTimersActive, this._activeTimeout);
-                await this._activityTimeout.finished();
-            } catch (_e) { /* aborted */ }
-            this._attachedTimers.forEach((t) => t.abort());
         } else {
-            this._activityTimeout.restart();
+            this._activeTimeout.restart();
+        }
+
+        if (!this._passiveTimeout.isRunning()) {
+            this._passiveTimeout.start();
+
+            this._runTimersUntilTimeout(this._attachedTimersPassive, this._passiveTimeout);
+        } else {
+            this._passiveTimeout.restart();
         }
     }
+
+    async _runTimersUntilTimeout(attachedTimers, timeout) {
+        attachedTimers.forEach((t) => t.start());
+        try {
+            await timeout.finished();
+        } catch (_e) { /* aborted */ }
+        attachedTimers.forEach((t) => t.abort());
+    }
 }
-
-
-module.exports = new UserActivity();

src/components/structures/TimelinePanel.js

@@ -451,12 +451,12 @@ var TimelinePanel = React.createClass({
                 //
                 // We ignore events we have sent ourselves; we don't want to see the
                 // read-marker when a remote echo of an event we have just sent takes
-                // more than the timeout on userCurrentlyActive.
+                // more than the timeout on userCurrentlyPassive.
                 //
                 const myUserId = MatrixClientPeg.get().credentials.userId;
                 const sender = ev.sender ? ev.sender.userId : null;
                 var callRMUpdated = false;
-                if (sender != myUserId && !UserActivity.userCurrentlyActive()) {
+                if (sender != myUserId && !UserActivity.sharedInstance().userCurrentlyPassive()) {
                     updatedState.readMarkerVisible = true;
                 } else if (lastEv && this.getReadMarkerPosition() === 0) {
                     // we know we're stuckAtBottom, so we can advance the RM
@@ -562,7 +562,7 @@ var TimelinePanel = React.createClass({
         this._readMarkerActivityTimer = new Timer(initialTimeout);
 
         while (this._readMarkerActivityTimer) { //unset on unmount
-            UserActivity.timeWhileActive(this._readMarkerActivityTimer);
+            UserActivity.sharedInstance().timeWhilePassive(this._readMarkerActivityTimer);
             try {
                 await this._readMarkerActivityTimer.finished();
             } catch(e) { continue; /* aborted */ }
@@ -574,7 +574,7 @@ var TimelinePanel = React.createClass({
     updateReadReceiptOnUserActivity: async function() {
         this._readReceiptActivityTimer = new Timer(READ_RECEIPT_INTERVAL_MS);
         while (this._readReceiptActivityTimer) { //unset on unmount
-            UserActivity.timeWhileActive(this._readReceiptActivityTimer);
+            UserActivity.sharedInstance().timeWhileActive(this._readReceiptActivityTimer);
             try {
                 await this._readReceiptActivityTimer.finished();
             } catch(e) { continue; /* aborted */ }

test/UserActivity-test.js

@@ -0,0 +1,134 @@
+/*
+Copyright 2019 New Vector Ltd
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import expect from 'expect';
+import lolex from 'lolex';
+import jest from 'jest-mock';
+import EventEmitter from 'events';
+import UserActivity from '../src/UserActivity';
+
+class FakeDomEventEmitter extends EventEmitter {
+    addEventListener(what, l) {
+        this.on(what, l);
+    }
+
+    removeEventListener(what, l) {
+        this.removeListener(what, l);
+    }
+};
+
+describe('UserActivity', function() {
+    let fakeWindow;
+    let fakeDocument;
+    let userActivity;
+    let clock;
+
+    beforeEach(function() {
+        fakeWindow = new FakeDomEventEmitter(),
+        fakeDocument = new FakeDomEventEmitter(),
+        userActivity = new UserActivity(fakeWindow, fakeDocument);
+        userActivity.start();
+        clock = lolex.install();
+    });
+
+    afterEach(function() {
+        userActivity.stop();
+        userActivity = null;
+        clock.uninstall();
+        clock = null;
+    });
+
+    it('should return the same shared instance', function() {
+        expect(UserActivity.sharedInstance()).toBe(UserActivity.sharedInstance());
+    });
+
+    it('should consider user inactive if no activity', function() {
+        expect(userActivity.userCurrentlyActive()).toBe(false);
+    });
+
+    it('should consider user not passive if no activity', function() {
+        expect(userActivity.userCurrentlyPassive()).toBe(false);
+    });
+
+    it('should not consider user active after activity if no window focus', function() {
+        fakeDocument.hasFocus = jest.fn().mockReturnValue(false);
+
+        userActivity._onUserActivity({});
+        expect(userActivity.userCurrentlyActive()).toBe(false);
+        expect(userActivity.userCurrentlyPassive()).toBe(false);
+    });
+
+    it('should consider user active shortly after activity', function() {
+        fakeDocument.hasFocus = jest.fn().mockReturnValue(true);
+
+        userActivity._onUserActivity({});
+        expect(userActivity.userCurrentlyActive()).toBe(true);
+        expect(userActivity.userCurrentlyPassive()).toBe(true);
+        clock.tick(200);
+        expect(userActivity.userCurrentlyActive()).toBe(true);
+        expect(userActivity.userCurrentlyPassive()).toBe(true);
+    });
+
+    it('should consider user not active after 10s of no activity', function() {
+        fakeDocument.hasFocus = jest.fn().mockReturnValue(true);
+
+        userActivity._onUserActivity({});
+        clock.tick(10000);
+        expect(userActivity.userCurrentlyActive()).toBe(false);
+    });
+
+    it('should consider user passive after 10s of no activity', function() {
+        fakeDocument.hasFocus = jest.fn().mockReturnValue(true);
+
+        userActivity._onUserActivity({});
+        clock.tick(10000);
+        expect(userActivity.userCurrentlyPassive()).toBe(true);
+    });
+
+    it('should not consider user passive after 10s if window un-focused', function() {
+        fakeDocument.hasFocus = jest.fn().mockReturnValue(true);
+
+        userActivity._onUserActivity({});
+        clock.tick(10000);
+
+        fakeDocument.hasFocus = jest.fn().mockReturnValue(false);
+        fakeWindow.emit('blur', {});
+
+        expect(userActivity.userCurrentlyPassive()).toBe(false);
+    });
+
+    it('should not consider user passive after 3 mins', function() {
+        fakeDocument.hasFocus = jest.fn().mockReturnValue(true);
+
+        userActivity._onUserActivity({});
+        clock.tick(3 * 60 * 1000);
+
+        expect(userActivity.userCurrentlyPassive()).toBe(false);
+    });
+
+    it('should extend timer on activity', function() {
+        fakeDocument.hasFocus = jest.fn().mockReturnValue(true);
+
+        userActivity._onUserActivity({});
+        clock.tick(1 * 60 * 1000);
+        userActivity._onUserActivity({});
+        clock.tick(1 * 60 * 1000);
+        userActivity._onUserActivity({});
+        clock.tick(1 * 60 * 1000);
+
+        expect(userActivity.userCurrentlyPassive()).toBe(true);
+    });
+});