[web] Proxy tunnelbroker messages to the active tab
ClosedPublic
Actions

Authored by michal on Feb 20 2024, 7:21 AM.

Details

Reviewers

kamil
inka
bartek

Commits

rCOMM21231f38fb9e: [web] Proxy tunnelbroker messages to the active tab

Summary

ENG-6670 : Allow non-primary tabs to send tunnelbroker messages

After previous diff, only one tab at a time can have a tunnelbroker connection. While that isn't a problem if user is just switching between tabs, user can have multiple tabs open at the same time. This means they can interact with both of them, but only one will be able to do tunnelbroker-related operations. Because of this we need to introduce a way for the tabs without the tunnelbroker connection to send the messages to the tab with the connection. This tab will then send the messages to the tunnelbroker. It also needs to notify the other tabs when the message upload completed.

Tab interaction can be implemented using the BroadcastChannelAPI. Each tab connects to a broadcast (differentiated by name) and is able to send and listen to messages.

We don't need to do that for message handling (only message sending) because it's the whole point of this stack -> that only one tab at a time will handle all tunnelbroker messages.

Depends on D11118

Test Plan

Connect to the tunnelbroker from webapp with multiple tabs and a phone.
Send a tunnelbroker message to the phone, from a tab with the tunnelbroker connection, and check if it's received
Send a tunnelbroker message from a tab without a tunnelbroker connection
Switch which tab has the tunnelbroker connection, and which one just send them to the other tab
Repeat tests

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

michal created this revision.Feb 20 2024, 7:21 AM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptFeb 20 2024, 7:21 AM

Harbormaster completed remote builds in B27007: Diff 37368.Feb 20 2024, 7:36 AM

michal requested review of this revision.Feb 20 2024, 7:36 AM

michal added a child revision: D11132: [web] Tab sync redux actions.Feb 21 2024, 6:53 AM

Rebase

Harbormaster completed remote builds in B27202: Diff 37651.Feb 27 2024, 6:18 AM

kamil accepted this revision.Mar 4 2024, 6:12 AM

kamil added inline comments.

lib/tunnelbroker/secondary-tunnelbroker-connection.js
3 ↗	(On Diff #37651)
web/app.react.js
527–528 ↗	(On Diff #37651)	names here looks inconsistent

This revision is now accepted and ready to land.Mar 4 2024, 6:12 AM

Rebase, fix path

web/app.react.js
527–528 ↗	(On Diff #37651)	I have decided to go with this naming because: `SecondaryTunnelbrokerConnection` is defined in lib and so it's a general non platform-specific name But on web when we implement a concrete implementation of it, I think it makes sense to name it something more understandable so `useOtherTabsTunnelbrokerConnection`

Harbormaster completed remote builds in B27299: Diff 37776.Mar 4 2024, 7:08 AM

Closed by commit rCOMM21231f38fb9e: [web] Proxy tunnelbroker messages to the active tab (authored by Michal Gniadek <michal.gniadek@swmansion.com>). · Explain WhyMar 4 2024, 7:18 AM

This revision was automatically updated to reflect the committed changes.

Michal Gniadek <michal.gniadek@swmansion.com> added a commit: rCOMM21231f38fb9e: [web] Proxy tunnelbroker messages to the active tab.

ashoat added inline comments.Mar 4 2024, 5:11 PM

lib/tunnelbroker/secondary-tunnelbroker-connection.js
7–19		Sorry I missed this earlier, but it looks like these types are not read-only either I shouldn't have to go through each of your diffs and call this out for every single new Flow type you create... can you submit a follow-up diff to make these read-only, and make sure that going forward, you consider read-only-ness in all new types that you create?
7 ↗	(On Diff #37368)	Should this be read-only?
7 ↗	(On Diff #37368)	Ah looks like I had queued up a comment previously, but didn't submit it accidentally...