Merge pull request #2770 from matrix-org/dbkr/fix_instant_rrs_pt2

Fix instantly sending RRs

src/Lifecycle.js

@@ -457,7 +457,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();
@@ -503,7 +503,7 @@ function _clearStorage() {
  */
 export function stopMatrixClient() {
     Notifier.stop();
-    UserActivity.stop();
+    UserActivity.sharedInstance().stop();
     Presence.stop();
     ActiveWidgetStore.stop();
     if (DMRoomMap.shared()) DMRoomMap.shared().stop();

src/UserActivity.js

@@ -18,21 +18,34 @@ limitations under the License.
 import dis from './dispatcher';
 import Timer from './utils/Timer';
 
-// important this is larger than the timeouts of timers
+// important these are larger than the timeouts of timers
-// used with UserActivity.timeWhileActive,
+// used with UserActivity.timeWhileActive*,
-// such as READ_MARKER_INVIEW_THRESHOLD_MS,
+// such as READ_MARKER_INVIEW_THRESHOLD_MS (timeWhileActiveRecently),
-// READ_MARKER_OUTOFVIEW_THRESHOLD_MS,
+// READ_MARKER_OUTOFVIEW_THRESHOLD_MS (timeWhileActiveRecently),
-// READ_RECEIPT_INTERVAL_MS in TimelinePanel
+// READ_RECEIPT_INTERVAL_MS (timeWhileActiveNow) in TimelinePanel
-const CURRENTLY_ACTIVE_THRESHOLD_MS = 2 * 60 * 1000;
+
+// 'Under a few seconds'. Must be less than 'RECENTLY_ACTIVE_THRESHOLD_MS'
+const CURRENTLY_ACTIVE_THRESHOLD_MS = 700;
+
+// 'Under a few minutes'.
+const RECENTLY_ACTIVE_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': 'active now' and 'active recently'
+ * see doc on the userActive* 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._attachedActiveNowTimers = [];
+        this._attachedActiveRecentlyTimers = [];
+        this._activeNowTimeout = new Timer(CURRENTLY_ACTIVE_THRESHOLD_MS);
+        this._activeRecentlyTimeout = new Timer(RECENTLY_ACTIVE_THRESHOLD_MS);
         this._onUserActivity = this._onUserActivity.bind(this);
         this._onWindowBlurred = this._onWindowBlurred.bind(this);
         this._onPageVisibilityChanged = this._onPageVisibilityChanged.bind(this);
@@ -40,88 +53,138 @@ 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 now', aborting when the user is no longer
+     * considered currently active.
+     * See userActiveNow() 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) {
+    timeWhileActiveNow(timer) {
+        this._timeWhile(timer, this._attachedActiveNowTimers);
+        if (this.userActiveNow()) {
+            timer.start();
+        }
+    }
+
+    /**
+     * Runs the given timer while the user is 'active' now or recently,
+     * aborting when the user becomes inactive.
+     * See userActiveRecently() for what it means for a user to be 'active recently'.
+     * 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
+     */
+    timeWhileActiveRecently(timer) {
+        this._timeWhile(timer, this._attachedActiveRecentlyTimers);
+        if (this.userActiveRecently()) {
+            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.addEventListener('mousedown', this._onUserActivity);
-        document.onmousemove = this._onUserActivity;
+        this._document.addEventListener('mousemove', this._onUserActivity);
-        document.onkeydown = this._onUserActivity;
+        this._document.addEventListener('keydown', 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 });
+            passive: true, capture: true,
+        });
     }
 
     /**
      * Stop tracking user activity
      */
     stop() {
-        document.onmousedown = undefined;
+        this._document.removeEventListener('mousedown', this._onUserActivity);
-        document.onmousemove = undefined;
+        this._document.removeEventListener('mousemove', this._onUserActivity);
-        document.onkeydown = undefined;
+        this._document.removeEventListener('keydown', this._onUserActivity);
-        window.removeEventListener('wheel', this._onUserActivity,
+        this._window.removeEventListener('wheel', this._onUserActivity, {
-                                   { passive: true, capture: true });
+            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() {
+    userActiveNow() {
-        return this._activityTimeout.isRunning();
+        return this._activeNowTimeout.isRunning();
+    }
+
+    /**
+     * Return true if the user is currently active or has been recently
+     * A user is 'active recently' 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 has been active recently
+     */
+    userActiveRecently() {
+        return this._activeRecentlyTimeout.isRunning();
     }
 
     _onPageVisibilityChanged(e) {
-        if (document.visibilityState === "hidden") {
+        if (this._document.visibilityState === "hidden") {
-            this._activityTimeout.abort();
+            this._activeNowTimeout.abort();
+            this._activeRecentlyTimeout.abort();
         } else {
             this._onUserActivity(e);
         }
     }
 
     _onWindowBlurred() {
-        this._activityTimeout.abort();
+        this._activeNowTimeout.abort();
+        this._activeRecentlyTimeout.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 +195,29 @@ class UserActivity {
         }
 
         dis.dispatch({action: 'user_activity'});
-        if (!this._activityTimeout.isRunning()) {
+        if (!this._activeNowTimeout.isRunning()) {
-            this._activityTimeout.start();
+            this._activeNowTimeout.start();
             dis.dispatch({action: 'user_activity_start'});
-            this._attachedTimers.forEach((t) => t.start());
+
-            try {
+            this._runTimersUntilTimeout(this._attachedActiveNowTimers, this._activeNowTimeout);
-                await this._activityTimeout.finished();
-            } catch (_e) { /* aborted */ }
-            this._attachedTimers.forEach((t) => t.abort());
         } else {
-            this._activityTimeout.restart();
+            this._activeNowTimeout.restart();
         }
+
+        if (!this._activeRecentlyTimeout.isRunning()) {
+            this._activeRecentlyTimeout.start();
+
+            this._runTimersUntilTimeout(this._attachedActiveRecentlyTimers, this._activeRecentlyTimeout);
+        } else {
+            this._activeRecentlyTimeout.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

@@ -1,6 +1,7 @@
 /*
 Copyright 2016 OpenMarket Ltd
 Copyright 2017 Vector Creations Ltd
+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.
@@ -451,12 +452,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 userActiveRecently.
                 //
                 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().userActiveRecently()) {
                     updatedState.readMarkerVisible = true;
                 } else if (lastEv && this.getReadMarkerPosition() === 0) {
                     // we know we're stuckAtBottom, so we can advance the RM
@@ -562,7 +563,7 @@ var TimelinePanel = React.createClass({
         this._readMarkerActivityTimer = new Timer(initialTimeout);
 
         while (this._readMarkerActivityTimer) { //unset on unmount
-            UserActivity.timeWhileActive(this._readMarkerActivityTimer);
+            UserActivity.sharedInstance().timeWhileActiveRecently(this._readMarkerActivityTimer);
             try {
                 await this._readMarkerActivityTimer.finished();
             } catch(e) { continue; /* aborted */ }
@@ -574,7 +575,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().timeWhileActiveNow(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.userActiveNow()).toBe(false);
+    });
+
+    it('should consider user not active recently if no activity', function() {
+        expect(userActivity.userActiveRecently()).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.userActiveNow()).toBe(false);
+        expect(userActivity.userActiveRecently()).toBe(false);
+    });
+
+    it('should consider user active shortly after activity', function() {
+        fakeDocument.hasFocus = jest.fn().mockReturnValue(true);
+
+        userActivity._onUserActivity({});
+        expect(userActivity.userActiveNow()).toBe(true);
+        expect(userActivity.userActiveRecently()).toBe(true);
+        clock.tick(200);
+        expect(userActivity.userActiveNow()).toBe(true);
+        expect(userActivity.userActiveRecently()).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.userActiveNow()).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.userActiveRecently()).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.userActiveRecently()).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.userActiveRecently()).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.userActiveRecently()).toBe(true);
+    });
+});