Refactor trackers for bridged engines
Categories
(Firefox :: Sync, task)
Tracking
()
Tracking | Status | |
---|---|---|
firefox78 | --- | fixed |
People
(Reporter: lina, Assigned: lina)
References
Details
Attachments
(2 files)
I didn't spend much time on the BridgedTracker
stub, but it's not quite enough to build your own tracker. In practice, bridged trackers will only do the "listen for observer notifications and bump the score" part, and not persist a list of changed IDs.
I think a better way to do this is to split out persistence into a PersistentTracker
, put a scary "don't use this" comment above it, and make the engines that actually need it (add-ons, clients, forms, history, and passwords) use it. Everyone else can use the base tracker that throws if you try to make it persist anything.
(I thought about doing this while I was adding more docs for BridgedEngine
, let's do that, too!).
Assignee | ||
Comment 1•4 years ago
|
||
Actually, I'm going to be more opinionated, and call it a LegacyTracker
! 😈
Comment 2•4 years ago
|
||
Yes! That's exactly the kind of optimism that's lead to us hauling around a scope named "oldsync" for all these years :thumbsup:
(Seriously though, +1 to the name)
Assignee | ||
Comment 3•4 years ago
|
||
The tracker base class currently does two things: bump the score in
response to observer notifications, and store a list of changed IDs.
The bookmarks, form autofill, and now bridged trackers need to hack
around this to opt out of persistence, since they handle change
tracking in the storage layer.
This commit keeps the score logic in Tracker
, but moves all the
persistence code into an intermediate LegacyTracker
class, and
changes all engines that need persistence to inherit from it.
ignoreAll
is more interesting. We want new-style stores to emit
observer notifications with change sources, so that the tracker knows
to ignore changes made by Sync. Ignoring all observer notifications
during a sync is a blunter version of this. But, not every new store
supports change sources, so we reimplement ignoreAll
manually for
ones that don't.
Assignee | ||
Comment 4•4 years ago
|
||
Now that we have a Tracker
base class without persistence, we can
have bridged engines subclass it directly.
Depends on D74374
Assignee | ||
Comment 5•4 years ago
|
||
Yes! That's exactly the kind of optimism that's lead to us hauling around a scope named "oldsync" for all these years :thumbsup:
🤣🤣🤣
A girl can dream, right? Right? ☀️
Updated•4 years ago
|
Updated•4 years ago
|
Pushed by kcambridge@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/40d980163482 Split out persistence from `Tracker` into `LegacyTracker`. r=markh,rfkelly https://hg.mozilla.org/integration/autoland/rev/86a1d0814d03 Add more docs for `BridgedEngine`, and remove `BridgedTracker`. r=markh,rfkelly
Assignee | ||
Comment 7•4 years ago
|
||
Comment 8•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/40d980163482
https://hg.mozilla.org/mozilla-central/rev/86a1d0814d03
Description
•