[FIX] mail: more consistent async handling of scroll feature

task-2333535
task-2388676

closes odoo/odoo#64505

Signed-off-by: Alexandre Kühn (aku) <aku@odoo.com>

4 files changed, 189 insertions, 230 deletions

diff --git a/addons/mail/static/src/bugfix/bugfix.js b/addons/mail/static/src/bugfix/bugfix.js
index 1253e69da8e..162bf606db5 100644
--- a/addons/mail/static/src/bugfix/bugfix.js
+++ b/addons/mail/static/src/bugfix/bugfix.js
@@ -10,6 +10,44 @@ odoo.define('mail/static/src/bugfix/bugfix.js', function (require) {
 });
 
 // Should be moved to its own file in master.
+odoo.define('mail/static/src/component_hooks/use_rendered_values/use_rendered_values.js', function (require) {
+'use strict';
+
+const { Component } = owl;
+const { onMounted, onPatched } = owl.hooks;
+
+/**
+ * This hooks provides support for accessing the values returned by the given
+ * selector at the time of the last render. The values will be updated after
+ * every mount/patch.
+ *
+ * @param {function} selector function that will be executed at the time of the
+ *  render and of which the result will be stored for future reference.
+ * @returns {function} function to call to retrieve the last rendered values.
+ */
+function useRenderedValues(selector) {
+    const component = Component.current;
+    let renderedValues;
+    let patchedValues;
+
+    const __render = component.__render.bind(component);
+    component.__render = function () {
+        renderedValues = selector();
+        return __render(...arguments);
+    };
+    onMounted(onUpdate);
+    onPatched(onUpdate);
+    function onUpdate() {
+        patchedValues = renderedValues;
+    }
+    return () => patchedValues;
+}
+
+return useRenderedValues;
+
+});
+
+// Should be moved to its own file in master.
 odoo.define('mail/static/src/component_hooks/use_update/use_update.js', function (require) {
 'use strict';
 
diff --git a/addons/mail/static/src/components/message_list/message_list.js b/addons/mail/static/src/components/message_list/message_list.js
index cdb51b19aae..4f16fe62f54 100644
--- a/addons/mail/static/src/components/message_list/message_list.js
+++ b/addons/mail/static/src/components/message_list/message_list.js
@@ -5,6 +5,7 @@ const components = {
     Message: require('mail/static/src/components/message/message.js'),
 };
 const useRefs = require('mail/static/src/component_hooks/use_refs/use_refs.js');
+const useRenderedValues = require('mail/static/src/component_hooks/use_rendered_values/use_rendered_values.js');
 const useStore = require('mail/static/src/component_hooks/use_store/use_store.js');
 const useUpdate = require('mail/static/src/component_hooks/use_update/use_update.js');
 
@@ -37,17 +38,14 @@ class MessageList extends Component {
             },
         });
         this._getRefs = useRefs();
-        useUpdate({ func: () => this._update() });
         /**
-         * Determine whether the auto-scroll on load is active or not. This
-         * is useful to disable some times, such as when mounting message list
-         * in ASC order: the initial scroll position is at the top of the
-         * conversation, and most of the time the expected initial scroll
-         * position should be at the bottom of the thread. During this time,
-         * the programmatical scrolling should not trigger auto-load messages
-         * on scroll.
+         * States whether there was at least one programmatic scroll since the
+         * last scroll event was handled (which is particularly async due to
+         * throttled behavior).
+         * Useful to avoid loading more messages or to incorrectly disabling the
+         * auto-scroll feature when the scroll was not made by the user.
          */
-        this._isAutoLoadOnScrollActive = true;
+        this._isLastScrollProgrammatic = false;
         /**
          * Reference of the "load more" item. Useful to trigger load more
          * on scroll when it becomes visible.
@@ -58,6 +56,29 @@ class MessageList extends Component {
          */
         this._willPatchSnapshot = undefined;
         this._onScrollThrottled = _.throttle(this._onScrollThrottled.bind(this), 100);
+        /**
+         * State used by the component at the time of the render. Useful to
+         * properly handle async code.
+         */
+        this._lastRenderedValues = useRenderedValues(() => {
+            const threadView = this.threadView;
+            const thread = threadView && threadView.thread;
+            return {
+                componentHintList: threadView ? [...threadView.componentHintList] : [],
+                hasAutoScrollOnMessageReceived: threadView && threadView.hasAutoScrollOnMessageReceived,
+                hasScrollAdjust: this.props.hasScrollAdjust,
+                mainCache: thread && thread.mainCache,
+                order: this.props.order,
+                threadCache: threadView && threadView.threadCache,
+                threadCacheInitialScrollHeight: threadView && threadView.threadCacheInitialScrollHeight,
+                threadCacheInitialScrollPosition: threadView && threadView.threadCacheInitialScrollPosition,
+                threadView,
+                threadViewer: threadView && threadView.threadViewer,
+            };
+        });
+        // useUpdate must be defined after useRenderedValues to guarantee proper
+        // call order
+        useUpdate({ func: () => this._update() });
     }
 
     willPatch() {
@@ -103,37 +124,33 @@ class MessageList extends Component {
      * component is patched. This is acceptable when <ThreadView/> has a
      * fixed height, which is the case for the moment. task-2358066
      */
-    async adjustFromComponentHints() {
-        if (!this.threadView) {
-            return;
-        }
-        if (!this.el) {
-            return;
-        }
-        for (const hint of this.threadView.componentHintList) {
+    adjustFromComponentHints() {
+        const { componentHintList, threadView } = this._lastRenderedValues();
+        for (const hint of componentHintList) {
             switch (hint.type) {
                 case 'change-of-thread-cache':
-                    this._adjustFromChangeOfThreadCache(hint);
-                    break;
                 case 'home-menu-hidden':
-                    this._adjustFromHomeMenuHidden(hint);
-                    break;
                 case 'home-menu-shown':
-                    this._adjustFromHomeMenuShown(hint);
-                    break;
-                case 'messages-loaded':
-                    this.threadView.markComponentHintProcessed(hint);
+                    // thread just became visible, the goal is to restore its
+                    // saved position if it exists or scroll to the end
+                    this._adjustScrollFromModel();
                     break;
                 case 'message-received':
-                    this._adjustFromMessageReceived(hint);
+                case 'messages-loaded':
+                case 'new-messages-loaded':
+                    // messages have been added at the end, either scroll to the
+                    // end or keep the current position
+                    this._adjustScrollForExtraMessagesAtTheEnd();
                     break;
                 case 'more-messages-loaded':
-                    this._adjustFromMoreMessagesLoaded(hint);
-                    break;
-                case 'new-messages-loaded':
-                    this.threadView.markComponentHintProcessed(hint);
+                    // messages have been added at the start, keep the current
+                    // position
+                    this._adjustScrollForExtraMessagesAtTheStart();
                     break;
             }
+            if (threadView && threadView.exists()) {
+                threadView.markComponentHintProcessed(hint);
+            }
         }
         this._willPatchSnapshot = undefined;
     }
@@ -174,7 +191,8 @@ class MessageList extends Component {
      * @returns {mail/static/src/components/message/message.js|undefined}
      */
     get mostRecentMessageRef() {
-        if (this.props.order === 'desc') {
+        const { order } = this._lastRenderedValues();
+        if (order === 'desc') {
             return this.messageRefs[0];
         }
         const { length: l, [l - 1]: mostRecentMessageRef } = this.messageRefs;
@@ -213,6 +231,7 @@ class MessageList extends Component {
      * @returns {mail/static/src/components/message/message.js[]}
      */
     get messageRefs() {
+        const { order } = this._lastRenderedValues();
         const refs = this._getRefs();
         const ascOrderedMessageRefs = Object.entries(refs)
             .filter(([refId, ref]) => (
@@ -225,7 +244,7 @@ class MessageList extends Component {
             )
             .map(([refId, ref]) => ref)
             .sort((ref1, ref2) => (ref1.message.id < ref2.message.id ? -1 : 1));
-        if (this.props.order === 'desc') {
+        if (order === 'desc') {
             return ascOrderedMessageRefs.reverse();
         }
         return ascOrderedMessageRefs;
@@ -245,11 +264,12 @@ class MessageList extends Component {
     /**
      * @param {integer} value
      */
-    async setScrollTop(value) {
-        this._isAutoLoadOnScrollActive = false;
+    setScrollTop(value) {
+        if (this.el.scrollTop === value) {
+            return;
+        }
+        this._isLastScrollProgrammatic = true;
         this.el.scrollTop = value;
-        await new Promise(resolve => setTimeout(resolve, 0));
-        this._isAutoLoadOnScrollActive = true;
     }
 
     /**
@@ -316,158 +336,88 @@ class MessageList extends Component {
 
     /**
      * @private
-     * @param {Object} hint
      */
-    async _adjustFromChangeOfThreadCache(hint) {
-        const threadCache = this.threadView.threadCache;
-        if (!threadCache.isLoaded) {
+    _adjustScrollForExtraMessagesAtTheEnd() {
+        const {
+            hasAutoScrollOnMessageReceived,
+            hasScrollAdjust,
+            order,
+        } = this._lastRenderedValues();
+        if (!this.el || !hasScrollAdjust) {
             return;
         }
-        let isProcessed = false;
-        if (threadCache.messages.length > 0) {
-            if (this.threadView.threadCacheInitialScrollPosition !== undefined) {
-                if (this.props.hasScrollAdjust) {
-                    if (this.el.scrollHeight === this.threadView.threadCacheInitialScrollHeight) {
-                        this.el.scrollTop = this.threadView.threadCacheInitialScrollPosition;
-                        isProcessed = true;
-                    }
-                } else {
-                    isProcessed = true;
-                }
-            } else {
-                const lastMessage = threadCache.lastMessage;
-                if (this.messageRefFromId(lastMessage.id)) {
-                    if (this.props.hasScrollAdjust) {
-                        this._scrollToMostRecentMessage();
-                    }
-                    isProcessed = true;
-                }
+        if (!hasAutoScrollOnMessageReceived) {
+            if (order === 'desc' && this._willPatchSnapshot) {
+                const { scrollHeight, scrollTop } = this._willPatchSnapshot;
+                this.setScrollTop(this.el.scrollHeight - scrollHeight + scrollTop);
             }
-        } else {
-            isProcessed = true;
-        }
-        if (isProcessed) {
-            this.threadView.markComponentHintProcessed(hint);
-        }
-    }
-
-    /**
-     * @private
-     * @param {Object} hint
-     */
-    _adjustFromChatWindowUnfolded(hint) {
-        if (this._adjustScrollFromModel()) {
-            this.threadView.markComponentHintProcessed(hint);
-        }
-    }
-
-    /**
-     * @private
-     * @param {Object} hint
-     */
-    _adjustFromHomeMenuHidden(hint) {
-        if (this._adjustScrollFromModel()) {
-            this.threadView.markComponentHintProcessed(hint);
-        }
-    }
-
-    /**
-     * @private
-     * @param {Object} hint
-     */
-    _adjustFromHomeMenuShown(hint) {
-        if (this._adjustScrollFromModel()) {
-            this.threadView.markComponentHintProcessed(hint);
+            return;
         }
+        this._scrollToEnd();
     }
 
     /**
      * @private
-     * @param {Object} hint
      */
-    async _adjustFromMessageReceived(hint) {
-        const threadCache = this.threadView.threadCache;
-        if (!threadCache.isLoaded) {
-            return;
-        }
-        const { message } = hint.data;
-        if (!threadCache.messages.includes(message)) {
-            return;
-        }
-        if (!this.messageRefFromId(message.id)) {
-            return;
-        }
-        if (!this.props.hasScrollAdjust) {
-            this.threadView.markComponentHintProcessed(hint);
-            return;
-        }
-        if (!this.threadView.hasAutoScrollOnMessageReceived) {
-            this.threadView.markComponentHintProcessed(hint);
-            return;
-        }
+    _adjustScrollForExtraMessagesAtTheStart() {
+        const {
+            hasScrollAdjust,
+            order,
+        } = this._lastRenderedValues();
         if (
-            this.threadView.lastVisibleMessage &&
-            (message.id < this.threadView.lastVisibleMessage.id)
+            !this.el ||
+            !hasScrollAdjust ||
+            !this._willPatchSnapshot ||
+            order === 'desc'
         ) {
-            this.threadView.markComponentHintProcessed(hint);
-            return;
-        }
-        await this._scrollToMessage(message.id);
-        this.threadView.markComponentHintProcessed(hint);
-    }
-
-    /**
-     * @private
-     * @param {Object} hint
-     */
-    _adjustFromMoreMessagesLoaded(hint) {
-        if (!this._willPatchSnapshot) {
-            this.threadView.markComponentHintProcessed(hint);
             return;
         }
         const { scrollHeight, scrollTop } = this._willPatchSnapshot;
-        if (this.props.order === 'asc' && this.props.hasScrollAdjust) {
-            this.el.scrollTop = this.el.scrollHeight - scrollHeight + scrollTop;
-        }
-        this.threadView.markComponentHintProcessed(hint);
+        this.setScrollTop(this.el.scrollHeight - scrollHeight + scrollTop);
     }
 
     /**
      * @private
-     * @returns {boolean} whether the adjustment should be considered processed
      */
     _adjustScrollFromModel() {
+        const {
+            hasScrollAdjust,
+            threadCacheInitialScrollHeight,
+            threadCacheInitialScrollPosition,
+        } = this._lastRenderedValues();
+        if (!this.el || !hasScrollAdjust) {
+            return;
+        }
         if (
-            this.threadView.threadCacheInitialScrollPosition !== undefined &&
-            this.props.hasScrollAdjust
+            threadCacheInitialScrollPosition !== undefined &&
+            this.el.scrollHeight === threadCacheInitialScrollHeight
         ) {
-            if (this.el.scrollHeight === this.threadView.threadCacheInitialScrollHeight) {
-                this.el.scrollTop = this.threadView.threadCacheInitialScrollPosition;
-                return true;
-            } else {
-                return false;
-            }
+            this.setScrollTop(threadCacheInitialScrollPosition);
+            return;
         }
-        return true;
+        this._scrollToEnd();
+        return;
     }
 
     /**
      * @private
      */
     _checkMostRecentMessageIsVisible() {
-        if (!this.threadView) {
+        const {
+            mainCache,
+            threadCache,
+            threadView,
+        } = this._lastRenderedValues();
+        if (!threadView || !threadView.exists()) {
             return;
         }
-        const thread = this.threadView.thread;
-        const threadCache = this.threadView.threadCache;
         const lastMessageIsVisible =
             threadCache &&
-            threadCache.messages.length > 0 &&
             this.mostRecentMessageRef &&
-            threadCache === thread.mainCache &&
+            threadCache === mainCache &&
             this.mostRecentMessageRef.isPartiallyVisible();
         if (lastMessageIsVisible) {
-            this.threadView.handleVisibleMessage(this.mostRecentMessageRef.message);
+            threadView.handleVisibleMessage(this.mostRecentMessageRef.message);
         }
     }
 
@@ -490,55 +440,29 @@ class MessageList extends Component {
      * @private
      */
     _loadMore() {
-        this.threadView.threadCache.loadMoreMessages();
-    }
-
-    /**
-     * @private
-     * @returns {Promise}
-     */
-    async _scrollToMostRecentMessage() {
-        if (!this.mostRecentMessageRef) {
-            return;
-        }
-        this._isAutoLoadOnScrollActive = false;
-        await this.mostRecentMessageRef.scrollIntoView();
-        if (!this.el) {
-            this._isAutoLoadOnScrollActive = true;
+        const { threadCache } = this._lastRenderedValues();
+        if (!threadCache || !threadCache.exists()) {
             return;
         }
-        this.el.scrollTop = this.props.order === 'asc'
-            ? this.el.scrollTop + 15
-            : this.el.scrollTop - 15;
-        this._isAutoLoadOnScrollActive = true;
+        threadCache.loadMoreMessages();
     }
 
     /**
-     * @param {integer} messageId
+     * Scrolls to the end of the list.
+     *
+     * @private
      */
-    async _scrollToMessage(messageId) {
-        const messageRef = this.messageRefFromId(messageId);
-        if (!messageRef) {
-            return;
-        }
-        this._isAutoLoadOnScrollActive = false;
-        await messageRef.scrollIntoView();
-        if (!this.el) {
-            this._isAutoLoadOnScrollActive = true;
-            return;
-        }
-        this.el.scrollTop = this.props.order === 'asc'
-            ? this.el.scrollTop + 15
-            : this.el.scrollTop - 15;
-        this._isAutoLoadOnScrollActive = true;
+    _scrollToEnd() {
+        const { order } = this._lastRenderedValues();
+        this.setScrollTop(order === 'asc' ? this.el.scrollHeight - this.el.clientHeight : 0);
     }
 
     /**
      * @private
      */
     _update() {
-        this.adjustFromComponentHints();
         this._checkMostRecentMessageIsVisible();
+        this.adjustFromComponentHints();
     }
 
     //--------------------------------------------------------------------------
@@ -559,14 +483,6 @@ class MessageList extends Component {
      * @param {ScrollEvent} ev
      */
     onScroll(ev) {
-        if (!this.threadView) {
-            return;
-        }
-        // Clear pending hints to prevent them from potentially overriding the
-        // new scroll position.
-        for (const hint of this.threadView.componentHintList) {
-            this.threadView.markComponentHintProcessed(hint);
-        }
         this._onScrollThrottled(ev);
     }
 
@@ -575,35 +491,41 @@ class MessageList extends Component {
      * @param {ScrollEvent} ev
      */
     _onScrollThrottled(ev) {
+        const {
+            order,
+            threadCache,
+            threadView,
+            threadViewer,
+        } = this._lastRenderedValues();
         if (!this.el) {
             // could be unmounted in the meantime (due to throttled behavior)
             return;
         }
-        if (!this.threadView || !this.threadView.threadViewer) {
-            return;
-        }
         const scrollTop = this.el.scrollTop;
         this.env.messagingBus.trigger('o-component-message-list-scrolled', {
             scrollTop,
-            threadViewer: this.threadView.threadViewer,
+            threadViewer,
         });
-        // Margin to compensate for inaccurate scrolling to bottom.
-        const margin = 4;
-        // Automatically scroll to new received messages only when the list is
-        // currently fully scrolled.
-        const hasAutoScrollOnMessageReceived = (this.props.order === 'asc')
-            ? scrollTop >= this.el.scrollHeight - this.el.clientHeight - margin
-            : scrollTop <= margin;
-        this.threadView.update({ hasAutoScrollOnMessageReceived });
-        this.threadView.threadViewer.saveThreadCacheScrollHeightAsInitial(this.el.scrollHeight);
-        this.threadView.threadViewer.saveThreadCacheScrollPositionsAsInitial(scrollTop);
-        if (!this._isAutoLoadOnScrollActive) {
-            return;
-        }
-        if (this._isLoadMoreVisible()) {
+        if (!this._isLastScrollProgrammatic && threadView && threadView.exists()) {
+            // Margin to compensate for inaccurate scrolling to bottom and height
+            // flicker due height change of composer area.
+            const margin = 30;
+            // Automatically scroll to new received messages only when the list is
+            // currently fully scrolled.
+            const hasAutoScrollOnMessageReceived = (order === 'asc')
+                ? scrollTop >= this.el.scrollHeight - this.el.clientHeight - margin
+                : scrollTop <= margin;
+            threadView.update({ hasAutoScrollOnMessageReceived });
+        }
+        if (threadViewer && threadViewer.exists()) {
+            threadViewer.saveThreadCacheScrollHeightAsInitial(this.el.scrollHeight, threadCache);
+            threadViewer.saveThreadCacheScrollPositionsAsInitial(scrollTop, threadCache);
+        }
+        if (!this._isLastScrollProgrammatic && this._isLoadMoreVisible()) {
             this._loadMore();
         }
         this._checkMostRecentMessageIsVisible();
+        this._isLastScrollProgrammatic = false;
     }
 
 }
diff --git a/addons/mail/static/src/models/thread_view/thread_view.js b/addons/mail/static/src/models/thread_view/thread_view.js
index 5c65e19c0f8..bf4560998bd 100644
--- a/addons/mail/static/src/models/thread_view/thread_view.js
+++ b/addons/mail/static/src/models/thread_view/thread_view.js
@@ -45,17 +45,8 @@ function factory(dependencies) {
          * @param {Object} hint
          */
         markComponentHintProcessed(hint) {
-            let filterFun;
-            switch (hint.type) {
-                case 'message-received':
-                    filterFun = h => h.type !== hint.type && h.message !== hint.data.message;
-                    break;
-                default:
-                    filterFun = h => h.type !== hint.type;
-                    break;
-            }
             this.update({
-                componentHintList: this.componentHintList.filter(filterFun),
+                componentHintList: this.componentHintList.filter(h => h !== hint),
             });
             this.env.messagingBus.trigger('o-thread-view-hint-processed', {
                 hint,
@@ -143,6 +134,8 @@ function factory(dependencies) {
          * @private
          */
         _onThreadCacheChanged() {
+            // clear obsolete hints
+            this.update({ componentHintList: clear() });
             this.addComponentHint('change-of-thread-cache');
             if (this.threadCache) {
                 this.threadCache.update({ isCacheRefreshRequested: true });
@@ -236,7 +229,9 @@ function factory(dependencies) {
          * a new message. Detection of new message is done through the component
          * hint `message-received`.
          */
-        hasAutoScrollOnMessageReceived: attr(),
+        hasAutoScrollOnMessageReceived: attr({
+            default: true,
+        }),
         /**
          * Last message in the context of the currently displayed thread cache.
          */
diff --git a/addons/mail/static/src/models/thread_view/thread_viewer.js b/addons/mail/static/src/models/thread_view/thread_viewer.js
index 1e48e4ad805..c78022d4a53 100644
--- a/addons/mail/static/src/models/thread_view/thread_viewer.js
+++ b/addons/mail/static/src/models/thread_view/thread_viewer.js
@@ -14,9 +14,11 @@ function factory(dependencies) {
 
         /**
          * @param {integer} scrollHeight
+         * @param {mail.thread_cache} threadCache
          */
-        saveThreadCacheScrollHeightAsInitial(scrollHeight) {
-            if (!this.threadCache) {
+        saveThreadCacheScrollHeightAsInitial(scrollHeight, threadCache) {
+            threadCache = threadCache || this.threadCache;
+            if (!threadCache) {
                 return;
             }
             if (this.chatter) {
@@ -27,16 +29,18 @@ function factory(dependencies) {
             }
             this.update({
                 threadCacheInitialScrollHeights: Object.assign({}, this.threadCacheInitialScrollHeights, {
-                    [this.threadCache.localId]: scrollHeight,
+                    [threadCache.localId]: scrollHeight,
                 }),
             });
         }
 
         /**
          * @param {integer} scrollTop
+         * @param {mail.thread_cache} threadCache
          */
-        saveThreadCacheScrollPositionsAsInitial(scrollTop) {
-            if (!this.threadCache) {
+        saveThreadCacheScrollPositionsAsInitial(scrollTop, threadCache) {
+            threadCache = threadCache || this.threadCache;
+            if (!threadCache) {
                 return;
             }
             if (this.chatter) {
@@ -47,7 +51,7 @@ function factory(dependencies) {
             }
             this.update({
                 threadCacheInitialScrollPositions: Object.assign({}, this.threadCacheInitialScrollPositions, {
-                    [this.threadCache.localId]: scrollTop,
+                    [threadCache.localId]: scrollTop,
                 }),
             });
         }