Opened 3 years ago

Closed 6 weeks ago

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

Change History (6)

comment:1 Changed 3 years ago by fhd

Sounds good!

comment:2 Changed 3 years ago 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 3 years ago 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 3 years ago 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 3 years ago by saroyanm

  • Cc saroyanm added

comment:6 Changed 6 weeks ago by greiner

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

Closing this ticket in favor of webext#75.

Note: See TracTickets for help on using tickets.