Page MenuHomePhabricator

[web] Proxy tunnelbroker messages to the active tab
ClosedPublic

Authored by michal on Feb 20 2024, 7:21 AM.
Tags
None
Referenced Files
F3358119: D11123.diff
Sun, Nov 24, 3:30 AM
Unknown Object (File)
Thu, Nov 21, 6:20 AM
Unknown Object (File)
Thu, Nov 21, 5:38 AM
Unknown Object (File)
Wed, Nov 20, 3:24 PM
Unknown Object (File)
Tue, Nov 19, 7:06 PM
Unknown Object (File)
Tue, Nov 19, 6:19 PM
Unknown Object (File)
Mon, Nov 18, 8:53 PM
Unknown Object (File)
Mon, Nov 18, 7:00 PM
Subscribers

Details

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
No Lint Coverage
Unit
No Test Coverage

Event Timeline

kamil added inline comments.
lib/tunnelbroker/secondary-tunnelbroker-connection.js
3
web/app.react.js
527–528

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

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
lib/tunnelbroker/secondary-tunnelbroker-connection.js
7–19 ↗(On Diff #37780)

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...