From 8828e96b87b21604d11902d2563927029d620cea Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Wed, 11 Oct 2017 20:31:32 -0700 Subject: [PATCH] presence: Avoid checking activity when reloading. We sometimes get blueslip errors from browsers that are clearly still attempting to reload long after they should have. These browsers can produce a lot of unnecessary presence update exceptions. To solve that, we start checking reload_in_progress in the presence code path. While we're at it, we also add some blueslip logging for the reload code path, in case it becomes useful when debugging future issues. --- frontend_tests/node_tests/activity.js | 14 ++++++++++++++ static/js/activity.js | 5 ++++- static/js/reload.js | 6 +++++- static/js/server_events.js | 1 + 4 files changed, 24 insertions(+), 2 deletions(-) diff --git a/frontend_tests/node_tests/activity.js b/frontend_tests/node_tests/activity.js index cc450ff7e8..64b69b88b6 100644 --- a/frontend_tests/node_tests/activity.js +++ b/frontend_tests/node_tests/activity.js @@ -39,6 +39,13 @@ zrequire('presence'); zrequire('people'); zrequire('activity'); +set_global('blueslip', { + log: function () {}, +}); + +set_global('reload', { + is_in_progress: function () {return false;}, +}); set_global('resize', { resize_page_components: function () {}, }); @@ -706,4 +713,11 @@ $('.user-list-filter').is = function (sel) { assert($('#zephyr-mirror-error').hasClass('show')); assert(!activity.new_user_input); assert(!activity.has_focus); + + // Now execute the reload-in-progress code path + reload.is_in_progress = function () { + return true; + }; + activity.initialize(); + }()); diff --git a/static/js/activity.js b/static/js/activity.js index 82a9aba656..bdee60177f 100644 --- a/static/js/activity.js +++ b/static/js/activity.js @@ -399,8 +399,11 @@ exports.update_huddles = function () { exports.update_scrollbar.group_pms(); }; - function focus_ping(want_redraw) { + if (reload.is_in_progress()) { + blueslip.log("Skipping querying presence because reload in progress"); + return; + } channel.post({ url: '/json/users/me/presence', data: {status: (exports.has_focus) ? exports.ACTIVE : exports.IDLE, diff --git a/static/js/reload.js b/static/js/reload.js index a32e981dc0..9d194293a0 100644 --- a/static/js/reload.js +++ b/static/js/reload.js @@ -175,7 +175,10 @@ exports.initialize = function reload__initialize() { }; function do_reload_app(send_after_reload, save_pointer, save_narrow, save_compose, message) { - if (reload_in_progress) { return; } + if (reload_in_progress) { + blueslip.log("do_reload_app: Doing nothing since reload_in_progress"); + return; + } // TODO: we should completely disable the UI here if (save_pointer || save_narrow || save_compose) { @@ -305,6 +308,7 @@ window.addEventListener('beforeunload', function () { // When that happens we reload the page to correct the problem. If this // happens before the navigation is complete the user is kept captive at // zulip. + blueslip.log("Setting reload_in_progress in beforeunload handler"); reload_in_progress = true; }); diff --git a/static/js/server_events.js b/static/js/server_events.js index 358d4adb76..2d0be4b07e 100644 --- a/static/js/server_events.js +++ b/static/js/server_events.js @@ -270,6 +270,7 @@ exports.cleanup_event_queue = function cleanup_event_queue() { if (page_params.event_queue_expired === true) { return; } + blueslip.log("Cleaning up our event queue"); // Set expired because in a reload we may be called twice. page_params.event_queue_expired = true; channel.del({