Opened on 10/05/2016 at 10:25:29 AM

Closed on 09/05/2019 at 04:22:08 PM

#4492 closed change (duplicate)

Refactor messageResponder to be less monolithic and more convenient to use

Reported by: trev Assignee:
Priority: Unknown Milestone:
Module: User-Interface Keywords:
Cc: greiner, fhd, sebastian, saroyanm Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

Description

Background

We currently have messageResponder as our message backend for all of UI, content scripts etc. It is a monolithic script which is misplaced in UI and hard to test.

What to change

I used a different approach in Easy Passwords, one that would forward function calls from content to background (content part, background part). The idea here is a minimal messaging API on both ends that will translate function calls into messages and return the result as a promise. A function can also return a result asynchronously by returning a promise, it will be resolved automatically. It can also throw and the exception will be forwarded to the caller as well (promise will be rejected).

We probably won't have UI code calling FilterStorage and friends directly - these APIs aren't meant for it and don't return serializable data. However, we can still split up messageResponder.js into several modules like filtersBackend.js:

let filters = {
  get: function(what)
  {
    if (what == "cssproperties")
      return ...;
    else
      return ...;
  },
  importRaw: function(text)
  {
    ...
  }
};

While we are at it, it makes sense to have the new modules in adblockpluscore - with proper unit tests and transpiling.

Attachments (0)

Change History (6)

comment:1 Changed on 10/05/2016 at 01:51:20 PM by fhd

Sounds good!

comment:2 Changed on 10/05/2016 at 05:58:12 PM by sebastian

  • Cc sebastian added

I'm all for moving away from ext.onMessage, besides being a quite monolithic mechanism, the whole abstraction layer implemented by the ext namespace is matter to be removed, now where we decided to move Safari support into a branch, and eventually discontinue it.

However, I thought we might want to adapt the new messaging API (#3499) which we already use in adblockplus (Firefox) and adblockpluschrome. Basically, the only difference I see, is that instead of registering each method individually, we would wrap object/namepsaces/modules more generically.

comment:3 Changed on 10/06/2016 at 08:27:55 AM by trev

The port API we've created is a lot better than ext.onMessage but I'd still prefer a minimal higher-level API on top of it - I definitely prefer calling functions to working with messages directly.

comment:4 Changed on 10/06/2016 at 10:45:25 AM by greiner

I do agree with the overall idea of splitting up/off messageResponder.js and I also agree that it'd be desirable to reuse the new messaging API.

The introduction of RPC appears to be unrelated though given the stated problems we're trying to solve:

  • monolithic
  • misplacedin UI
  • hard to test

We should also consider scenarios with multiple asynchronous responses (e.g. listenting for events) for which it'd be insufficient to return a single promise.

comment:5 Changed on 03/03/2017 at 10:09:33 AM by saroyanm

  • Cc saroyanm added

comment:6 Changed on 09/05/2019 at 04:22:08 PM by greiner

  • Resolution set to duplicate
  • Status changed from new to closed

Closing this ticket in favor of webext#75.

Add Comment

Modify Ticket

Change Properties
Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from (none).
 
Note: See TracTickets for help on using tickets.