Opened 4 years ago

Last modified 3 years ago

#3499 closed change

Create a clean messaging API for internal use — at Version 6

Reported by: trev Assignee: trev
Priority: P3 Milestone: Adblock-Plus-2.7.3-for-Firefox
Module: Adblock-Plus-for-Firefox Keywords:
Cc: sebastian, sergz Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29338275/
https://codereview.adblockplus.org/29338279/
https://codereview.adblockplus.org/29338283/
https://codereview.adblockplus.org/29338417/
https://codereview.adblockplus.org/29338421/
https://codereview.adblockplus.org/29338425/

Description (last modified by trev)

Background

For our E10S support, we are currently using message manager directly. This results in overly complicated code however: message names have to avoid conflicts with other extensions, message handlers have to be removed explicitly, response handling has to be implemented explicitly, messaging code is duplicated for the first-run page.

What to change

Create a clean internal API, accessible in both parent and child via the messaging module:

  • Port constructor binds a communication port to a message manager.
  • port instance is the default port (communicating with the parent for process scripts, broadcasting to all children for the parent).
  • Port.disconnect() shuts down the port (done automatically for the default port when the extension is shut down).
  • Port.on() adds a handler for a particular message (message names are specific to Adblock Plus and won't conflict with other extensions).
  • Port.off() removes a handler.
  • Port.emit() sends/broadcasts an asynchronous message, if provided the response callback will be triggered with the response.
  • Port.emitSync() sends a synchronous message, response is returned from the function.

Message handlers have two parameters: payload (the data sent by the sender) and sender (a Port instance). The handler has the choice between returning undefined (no response), a value (actual response) or a promise (indicates an async response). We should warn if multiple handlers choose to return a value for the same message or if a handler chooses to return a promise for a sync message.

Change History (6)

comment:1 Changed 4 years ago by trev

  • Description modified (diff)

comment:2 Changed 4 years ago by trev

  • Description modified (diff)

comment:3 Changed 4 years ago by trev

  • Ready set

comment:4 Changed 4 years ago by sebastian

  • Cc sebastian added

comment:5 in reply to: ↑ description Changed 4 years ago by sebastian

Replying to trev:

and sendResponse (callback to be called with the response).

What do we need that callback for if the return value can be the result or a promise that provides the result?

Last edited 4 years ago by sebastian (previous) (diff)

comment:6 Changed 4 years ago by trev

  • Description modified (diff)

True, I forgot to remove that parameter.

Actually, there is a complication I didn't consider. When the parent is broadcasting a message, there will normally be two child processes receiving it. How is the parent going to know that it received the response if each child process is going to send one? It seems that the parent needs to wait for two responses (more precisely, nsIMessageBroadcaster.childCount) to arrive, then choose the one that isn't undefined (also warn if more than one is).

Note: See TracTickets for help on using tickets.