1735721 - Move addons-search-detection in tree

Status: VERIFIED FIXED

(WebExtensions :: General, task, P3)

Description

[William Durand [:willdurand]]

We'd like to move the addons-search-detection system add-on in tree (in this folder) to be able to: (1) write some tests, (2) avoid the signing/Balrog processes in the future.

We don't have much plans to integrate some of the features of the add-on in a better place yet so there is value in simply transferring most of the existing code into browsers/extensions/ and write tests (in a different bug probably).

:mixedpuppy said it was OK to do it because the code of the extension on GitHub has been reviewed by both :mixedpuppy and :robwu already. Some additional build-related files/configs will be needed but that should be it.

Comment 1

[William Durand [:willdurand]]

Note: we should create a built-in add-on.

Comment 2

[William Durand [:willdurand]]

Attachment: Bug 1735721 - Move addons-search-detection in tree. r?florian!,mixedpuppy,rpl

This patch moves the addons-search-detection system add-on in tree and
adjusts the build configuration to make it a built-in add-on (which
requires changes in the BrowserGlue.jsm file).

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b187514d31fc
Move addons-search-detection in tree. r=mixedpuppy

Comment 4

[Atila Butkovits]

Backed out for causing failures at browser_all_files_referenced.js.

Backout link: https://hg.mozilla.org/integration/autoland/rev/47cd6e2ed3184ec2001ea1255c03bd45c7fb7d18

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=JggHYHkVQA6jcd27oD7MYA.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=b187514d31fcba1b0943251c227cb405202a574c

Failure log: https://treeherder.mozilla.org/logviewer?job_id=355367382&repo=autoland&lineNumber=3562

Comment 5

[William Durand [:willdurand]]

Looks like the test case does not support all the files we bundle for this add-on. I talked to :rpl and he told me to fix the test case as opposed to exclude the built-in add-on in this test case so I am going to do that now.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/80d2e27ea03f
Move addons-search-detection in tree. r=rpl

Comment 7

[Iulian Moraru]

Backed out for causing mochitests browser chrome failures on browser_startup_syncIPC.js.

Push with failures

Failure log

Backout link

Comment 8

[William Durand [:willdurand]]

I updated the patch to fix the issue in browser_startup_syncIPC.js. Not sure exactly why this happened right now but :florian agreed to add an exception in the test file for this add-on.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/90f87be213ac
Move addons-search-detection in tree. r=rpl,florian

Comment 10

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/90f87be213ac

Comment 11

[Alex Cornestean]

Verified the patch on the latest Nightly 95 (95.0a1/20211021215643) under Windows 10 x64 and Ubuntu 16.04 LTS.

The “Add-ons Search Detection” add-on is now built-in Firefox. It’s listed in about:support under “Add-ons” with version 2.0.0, confirming the patch.

Comment 12

[Acasandrei Beatrice (needinfo me)]

Hello. This bug or Bug 1736825 has caused this regression. Can you please help with identifying which one might be the cause?

Comment 13

[Andra Esanu (needinfo me)]

Hi,
Could you please tell me if it makes sense for these improvements to be caused by this patch?

== Change summary for alert #32142 (as of Wed, 27 Oct 2021 14:41:10 GMT) ==

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
6%	build times	windows2012-32-shippable	nightly taskcluster-m5d.4xlarge	1,951.29 -> 1,840.96
4%	build times	windows2012-64-shippable	nightly taskcluster-m5.4xlarge	1,996.99 -> 1,923.03
4%	build times	osx-shippable	nightly taskcluster-c5n.4xlarge	2,617.88 -> 2,524.89

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=32142

Here is a link with all the alerts in the same graph
https://treeherder.mozilla.org/perfherder/graphs?highlightAlerts=1&highlightChangelogData=1&series=autoland,2741922,1,2&series=autoland,2310089,1,2&series=autoland,2734448,1,2&timerange=2592000&zoom=1634606732479,1634860649898,1587.2015054889448,2882.455655980151

Comment 14

[Shane Caraveo (:mixedpuppy)]

This added more to the build, so I don't think that this would cause an improvement.

Comment 15

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Shane Caraveo (:mixedpuppy) from comment #14)

This added more to the build, so I don't think that this would cause an improvement.

This appears to be the pushlog for the improvement range: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=fefc5982e76658e6d6dd49308c51dd8933542153&tochange=8de0d1e269d09e73d54b04213adce6ee0e5988ab.

Is there another revision that looks more likely to be the cause?

Comment 16

[Tomislav Jovanovic :zombie]

I would say the patch to upgrade the build system to clang-13 looks like the best candidate to affect build times:
https://hg.mozilla.org/integration/autoland/rev/dcb91b95f32bb7692169357bc6c8d1cfa81e34c1

Comment 17

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Tomislav Jovanovic :zombie from comment #16)

I would say the patch to upgrade the build system to clang-13 looks like the best candidate to affect build times:
https://hg.mozilla.org/integration/autoland/rev/dcb91b95f32bb7692169357bc6c8d1cfa81e34c1

Agreed, thanks!