Issue 3915 - Dispatch filters.added consistently

Issue 3915 - Dispatch filters.added consistently

[kzar, 2016-04-13 07:45:32]

Patch Set 1

[Wladimir Palant, 2016-04-14 09:20:55]

NOT LGTM

With this approach there will be two notifications sent out for this filter,
first subscription.added and then filter.added. Note that the former already
implies filter.added because the filter is listed in the subscription. This will
very likely cause issues with the Filter Preferences in Firefox because filters
aren't treated as a plain list there.

Proper solution would be handling subscription.added correctly in Chrome - for
SpecialSubscription instances all filters on the subscription should be added.

[Sebastian Noack, 2016-04-14 09:50:41]

On 2016/04/14 09:20:55, Wladimir Palant wrote:
> NOT LGTM
> 
> With this approach there will be two notifications sent out for this filter,
> first subscription.added and then filter.added. Note that the former already
> implies filter.added because the filter is listed in the subscription. This
will
> very likely cause issues with the Filter Preferences in Firefox because
filters
> aren't treated as a plain list there.
> 
> Proper solution would be handling subscription.added correctly in Chrome - for
> SpecialSubscription instances all filters on the subscription should be added.

Yes, alternatively we could handle the filters that are already in the
subscription when a special subscription is added. However, this makes the logic
more complicated, as everywhere where you have to respond to new custom filters
you'd have to handle subscription.added and filter.added. In particular where we
have to rely on asynchronous messaging this requires quite some addional logic.

I'm not exactly sure about the implications for Filter Preferences on Firefox,
but IMO the code should handle filter.added anyway, so it shouldn't make any
difference whether we first create an empty subscription and then add filter(s)
to it, or whether we create the subscription with the first filter already in
there.