Closed Bug 1618188 Opened 8 months ago Closed 8 months ago

Remove XML blocklist backend from mozilla-central

Categories

(Toolkit :: Blocklist Implementation, task, P3)

task

Tracking

()

RESOLVED FIXED
mozilla76
Tracking Status
firefox75 --- wontfix
firefox76 --- fixed

People

(Reporter: Gijs, Assigned: Gijs)

References

Details

Attachments

(2 files)

With the switch to remote settings complete, we should stop shipping the 600k XML file and remove all the code.

Blocks: 1577304

We still need to maintain and ship the XML blocklist on the server-side though, for ESR 68 people at least. How long do we need to support people who are on really old versions and don't update?

(In reply to Daniel Veditz [:dveditz] from comment #2)

We still need to maintain and ship the XML blocklist on the server-side though, for ESR 68 people at least. How long do we need to support people who are on really old versions and don't update?

I think probably until 78 (next ESR).

Stuart/Amy, can you confirm if I'm good to remove the v1 (XML) client-side stuff from nightly 75 or 76, leaving only the v2 (remote settings) one, ie we don't currently see a reason we would want to switch the clients back to XML? We've shipped RS as the default with 73 (having rolled it out as the default through the 72 cycle) and so removing the old stuff unships the 600kb XML text file itself that is currently part of the build, as well as some 8 thousand other lines of code, tests and updating logic.

Flags: needinfo?(scolville)
Flags: needinfo?(atsay)

(In reply to :Gijs (he/him) from comment #3)

(In reply to Daniel Veditz [:dveditz] from comment #2)

We still need to maintain and ship the XML blocklist on the server-side though, for ESR 68 people at least. How long do we need to support people who are on really old versions and don't update?

I think probably until 78 (next ESR).

Oh, I guess I just realized that you meant, we may need to support people who keep running versions after we EOL them, on the server-side. I think that's the AMO team's call (in conjunction with product and security). This bug is just about not shipping the unused implementation in the client anymore, and cleaning up the blocklist implementation as a result, making it easier to understand and to start work on v3...

Priority: -- → P3

(In reply to :Gijs (he/him) from comment #3)

(In reply to Daniel Veditz [:dveditz] from comment #2)

We still need to maintain and ship the XML blocklist on the server-side though, for ESR 68 people at least. How long do we need to support people who are on really old versions and don't update?

I think probably until 78 (next ESR).

Stuart/Amy, can you confirm if I'm good to remove the v1 (XML) client-side stuff from nightly 75 or 76, leaving only the v2 (remote settings) one, ie we don't currently see a reason we would want to switch the clients back to XML? We've shipped RS as the default with 73 (having rolled it out as the default through the 72 cycle) and so removing the old stuff unships the 600kb XML text file itself that is currently part of the build, as well as some 8 thousand other lines of code, tests and updating logic.

Removing the un-used client code sounds reasonable to me. As noted this is separate to removing the server-side parts which we'll need to handle independently.

Flags: needinfo?(scolville)

Removing the un-used client code sounds reasonable to me. As noted this is separate to removing the server-side parts which we'll need to handle independently.

Just FWIW, as long the data in blocklists/* continues to be updated in the same schema, the server-side part can be kept in place with too much efforts.
However, now that we don't rely on the XML download to count ADU, we could serve it through the CDN, instead of the current Nginx. In addition to simplifying to architecture, I believe that the cost could also be reduced significantly.
What do you think?

(In reply to Mathieu Leplatre [:leplatrem] from comment #6)

However, now that we don't rely on the XML download to count ADU, we could serve it through the CDN, instead of the current Nginx. In addition to simplifying to architecture, I believe that the cost could also be reduced significantly.
What do you think?

I'm sympathetic to this, but discussion about this should probably happen in a separate bug - Mathieu, can you file one? Thanks!

Flags: needinfo?(mathieu)
Flags: needinfo?(atsay)

Sorry to clear my NI without a comment--I meant to say "What Stuart said."

See Also: → 1619209
Flags: needinfo?(mathieu)

This removes the obsolete backend. Notes on some of the less obvious changes
made as part of this patch:

  • some of the gFoo style getters in Blocklist.jsm were only used by the XML
    version of the blocklist; I've removed them and tried to remove spurious
    settings of those properties in the remaining tests.
  • some utility methods (e.g. distribution information getters) were also only
    used for the XML version (for the update URL).
  • it's no longer necessary to test switching implementations.
  • in browser/base/content/test/plugins/, we ran some tests from two manifests
    in order to run them with both blocklist backends. The simplest way of
    reducing this back down to one was to remove the remote-settings one. If I'd
    been more future-oriented when I created the duplication, perhaps I would
    have moved the XML version out into a different manifest instead, but I
    didn't, so now it looks like we're removing the modern one, whereas really
    we're going to be running the modern one as part of the "normal" tests and
    we're no longer running the "old" tests.
  • removed all mentions I could see of extensions.blocklist.url which is no
    longer used for anything.
  • per https://bugzilla.mozilla.org/show_bug.cgi?id=1016555#c23, updated
    references for the OneCRL timing and how it relates to blocklist updates.

Depends on D64932

Blocks: 1619287
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/cbe2fe9d1c54
stop updating the mozilla-central copy of blocklist.xml, r=RyanVM
https://hg.mozilla.org/integration/autoland/rev/06ca3c111fc7
remove XML backend for plugin and add-on blocklisting, r=mconley,perftest-reviewers,whimboo
Status: NEW → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla76
You need to log in before you can comment on or make changes to this bug.