1302020 - Messenger should require explicit filters

Status: RESOLVED FIXED

(WebExtensions :: Untriaged, defect, P3)

Description

[Rob Wu [:robwu]]

Messenger in ExtensionUtils takes a `filter` parameter, seemingly to require senders to match the given filter.
Except that if the sender omits all fields, the message still gets delivered...

We need to distinguish between keys that are mandatory and keys that are optional.

(added blockers because the linked issue introduce new instances of Messenger)

Comment 1

[Rob Wu [:robwu]]

Attachment: Bug 1302020 - Add filter to Messenger + fix code documentation

- Make all keys in `filter` mandatory.
- Add `optionalFilter` to Messenger to take over the role of the former
  `filter` parameter.
- Add documentation to Messenger to explain what it really does.
- Fix type comments. Many message message managers were declared as a
  sender, while they are at most a receiver (e.g. `Services.pppm`).

Review commit: https://reviewboard.mozilla.org/r/78110/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/78110/

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8790157 [details]
Bug 1302020 - Add filter to Messenger + fix code documentation

https://reviewboard.mozilla.org/r/78110/#review78860

::: toolkit/components/extensions/Extension.jsm
(Diff revision 1)
>      if (uri) {
>        sender.url = uri.spec;
>      }
>      Management.emit("page-load", this, params, sender);
>  
> -    // Properties in |filter| must match those in the |recipient|

Why remove the comment?

::: toolkit/components/extensions/ExtensionContent.jsm:327
(Diff revision 1)
>      }
>  
>      let url = contentWindow.location.href;
>      // The |sender| parameter is passed directly to the extension.
>      let sender = {id: this.extension.uuid, frameId, url};
> -    // Properties in |filter| must match those in the |recipient|
> +    let filter = {extensionId: this.extension.id};

Same, why remove this?

Comment 3

[Rob Wu [:robwu]]

Comment on attachment 8790157 [details]
Bug 1302020 - Add filter to Messenger + fix code documentation

https://reviewboard.mozilla.org/r/78110/#review78860

> Why remove the comment?

Because it is already documented (in much more detail and with more precise phrasing) in Messenger.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/98c3c114c936
Add filter to Messenger + fix code documentation r=billm

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/98c3c114c936

Comment 6

[Gerry Chang [:gchang]]

Won't fix in 51 as this is not the feature shipped in 51.