Closed Bug 1451484 Opened 3 years ago Closed 3 years ago
Land Webcompat Go
Faster webextension port in Fenenc
In bug 1386807, webcompat will become a hybrid extension. But eventually it will need to get move off bootstrap altogether.
Marking as P2 to get it on my dashboards and since it seems to align with the bootstrapped add-ons deprecation timeframe. Please modify according to our plans.
Priority: -- → P2
I'm taking this and lifting it to P1, as this will be one of the very next GoFaster tasks I will be working on to avoid any blockers for the addons-team. :)
Assignee: nobody → dschubert
Priority: P2 → P1
Update: According to Tom, the addons team would really much like us to be done in 64, so I am now actively working on this.
Attaching a link to the Pull Request so people can track the current work-in-progress source. Pushed a new try run, as my last one didn't cover Android and some codes have changed, so I want to be sure we still do not set the tree on fire. Try run is at , Talos results will be visible on . : https://treeherder.mozilla.org/#/jobs?repo=try&revision=8e71e2cde7e7155efcfb6ebaca586db58b06f6f4 : https://treeherder.mozilla.org/perf.html#/comparechooser?newProject=try&newRevision=8e71e2cde7e7155efcfb6ebaca586db58b06f6f4
From a previous try run , I am aware that there is an apparent memleak with this extension. It's noteworthy that the leakcheck only triggers on serviceworker tests inside `dom/serviceworkers/test/`. After talking to Tom, I gathered that this might not be an actual leak at all, but simply something the tests don't expect. However, folks in #memshrink gave me valuable advise on how to further dig into this, which I will do today, to verify whether this is an issue or not - and if it is, how we can fix it. Will escalate this if I can't figure something out by this evening, as our timeline is rather ... short. :) The old try run also showed some significant performance impacts, but I did not look deeper into them just yet. : https://treeherder.mozilla.org/#/jobs?repo=try&revision=856cf6b19016f8aac28568291ac9a356fd5840d8 : https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-central&newProject=try&newRevision=856cf6b19016f8aac28568291ac9a356fd5840d8&framework=1&showOnlyImportant=1&selectedTimeRange=172800
Just to narrow it down a bit, would you mind removing experiment_apis from the manifest.json, and see if that passes try?
Hm, that looks suspiciously like bug 1426822, though that failure was intermittent and yours seems permanent. Extensions that mochitest uses (mochikit, specialpowers) are actually webextensions+experiments, those should work fine on Android. I'll try to keep looking but meanwhile ni? to Geoff who was just looking at related code as part of bug 1426822
Flags: needinfo?(aswan) → needinfo?(gbrown)
> Just to narrow it down a bit, would you mind removing experiment_apis from the manifest.json, and see if that passes try? I sure can! Here is the push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5c1de74d97057abddc75b905461e38805b74dfe2 Note that by removing the APIs, I had to comment out their usage, which now has turned the extension in a glorified NOOP, since we never get the chance to register anything.
Okay, the first mochitests ran, and they are still failing. I actually doubt it is an issue with the extension itself, as one can download the apk from treeherder, and that works just fine. Fenenc loads, and the extension is also working just fine, so this has to be something else. :/
(In reply to Dennis Schubert [:denschub] from comment #13) > Okay, the first mochitests ran, and they are still failing. I actually doubt > it is an issue with the extension itself, as one can download the apk from > treeherder, and that works just fine. Fenenc loads, and the extension is > also working just fine, so this has to be something else. :/ I tried to investigate this, and yes, I conclude that you're tickling something deeper. I don't understand how the test harness can be invoking `hookupTests` without `RunSet` defined, but that's what appears to be happening. That suggests that the calling context of `hookupTests` is changing somehow -- do we need to `bind` something? If so, how on earch could that be intermittent? I would like to see a try run with the logtag at https://searchfox.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/updater/PostUpdateHandler.java#31 set to DEBUG, but I don't know how to do that. Geoff, where do we swizzle log verbosity in the test harnesses? I want to see if the Web Extension is actually getting unpacked in automation.
Flags: needinfo?(nalexander) → needinfo?(gbrown)
I don't know of a very convenient way. The logcat artifact is generated by: https://dxr.mozilla.org/mozilla-central/rev/56b988a937689d5599400afa59b72c390b40abf2/testing/mozharness/mozharness/mozilla/testing/android.py#173 but you could just as easily change the log level in PostUpdateHandler.
While others are looking at the broken tests, the performance regression is still a thing. Looking at , this patch causes quite some performance regressions, especially during startup. Florian, I looked at some profiles and compared them with/without my patch, but unfortunately, I fail to notice anything obvious I could address in my sources. As you are much more experienced in profiling frontend code, do you mind having a look? :) : https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-central&newProject=try&newRevision=8e71e2cde7e7155efcfb6ebaca586db58b06f6f4&framework=1&showOnlyImportant=1&selectedTimeRange=172800
Florian, ignore that. This is actually being discussed in bug 1483172. Thanks Mike for linking that. :)
Attachment #9013361 - Attachment description: Bug 1451484 - Import WebExtension sources for the WebCompat GoFaster Addon → Bug 1451484 - Import WebExtension sources for the WebCompat GoFaster Addon. r=aswan,rhelmer
Dennis pushed what we think you asked for Nick (neither of us are Android devs...): https://treeherder.mozilla.org/#/jobs?repo=try&revision=4e736b22617c697c952d91db5e5d71629e0a5130&selectedJob=204282997 https://taskcluster-artifacts.net/UNeMBiLBRUi2rRihUOxczQ/0/public/logs/live_backing.log Relevant GeckoPostUpdateHandler debug stuff below: [task 2018-10-09T17:43:58.528Z] 17:43:58 INFO - /builds/worker/workspace/build/android-sdk-linux/platform-tools/adb -s emulator-5554 logcat -v threadtime Trace:S StrictMode:S ExchangeService:S GeckoPostUpdateHandler:D [task 2018-10-09T17:52:16.884Z] 17:52:16 INFO - 10-09 10:45:03.793 D/GeckoPostUpdateHandler( 826): Build ID changed since last start: '20181009172248', 'null' [task 2018-10-09T17:52:16.885Z] 17:52:16 INFO - 10-09 10:45:03.803 D/GeckoPostUpdateHandler( 826): Copying system add-ons from APK to dataDir [task 2018-10-09T17:52:16.890Z] 17:52:16 INFO - 10-09 10:45:04.883 D/GeckoPostUpdateHandler( 826): Copying 'firstname.lastname@example.org' from APK to dataDir [task 2018-10-09T17:52:16.890Z] 17:52:16 INFO - 10-09 10:45:04.883 D/GeckoPostUpdateHandler( 826): Creating /data/data/org.mozilla.fennec_aurora/features I'm not sure that actually tells us anything about web extensions getting unpacked, unless that's what "Copying system add-ons from APK to dataDir" means. Is this helpful, Nick?
Running Dennis' patches locally (on an emulator), I guess I get the same results as try (and I guess Bug 1494657)?: 14:00.55 INFO Running manifest: dom/animation/test/mochitest.ini 14:00.55 INFO The following extra prefs will be set: dom.animations-api.compositing.enabled=true dom.animations-api.core.enabled=true dom.animations-api.getAnimations.enabled=true dom.animations-api.implicit-keyframes.enabled=true dom.animations-api.timelines.enabled=true layout.css.motion-path.enabled=true 14:01.55 adb WARNING Ignoring attempt to chmod external storage pk12util: PKCS12 IMPORT SUCCESSFUL 14:01.99 INFO MochitestServer : launching [u'/Users/mitaylor/.mozbuild/android-device/host-utils-61.0a1.en-US.mac/xpcshell', '-g', '/Users/mitaylor/.mozbuild/android-device/host-utils-61.0a1.en-US.mac', '-f', '/Users/mitaylor/.mozbuild/android-device/host-utils-61.0a1.en-US.mac/components/httpd.js', '-e', "const _PROFILE_PATH = '/var/folders/vx/1lx2ynfd46lb9rh4zbl9rchh0000gn/T/tmpDQjdS0.mozrunner'; const _SERVER_PORT = '8888'; const _SERVER_ADDR = '192.168.2.83'; const _TEST_PREFIX = undefined; const _DISPLAY_RESULTS = false;", '-f', '/Users/mitaylor/dev/gecko/objdir-fennec/_tests/testing/mochitest/server.js'] 14:01.99 INFO runtests.py | Server pid: 53266 14:02.00 INFO runtests.py | Websocket server pid: 53267 14:02.01 INFO runtests.py | SSL tunnel pid: 53268 14:03.31 adb WARNING Ignoring attempt to chmod external storage 14:03.31 INFO runtests.py | Running with e10s: False 14:03.31 INFO runtests.py | Running with serviceworker_e10s: False 14:03.31 INFO runtests.py | Running tests: start. 14:03.96 adb INFO launch_application: am start -W -n org.mozilla.fennec_mitaylor/org.mozilla.gecko.BrowserApp -a android.intent.action.VIEW --es env9 MOZ_CRASHREPORTER_NO_REPORT=1 --es env8 R_LOG_DESTINATION=stderr --es args '-no-remote -profile /sdcard/tests/profile//' --es env3 DISABLE_UNSAFE_CPOW_WARNINGS=1 --es env2 R_LOG_VERBOSE=1 --es env1 XPCOM_DEBUG_BREAK=stack --es env0 MOZ_CRASHREPORTER=1 --es env7 MOZ_LOG_FILE=/sdcard/tests/mozlog/moz.log --es env6 MOZ_CRASHREPORTER_SHUTDOWN=1 --es env5 MOZ_IN_AUTOMATION=1 --es env4 MOZ_DISABLE_NONLOCAL_CONNECTIONS=1 --es env13 MOZ_HIDE_RESULTS_TABLE=1 --es env12 R_LOG_LEVEL=6 --es env11 MOZ_PROCESS_LOG=/var/folders/vx/1lx2ynfd46lb9rh4zbl9rchh0000gn/T/tmpLsXVWkpidlog --es env10 MOZ_DEVELOPER_REPO_DIR=/Users/mitaylor/dev/gecko -d 'http://mochi.test:8888/tests?autorun=1&closeWhenDone=1&logFile=%2Fsdcard%2Ftests%2Flogs%2Fmochitest.log&fileLevel=INFO&consoleLevel=INFO&hideResultsTable=1&manifestFile=tests.json&dumpOutputDirectory=%2Fsdcard%2Ftests' INFO | automation.py | Application pid: 2958 wait for org.mozilla.fennec_mitaylor complete; top activity=org.mozilla.fennec_mitaylor Browser unexpectedly found running. Killing... 21:13.77 INFO Failed to retrieve MOZ_UPLOAD_DIR env var TEST-UNEXPECTED-FAIL | remoteautomation.py | application timed out after 370 seconds with no output INFO | automation.py | Application ran for: 0:07:23.709434 INFO | zombiecheck | Reading PID log: /var/folders/vx/1lx2ynfd46lb9rh4zbl9rchh0000gn/T/tmpLsXVWkpidlog MOZ_UPLOAD_DIR not defined; tombstone check skipped 21:28.31 INFO Stopping web server 21:28.32 INFO Stopping web socket server 21:28.34 INFO Stopping ssltunnel 21:28.36 WARNING leakcheck | refcount logging is off, so leaks can't be detected! 21:28.36 INFO runtests.py | Running tests: end. 21:29.99 INFO Buffered messages finished 0 INFO TEST-START | Shutdown 1 INFO Passed: 0 2 INFO Failed: 0 3 INFO Todo: 0 4 INFO Mode: non-e10s 5 INFO SimpleTest FINISHED 21:30.98 INFO Buffered messages finished 21:30.98 SUITE_END 21:30.98 Overall Summary =============== mochitest-chrome ~~~~~~~~~~~~~~~~ Ran 0 checks () Expected results: 0 OK mochitest-plain ~~~~~~~~~~~~~~~ Ran 2 checks (2 tests) Expected results: 0 Skipped: 2 tests OK
Geoff, do we think Bug 1494657 is the culprit here? If so, a P5 is possibly too low. We're essentially blocked on that (if it's the actual root cause), and we're blocking Bug 1449052. Andrew, are you aware of any other web extensions system addons efforts in Fennec, or are we the lucky ones?
Bug 1494657 tracks an intermittent failure. It is fairly frequent, but only happens maybe 1 time in 100 mochitest tasks: https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&searchStr=android,4.3,mochitest&selectedJob=204421334 That seems a long way from the permafail reported here. "370 seconds with no output" means just that -- something is hung / tests aren't running. There could be many underlying causes. That said, there seem to be a lot of similar log "clues" between the two bugs. I am actively working on bug 1494657, will update the priority, and will be commenting there today. (I'm also feeling a little stuck, so would be happy for any help!)
Susheel, is there anyone on the mobile side of things who can help us debug why all mochitests would be failing (timing out) after adding a web extension?
(In reply to Mike Taylor [:miketaylr] (62 Regression Engineering Owner) from comment #20) > Geoff, do we think Bug 1494657 is the culprit here? If so, a P5 is possibly > too low. We're essentially blocked on that (if it's the actual root cause), > and we're blocking Bug 1449052. I agree with Geoff that this happens to have the same failure message but is almost certainly a separate issue. I downloaded the patch and using the emulator locally I am able to run mochitests successfully... > Andrew, are you aware of any other web extensions system addons efforts in > Fennec, or are we the lucky ones? We didn't have any system addons of any sort on Fennec until quite recently so I think you're blazing a new path here.
I feel like David Durst needs to be consulted here.
Flags: needinfo?(sdaswani) → needinfo?(ddurst)
OK, in the meantime new plan: Let's split the desktop and mobile parts of this and try to get desktop landed first, before the soft freeze. Kinda sucks, but we can aim for beta uplift of the mobile system addon.
Let's use this bug to track Fennec landing, as most of the comments here are about the failing Mochitests in Fennec. I'll update the patch in a bit, and file a new bug for landing in Desktop.
Summary: Convert webcompat extension away from bootstrap → Land Webcompat GoFaster webextension port in Fenenc
Attachment #9013248 - Attachment is obsolete: true
Attachment #9013361 - Attachment description: Bug 1451484 - Import WebExtension sources for the WebCompat GoFaster Addon. r=aswan,rhelmer → Bug 1451484 - Import WebExtension sources for the WebCompat GoFaster Addon. r=rhelmer
Attachment #9013361 - Attachment description: Bug 1451484 - Import WebExtension sources for the WebCompat GoFaster Addon. r=rhelmer → Bug 1451484 - Import WebExtension sources for the WebCompat GoFaster Addon to Fennec. r=rhelmer
I tried to understand what was going on here a few days. This patch tickles *something* such that the mochitest extension isn't correctly initialized. Given that we only see this in automation, I think we have a Web Extensions race somewhere. Sadly, the debug logging that I wanted turned on suggests that the "feature unpacking" code in Fennec is working just fine. Until we get somebody who can reproduce locally -- probably on a very slow ARM emulator, if my race hunch is correct -- there's not much I can do to help.
Thanks for taking a look Nick. As is, I guess we just lose this capability in 64 nightly and hope we can figure it out during the beta cycle. I'll circle back with ddurst about investigating the webext races (unless susheel has someone who can take a look first).
I also outlined a clumsier but perhaps simpler fix at https://bugzilla.mozilla.org/show_bug.cgi?id=1494657#c32
Amazing. Thanks Andrew, Luca and Geoff. Dennis, can we spin up a new try run once https://bugzilla.mozilla.org/show_bug.cgi?id=1494657#c37 is on m-c (or including that patch)?
Thanks everyone for the support here. :) Also thanks for the review, Robert. > Dennis, can we spin up a new try run We absolutely can! Here it goes: https://treeherder.mozilla.org/#/jobs?repo=try&revision=dee149b5d114a612ea5397b881f3578285bc1f30
Try is only showing some intermittents, the patch has r+, so let's check this in, so we have the Friday to work on anything unexpected (which I hope will be nothing, but one never knows...)
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/6aeee70256ad Import WebExtension sources for the WebCompat GoFaster Addon to Fennec. r=rhelmer
Backed out changeset 6aeee70256ad (Bug 1451484) for android robocop failures on testBrowserDiscovery. Backout: https://hg.mozilla.org/integration/autoland/rev/9a5870920ce33ae704c94615d6bc8ed43a7475da Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Csuccess%2Crunning%2Cpending%2Crunnable&revision=6aeee70256ad371b974717f50c400bc1091373d3&selectedJob=206551498 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=206551498&repo=autoland&lineNumber=1666
Thanks again Luca! I just had a look at another build (https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&selectedJob=206476364&revision=72b97e539421a38d16e843783c2359381bc6ac33) and since some of the intermittens looked the same, I figured these failures were just intermittents... :(
Hi geoff, it seems that a "SpecialPowers is not defined" issue similar to the one we just workarounded for the mochitest is also affecting the robocop testsuite and so the patch got backed out. I've added in comment 36 some details about the issue: this may have been a very infrequent intermittent (as for the mochitest one) which turns into a permafail once an additional extension (webcompat in this case) is added to the one that are starting up while the robocop webpage is being loaded (as it was happening for the mochitests). Do you mind to take a look and see if you think that we can proceed as we did for the similar mochitest issue? (basically, short-term warkaround + filing a followup issue on bugzilla to work on a more long-term solution). Thanks!
Thanks Luca. Yes, that looks fine to me. Land your fix with r=gbrown if you like. I've added a note to include robocop in bug 1499890 for the long-term solution.
As a side note: Luca, you are free to land my patch as well if you want to. :)
Attachment #9013361 - Attachment description: Bug 1451484 - Import WebExtension sources for the WebCompat GoFaster Addon to Fennec. r=rhelmer → Bug 1451484 - Import WebExtension sources for the WebCompat GoFaster Addon to Fennec. r=rhelmer,gbrown
As per discussion with :miketaylr, let's try to land this now. Since Luca told me he is already off for the weekend, I applied his patch on my branch and pushed the updated rev to Phabricator, carrying over the r=gbrown for the workaround.
Attachment #9013361 - Attachment is obsolete: true
https://hg.mozilla.org/integration/mozilla-inbound/rev/588d1e2f8c9d9f34ac3d3ac62960e1660fcc9afb Bug 1451484 - Import WebExtension sources for the WebCompat GoFaster Addon to Fennec. r=rhelmer,gbrown
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/ff70442b73b3 No bug - update code after eslint change from merge and remove files removed by 1451484 and restored during merge
3 years ago
Depends on: 1500924
You need to log in before you can comment on or make changes to this bug.