Closed Bug 1451527 Opened 2 years ago Closed Last year

Convert tps extension to a webextension

Categories

(Firefox :: Sync, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 64
Tracking Status
firefox64 --- fixed

People

(Reporter: aswan, Assigned: tcsc)

References

(Depends on 1 open bug)

Details

Attachments

(1 file)

No description provided.
Thanks for filing this, Andrew! I'm guessing we'll need to expose `ExtensionAPI` wrappers for TPS to use, similar to bug 1451463?
I don't know much about TPS but if there are things that it does that aren't available to regular webextensions, that would be the idea.  I'd be happy to walk you through it on irc or vidyo or something if that would help.
(In reply to Andrew Swan [:aswan] from comment #2)
> if there are things that it does that aren't
> available to regular webextensions...

A lot. :-) It's what we use for Sync integration tests, so everything in https://searchfox.org/mozilla-central/source/services/sync/tps/extensions/tps/resource/modules (and the FxA auth layer, in https://searchfox.org/mozilla-central/source/services/sync/tps/extensions/tps/resource/auth/fxaccounts.jsm) that doesn't have a WebExtension API will need to be ported.
(In reply to Kit Cambridge (they/them) [:kitcambridge] from comment #3)
> A lot. :-) It's what we use for Sync integration tests, so everything in
> https://searchfox.org/mozilla-central/source/services/sync/tps/extensions/
> tps/resource/modules (and the FxA auth layer, in
> https://searchfox.org/mozilla-central/source/services/sync/tps/extensions/
> tps/resource/auth/fxaccounts.jsm) that doesn't have a WebExtension API will
> need to be ported.

One interesting side-effect of this might be that about:sync could become a webextension. However, I suspect it is a significant amount of work that we will struggle to find time for this quarter.
Some of it could probably be done with existing webextension APIs. I don't think it's extremely important that TPS adds bookmarks via a Places API vs via `browser.bookmarks`.

That's certainly not true for everything though. Either way, it would might be worth using this opportunity to do the long-discussed "TPS Rewrite", given how clunky and error-prone it is...
See Also: → 1452168
Priority: -- → P3
Just FYI, bootstrapped extensions are going away. Soon. If this add-on still hasn't been converted when we're ready to turn them off, I'll probably take your P3 priority as an indication that these tests are not especially critical and can be disabled.
These tests are critical - while it's unlikely we'll get to this in this or the next iteration, if it makes your process easier I'm happy to flag this as P2.
Priority: P3 → P2
Depends on: 1320862
This is coming up so we should get to it asap
Priority: P2 → P1
I have this done locally (I'd put up a WIP patch but I'm not sure how to do that with the phabricator world and don't feel like figuring it out), but TPS fails for the same reason that TPS prod fails in automation.

I misidentified the fix before, bug 1453096 isn't the actual issue.

The real issue is a cleanup issue, caused by an exception from inside activity stream about removing a preference listener, although it's very sensitive to initialization timing.

If I put an artificial setTimeout before initializing TPS, AS doesn't throw, but TPS fails because TPS is a huge mess of race conditions controlled by observer notifications that must fire in the correct order, but this throws that order off.
Assignee: nobody → tchiovoloni
Status: NEW → ASSIGNED
This doesn't fix TPS's failures, but I've followed https://bugzilla.mozilla.org/show_bug.cgi?id=1451527 as a followup.

Note that we don't use the same technique as about:sync, since this way we don't need to steal the `browser.tps` namespace or anything.

(Also, TPS failed to initialize properly when I did it that way, although I didn't dig into this much -- it seemed mainly like some observer got fired before TPS was ready, and so it was spent waiting for an event that had already fired).
Comment on attachment 9004691 [details]
Bug 1451527 - Move sync's TPS tests into a webextension r?markh

Mark Hammond [:markh] has approved the revision.
Attachment #9004691 - Flags: review+
Pushed by tchiovoloni@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b886f2cf9d89
Move sync's TPS tests into a webextension r=markh
Backout by btara@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7ad7b862c561
Backed out changeset b886f2cf9d89 for eslint failure on extensions/tps/api.js:34 CLOSED TREE
Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=b886f2cf9d8966776ebf5344d10df260fb8c4383

Backout link: https://hg.mozilla.org/integration/autoland/rev/7ad7b862c561de147fbde605f4418b88d295470c

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=197410993&repo=autoland&lineNumber=288

[task 2018-09-04T16:49:12.589Z] Installing setuptools, pip, wheel...done.
[task 2018-09-04T16:49:13.775Z] running build_ext
[task 2018-09-04T16:49:13.775Z] building 'psutil.psutil_linux' extension
[task 2018-09-04T16:49:13.775Z] creating build
[task 2018-09-04T16:49:13.775Z] creating build/temp.linux-x86_64-2.7
[task 2018-09-04T16:49:13.775Z] creating build/temp.linux-x86_64-2.7/psutil
[task 2018-09-04T16:49:13.775Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_common.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_common.o
[task 2018-09-04T16:49:13.775Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_posix.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o
[task 2018-09-04T16:49:13.775Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_linux.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_linux.o
[task 2018-09-04T16:49:13.775Z] creating build/lib.linux-x86_64-2.7
[task 2018-09-04T16:49:13.775Z] creating build/lib.linux-x86_64-2.7/psutil
[task 2018-09-04T16:49:13.775Z] x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -Wl,-z,relro -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wl,-Bsymbolic-functions -Wl,-z,relro -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security build/temp.linux-x86_64-2.7/psutil/_psutil_common.o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o build/temp.linux-x86_64-2.7/psutil/_psutil_linux.o -o build/lib.linux-x86_64-2.7/psutil/_psutil_linux.so
[task 2018-09-04T16:49:13.775Z] building 'psutil.psutil_posix' extension
[task 2018-09-04T16:49:13.775Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_common.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_common.o
[task 2018-09-04T16:49:13.775Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_posix.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o
[task 2018-09-04T16:49:13.775Z] x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -Wl,-z,relro -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wl,-Bsymbolic-functions -Wl,-z,relro -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security build/temp.linux-x86_64-2.7/psutil/_psutil_common.o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o -o build/lib.linux-x86_64-2.7/psutil/_psutil_posix.so
[task 2018-09-04T16:49:13.775Z] copying build/lib.linux-x86_64-2.7/psutil/_psutil_linux.so -> psutil
[task 2018-09-04T16:49:13.776Z] copying build/lib.linux-x86_64-2.7/psutil/_psutil_posix.so -> psutil
[task 2018-09-04T16:49:13.776Z] 
[task 2018-09-04T16:49:13.776Z] Error processing command. Ignoring because optional. (optional:packages.txt:comm/build/virtualenv_packages.txt)
[task 2018-09-04T16:55:10.821Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/intl/l10n/L10nRegistry.jsm:91:10 | Expected to return a value at the end of async generator method 'generateContexts'. (consistent-return)
[task 2018-09-04T16:55:10.821Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/services/sync/tps/extensions/tps/api.js:34:71 | Missing semicolon. (semi)
[task 2018-09-04T16:55:10.821Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/services/sync/tps/extensions/tps/api.js:39:19 | Irregular whitespace not allowed. (no-irregular-whitespace)
[task 2018-09-04T16:55:10.821Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/services/sync/tps/extensions/tps/api.js:41:51 | 'err' is not defined. (no-undef)
[task 2018-09-04T16:55:10.821Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/services/sync/tps/extensions/tps/api.js:59:26 | 'ExtensionAPI' is not defined. (no-undef)
[taskcluster 2018-09-04 16:55:11.343Z] === Task Finished ===
[taskcluster 2018-09-04 16:55:11.344Z] Unsuccessful task run with exit code: 1 completed in 652.92 seconds
Flags: needinfo?(tchiovoloni)
Already fixed, sorry!
Flags: needinfo?(tchiovoloni)
Pushed by tchiovoloni@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f9c1de47280b
Move sync's TPS tests into a webextension r=markh
https://hg.mozilla.org/mozilla-central/rev/f9c1de47280b
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
You need to log in before you can comment on or make changes to this bug.