From c8312dd5ae0e94629bb6e69f420f3654ff42e94a Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Wed, 28 Mar 2018 10:25:28 +0100 Subject: [PATCH] Use a less fragile API to track page change performance --- src/Analytics.js | 23 +++------------- src/components/structures/MatrixChat.js | 35 ++++++++++++++++++++++--- 2 files changed, 35 insertions(+), 23 deletions(-) diff --git a/src/Analytics.js b/src/Analytics.js index ab47d30c1d..bb9f900145 100644 --- a/src/Analytics.js +++ b/src/Analytics.js @@ -148,24 +148,7 @@ class Analytics { return true; } - startPageChangeTimer() { - performance.clearMarks('riot_page_change_start'); - performance.mark('riot_page_change_start'); - } - - stopPageChangeTimer() { - performance.mark('riot_page_change_stop'); - performance.measure( - 'riot_page_change_delta', - 'riot_page_change_start', - 'riot_page_change_stop', - ); - - const measurement = performance.getEntriesByName('riot_page_change_delta').pop(); - this.generationTimeMs = measurement.duration; - } - - trackPageChange() { + trackPageChange(generationTimeMs) { if (this.disabled) return; if (this.firstPage) { // De-duplicate first page @@ -174,8 +157,8 @@ class Analytics { return; } this._paq.push(['setCustomUrl', getRedactedUrl()]); - if (typeof this.generationTimeMs === 'number') { - this._paq.push(['setGenerationTimeMs', this.generationTimeMs]); + if (typeof generationTimeMs === 'number') { + this._paq.push(['setGenerationTimeMs', generationTimeMs]); } this._paq.push(['trackPageView']); } diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index a898808141..2c38e3d26e 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -370,14 +370,14 @@ export default React.createClass({ componentWillUpdate: function(props, state) { if (this.shouldTrackPageChange(this.state, state)) { - Analytics.startPageChangeTimer(); + this.startPageChangeTimer(); } }, componentDidUpdate: function(prevProps, prevState) { if (this.shouldTrackPageChange(prevState, this.state)) { - Analytics.stopPageChangeTimer(); - Analytics.trackPageChange(); + const durationMs = this.stopPageChangeTimer(); + Analytics.trackPageChange(durationMs); } if (this.focusComposer) { dis.dispatch({action: 'focus_composer'}); @@ -385,6 +385,35 @@ export default React.createClass({ } }, + startPageChangeTimer() { + // This shouldn't happen because componentWillUpdate and componentDidUpdate + // are used. + if (this._pageChanging) { + console.warn('MatrixChat.startPageChangeTimer: timer already started'); + return; + } + this._pageChanging = true; + performance.mark('riot_MatrixChat_page_change_start'); + }, + + stopPageChangeTimer() { + if (!this._pageChanging) { + console.warn('MatrixChat.stopPageChangeTimer: timer not started'); + return; + } + this._pageChanging = false; + performance.mark('riot_MatrixChat_page_change_stop'); + performance.measure( + 'riot_MatrixChat_page_change_delta', + 'riot_MatrixChat_page_change_start', + 'riot_MatrixChat_page_change_stop', + ); + performance.clearMarks('riot_MatrixChat_page_change_start'); + performance.clearMarks('riot_MatrixChat_page_change_stop'); + const measurement = performance.getEntriesByName('riot_MatrixChat_page_change_delta').pop(); + return measurement.duration; + }, + shouldTrackPageChange(prevState, state) { return prevState.currentRoomId !== state.currentRoomId || prevState.view !== state.view ||