1451527 - Convert tps extension to a webextension

Status: RESOLVED FIXED

(Firefox :: Sync, enhancement, P1)

Comment 1

[Lina Butler [:lina]]

Thanks for filing this, Andrew! I'm guessing we'll need to expose `ExtensionAPI` wrappers for TPS to use, similar to bug 1451463?

Comment 2

[Andrew Swan [:aswan]]

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.

Comment 3

[Lina Butler [:lina]]

(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.

Comment 4

[Mark Hammond (PTO; back Oct-3) [:markh] [:mhammond]]

(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.

Comment 5

[Thom Chiovoloni [:tcsc] (ex-moco)]

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...

Comment 6

[Kris Maglione [:kmag]]

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.

Comment 7

[Mark Hammond (PTO; back Oct-3) [:markh] [:mhammond]]

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.

Comment 8

[Mark Hammond (PTO; back Oct-3) [:markh] [:mhammond]]

This is coming up so we should get to it asap

Comment 9

[Thom Chiovoloni [:tcsc] (ex-moco)]

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.

Comment 10

[Thom Chiovoloni [:tcsc] (ex-moco)]

Attachment: Bug 1451527 - Move sync's TPS tests into a webextension r?markh

Comment 11

[Thom Chiovoloni [:tcsc] (ex-moco)]

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 12

[Mark Hammond (PTO; back Oct-3) [:markh] [:mhammond]]

Comment on attachment 9004691 [details]
Bug 1451527 - Move sync's TPS tests into a webextension r?markh

Mark Hammond [:markh] has approved the revision.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b886f2cf9d89
Move sync's TPS tests into a webextension r=markh

Comment 14

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7ad7b862c561
Backed out changeset b886f2cf9d89 for eslint failure on extensions/tps/api.js:34 CLOSED TREE

Comment 15

[Bogdan Tara[:bogdan_tara | bogdant]]

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

Comment 16

[Thom Chiovoloni [:tcsc] (ex-moco)]

Already fixed, sorry!

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f9c1de47280b
Move sync's TPS tests into a webextension r=markh

Comment 18

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/f9c1de47280b