Add a communication channel for forcing reloads across tab sessions.

Review Request #13048 — Created May 16, 2023 and submitted

Information

Review Board
release-6.x

Reviewers

The new behavior for the unified banner means that individual tabs fetch
and keep track of pending review/draft IDs more eagerly than before. The
result of this is that it was easier to hit bugs when performing actions
with a review request open in multiple tabs.

This change adds a small utility that makes use of the
BroadcastChannel API to coordinate across multiple tabs/windows within
a browser session. Right now the only message that we send is "reload",
which asks other tabs to reload their contents. This is sent when
something is either published or discarded, which invalidates draft IDs.

The broadcast channel is origin-wide, so each PageView can implement a
method to return a data blob that's used for filtering whether or not to
reload. The default implementation is empty, and ReviewablePageView
will filter based on the review request ID (so any open tabs related to
that review request will be triggered).

  • Ran js-tests.
  • Published review requests, reviews, and replies from one tab, and saw
    other tabs pop up the alert and reload.
  • Discarded drafts from the unified banner and from the review dialog,
    and saw other tabs pop up the alert and reload.
Summary ID
Add a communication channel for forcing reloads across tab sessions.
The new behavior for the unified banner means that individual tabs fetch and keep track of pending review/draft IDs more eagerly than before. The result of this is that it was easier to hit bugs when performing actions with a review request open in multiple tabs. This change adds a small utility that makes use of the `BroadcastChannel` API to coordinate across multiple tabs/windows within a browser session. Right now the only message that we send is "reload", which asks other tabs to reload their contents. This is sent when something is either published or discarded, which invalidates draft IDs. The broadcast channel is origin-wide, so each `PageView` can implement a method to return a data blob that's used for filtering whether or not to reload. The default implementation is empty, and `ReviewablePageView` will filter based on the review request ID (so any open tabs related to that review request will be triggered). Testing Done: - Ran js-tests. - Published review requests, reviews, and replies from one tab, and saw other tabs pop up the alert and reload. - Discarded drafts from the unified banner and from the review dialog, and saw other tabs pop up the alert and reload.
1bbc4c4e7e9ba3cdd2e68c13c93a619834cb6771
Description From Last Updated

We're going to need tests for this, which will be especially important for any filtering logic and any future use …

chipx86chipx86

I also want to discuss naming. Eventually, we'll want an open comm channel to the Review Board server as well. …

chipx86chipx86

Can you add a Version Added?

chipx86chipx86

This is missing docs. Should also have Version Added, which I've unfortunately left out of my other TS reviews.

chipx86chipx86

Missing Version Added.

chipx86chipx86

Should document this.

maubinmaubin

Should document this.

maubinmaubin

I think you could get rid of this and replace it with some info about how we're going to trigger …

maubinmaubin
chipx86
  1. So I think BroadcastChannel is origin-wide, so a reload is going to end up applying to every other tab regardless of review request. We'd need to bake in some identifying information into the event dispatch and listening to filter these events.

  2. Show all issues

    We're going to need tests for this, which will be especially important for any filtering logic and any future use of the broadcast channel.

  3. Show all issues

    I also want to discuss naming. Eventually, we'll want an open comm channel to the Review Board server as well. Is this going to be designed with the intent of being used for that as well? Or should we name this something more specific?

    1. Perhaps ClientCommChannel?

  4. Show all issues

    Can you add a Version Added?

  5. Show all issues

    This is missing docs. Should also have Version Added, which I've unfortunately left out of my other TS reviews.

  6. Show all issues

    Missing Version Added.

  7. 
      
david
maubin
  1. 
      
  2. Show all issues

    Should document this.

  3. Show all issues

    Should document this.

  4. Show all issues

    I think you could get rid of this and replace it with some info about how we're going to trigger tabs related to the review request.

  5. 
      
david
maubin
  1. Ship It!
  2. 
      
david
Review request changed
Status:
Completed
Change Summary:
Pushed to release-6.x (98d6ddc)