1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-18 12:52:42 +01:00

Probably improve behavior around duplicate notifications

Summary:
We're sometimes getting duplicate notifications right now. I think this is because multiple windows are racing and becoming leaders.

Clean this up a little:

  - Fix the `timeoout` typo.
  - Only try to usurp once.
  - Use different usurp and expire delays, so we don't fire them at the exact same time.

Not sure if this'll work or not but it should theoretically be a little cleaner.

Test Plan:
  - Quit Safari, reopened Safari, still saw a fast reconnect to the notification server (this is the goal of usurping).
  - Did normal notification stuff like opening a chat in two windows, got notifications.
  - Hard to reproduce the race for sure, but this at least fixes the outright `timeoout` bug.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D15806
This commit is contained in:
epriestley 2016-04-26 16:27:31 -07:00
parent 467c4e84e5
commit 58b55c2fa6
2 changed files with 22 additions and 15 deletions

View file

@ -8,7 +8,7 @@
return array(
'names' => array(
'core.pkg.css' => '04a95108',
'core.pkg.js' => '37344f3c',
'core.pkg.js' => '6972d365',
'darkconsole.pkg.js' => 'e7393ebb',
'differential.pkg.css' => '7ba78475',
'differential.pkg.js' => 'd0cd0df6',
@ -232,7 +232,7 @@ return array(
'rsrc/externals/javelin/lib/DOM.js' => '805b806a',
'rsrc/externals/javelin/lib/History.js' => 'd4505101',
'rsrc/externals/javelin/lib/JSON.js' => '69adf288',
'rsrc/externals/javelin/lib/Leader.js' => 'b4ba945c',
'rsrc/externals/javelin/lib/Leader.js' => 'fea0eb47',
'rsrc/externals/javelin/lib/Mask.js' => '8a41885b',
'rsrc/externals/javelin/lib/Quicksand.js' => '6b8ef10b',
'rsrc/externals/javelin/lib/Request.js' => '94b750d2',
@ -704,7 +704,7 @@ return array(
'javelin-history' => 'd4505101',
'javelin-install' => '05270951',
'javelin-json' => '69adf288',
'javelin-leader' => 'b4ba945c',
'javelin-leader' => 'fea0eb47',
'javelin-magical-init' => '3010e992',
'javelin-mask' => '8a41885b',
'javelin-quicksand' => '6b8ef10b',
@ -1772,9 +1772,6 @@ return array(
'javelin-typeahead-preloaded-source',
'javelin-util',
),
'b4ba945c' => array(
'javelin-install',
),
'b59e1e96' => array(
'javelin-behavior',
'javelin-stratcom',
@ -2176,6 +2173,9 @@ return array(
'javelin-view-visitor',
'javelin-util',
),
'fea0eb47' => array(
'javelin-install',
),
),
'packages' => array(
'core.pkg.css' => array(

View file

@ -118,6 +118,10 @@ JX.install('Leader', {
// Read the current leadership lease.
var lease = self._read();
// Stagger these delays so that they are unlikely to race one another.
var expire_delay = 50;
var usurp_delay = 75;
// If the lease is good, we're all set.
var now = +new Date();
if (lease.until > now) {
@ -135,15 +139,17 @@ JX.install('Leader', {
} else {
// Set a callback to try to become the leader shortly after the
// current lease expires. This lets us recover from cases where the
// leader goes missing quickly.
if (self._timeoout) {
window.clearTimeout(self._timeout);
self._timeout = null;
// current lease expires. This lets us quickly recover from cases
// where the leader goes missing.
// In particular, this can happen in Safari if you close windows or
// quit the browser instead of browsing away: the "pagehide" event
// does not fire when the leader is simply destroyed, so it does not
// evict itself from the throne of power.
if (!self._timeout) {
var usurp_at = (lease.until - now) + usurp_delay;
self._timeout = window.setTimeout(self._usurp, usurp_at);
}
self._timeout = window.setTimeout(
self._usurp,
(lease.until - now) + 50);
follower_callback();
}
@ -174,7 +180,7 @@ JX.install('Leader', {
window.setTimeout(
JX.bind(null, self._callIf, leader_callback, follower_callback),
50);
expire_delay);
},
@ -306,6 +312,7 @@ JX.install('Leader', {
_usurp: function() {
var self = JX.Leader;
self.call(JX.bag);
self._timeout = null;
},