• Robert Knight's avatar
    Combine Bridge and PortRPC classes · bb510f43
    Robert Knight authored
    Reduce the number of layers involved in inter-frame communication by combining
    the Bridge and PortRPC classes. PortRPC still only supports sending requests to
    a single port, but now has an API that is more like the Bridge class, where
    construction of the class, registering of RPC method handlers and connecting to
    a MessagePort are separate steps. This separation is convenient because it
    allows the RPC client to be set up before the MessagePort retrieval process,
    which is async in non-host frames, has completed.
    
    Existing uses of Bridge have been replaced either with a single PortRPC
    instance, for 1:1 connections between frames, or an array of PortRPC
    instances for 1:N connections between the sidebar/host and guest frames.
    Migrating to an array for 1:N connections paves the way to sending certain
    requests only to specific guests, whereas they are currently sent to _all_
    guests.
    
    Bridge's `call` method had logic for gathering results from multiple guests,
    timing out and disconnecting clients in the event of an error.  This logic was
    inappropriate in various ways and has not been re-implemented in PortRPC.
    
     - The timeout value was hardcoded at 1000ms for all calls. This may be too
       short depending on what the call does or what code is executing in the
       destination frame.
     - Disconnecting clients in the event of _any_ error, whether with the
       connection or an error on the client end in handling the call, doesn't make sense.
     - The `call` method returned a single Promise, so an error from any client
       would replace successful results from other clients. Also the client would
       have to wait until the results from all callers was available before sending
       a response.
     - The `call` method always passed a callback function to `PortRPC.call`,
       resulting in PortRPC always sending a response message back over the message
       channel, even when the code calling `bridge.call` did not use the response.
       Currently _no_ calls to `bridge.call` used the response, so this was just
       wasted traffic.
    
    The changes in detail are:
    
     1. Change the PortRPC API to be more like Bridge, where an instance is
        constructed, RPC handlers are added via `on` and the client is then
        connected to a port via `connect`.
    
     2. Replace use of Bridge with either a single PortRPC or array of PortRPCs in
        the `Guest`, `Sidebar` and `FrameSyncService` classes.
    
        This required changes in various tests to simulate a guest frame connecting
        before testing that a particular event resulted in an RPC call being made to
        a guest frame.
    
     3. Rename `bridge-events.d.ts` => `port-rpc-events.d.ts` to match (2)
    
     4. Remove the `Bridge` class, its tests and the PortRPC-Bridge integration
        tests, as they are now unused.
    
    Two existing issues were identified in this work but are not yet resolved:
    
     1. `FrameSyncService` does not tear down the PortRPC client for a guest when
         the guest frame disconnects.
     2. If a cross-frame RPC call is attempted before the async port discovery has
        completed, nothing happens. It will probably make sense to queue calls
        similar to how MessagePort queues messages.
    bb510f43
guest.js 23.3 KB