1173324 - Storage Inspector: Simplify the process of parent / child communication

Status: RESOLVED WORKSFORME

(DevTools :: Storage Inspector, defect)

Description

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

toolkit/devtools/server/actors/storage.js

> @@ +722,5 @@
> > +      };
> > +      this.removeCookieObservers = () => {
> > +        Services.obs.removeObserver(this, "cookie-changed", false);
> > +        Services.obs.removeObserver(this, "http-on-response-set-cookie", false);
> > +      };
> 
> That looks so hacky to define methods like this...
> I'm wondering if instead of doing that...
> we were just calling cookieHelpers directly.
> But. We would define cookieHelpers conditonally.
> 
> let cookieHelpers = {
>   // existing cookieHelpers definition
> };
> if (DebuggerServer.isInChildProcess) {
>   const { sendSyncMessage, addMessageListener } =
> DebuggerServer.parentMessageManager;
>   cookieHelper = {
>     getCookiesFromHost: callParentProcess...,
>     addObservers: ...,
>     removeObservers: ...
>   };
>   // you would need to register the message listener in addObservers.
> }
>   }
> }
> 
> Note that you could get rid of maybeSetupChildProcess if you prefer. Or you
> could define this.cookieHelper instead of overriding the cookieHelper global.
> 

I have spent a day playing around with this and other methods turn out just as hacky due to having to bind and manipulate contexts. This bug is a reminder to look into our options here.

Comment 1

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Spent a couple of days on this but it really is hard to improve upon the current setup.

I will come back to this at some time in the future.

Comment 2

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

A number of devs have looked at this over time and the truth is that the current setup is about the best we can do.