Skip to content

Commit f7991e7

Browse files
committed
fix: race, only one banner should be on screen at a time even in rerender
1 parent fd719c9 commit f7991e7

File tree

2 files changed

+60
-4
lines changed

2 files changed

+60
-4
lines changed

src/extensionsIntegrated/InAppNotifications/banner.js

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
*
2020
*/
2121

22-
/*global Phoenix*/
22+
/*global*/
2323
/**
2424
* module for displaying in-app banner notifications
2525
*
@@ -31,10 +31,14 @@ define(function (require, exports, module) {
3131
ExtensionUtils = require("utils/ExtensionUtils"),
3232
Metrics = require("utils/Metrics"),
3333
semver = require("thirdparty/semver.browser"),
34-
NotificationBarHtml = require("text!./htmlContent/notificationContainer.html");
34+
NotificationBarHtml = require("text!./htmlContent/notificationContainer.html"),
35+
ExtensionInterface = require("utils/ExtensionInterface");
3536

3637
ExtensionUtils.loadStyleSheet(module, "styles/styles.css");
3738

39+
const IN_APP_NOTIFICATION_INTERFACE = "Extn.Phoenix.inAppNotification";
40+
ExtensionInterface.registerExtensionInterface(IN_APP_NOTIFICATION_INTERFACE, exports);
41+
3842
let latestBannerJSON;
3943
let customFilterCallback;
4044

@@ -193,13 +197,22 @@ define(function (require, exports, module) {
193197
});
194198
}
195199

200+
let reRenderInProgress = Promise.resolve();
201+
196202
/**
197203
* Re-renders notifications using the latest cached banner JSON
204+
* Ensures renders are strictly serialized
198205
*/
199206
function reRenderNotifications() {
200-
if(latestBannerJSON) {
201-
_renderNotifications(latestBannerJSON);
207+
if (!latestBannerJSON) {
208+
return Promise.resolve();
202209
}
210+
211+
reRenderInProgress = reRenderInProgress
212+
.catch(() => {}) // prevent lock break on error
213+
.then(() => _renderNotifications(latestBannerJSON));
214+
215+
return reRenderInProgress;
203216
}
204217

205218

test/spec/Extn-InAppNotifications-integ-test.js

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,5 +330,48 @@ define(function (require, exports, module) {
330330
banner.registerCustomFilter(null);
331331
banner.cleanNotificationBanner();
332332
});
333+
334+
it("Should serialize multiple concurrent reRenderNotifications calls", async function () {
335+
banner.cleanNotificationBanner();
336+
337+
const {notification} = getRandomNotification("all", true);
338+
let renderCount = 0;
339+
340+
// Set cache
341+
banner._setBannerCache(notification);
342+
343+
// Register filter to track render calls
344+
banner.registerCustomFilter(async () => {
345+
renderCount++;
346+
return true;
347+
});
348+
349+
// Make 3 concurrent calls
350+
const promise1 = banner.reRenderNotifications();
351+
const promise2 = banner.reRenderNotifications();
352+
const promise3 = banner.reRenderNotifications();
353+
354+
// First render: wait for banner, close it, wait for promise to resolve
355+
await _waitForBannerShown();
356+
expect(renderCount).toEqual(1);
357+
testWindow.$('.close-icon').click();
358+
await promise1;
359+
360+
// Second render: wait for banner, close it, wait for promise to resolve
361+
await _waitForBannerShown();
362+
expect(renderCount).toEqual(2);
363+
testWindow.$('.close-icon').click();
364+
await promise2;
365+
366+
// Third render: wait for banner, close it, wait for promise to resolve
367+
await _waitForBannerShown();
368+
expect(renderCount).toEqual(3);
369+
testWindow.$('.close-icon').click();
370+
await promise3;
371+
372+
// Cleanup
373+
banner.registerCustomFilter(null);
374+
banner.cleanNotificationBanner();
375+
});
333376
});
334377
});

0 commit comments

Comments
 (0)