1361578 - Remove SelfSupport, since it is obsoleted by shield-recipe-client

Status: RESOLVED FIXED

(Firefox :: General, enhancement, P1)

Description

[Michael Cooper [:mythmon]]

With bug 1351454, we are have now shipped shield-recipe-client, the replacement for SelfSupport. We should remove SelfSupport now, to avoid dead code.

Comment 1

[Matthew N. [:MattN]]

Yay! Can UITour.showHeartbeat also be removed or is that used by SHIELD?

Comment 2

[:Gijs (he/him)]

(In reply to Matthew N. [:MattN] (behind on bugmail; PM if requests are blocking you) from comment #1)
> Yay! Can UITour.showHeartbeat also be removed or is that used by SHIELD?

SHIELD has its own implementation - https://dxr.mozilla.org/mozilla-central/source/browser/extensions/shield-recipe-client/lib/Heartbeat.jsm

Comment 3

[Michael Cooper [:mythmon]]

(In reply to Matthew N. [:MattN] (PM if requests are blocking you) from comment #1)
> Yay! Can UITour.showHeartbeat also be removed or is that used by SHIELD?

I don't see any other usages of showHeartbeat in m-c or on Github (since we exposed this to some Mozilla websites). I think it would be safe to remove. Should we do that in this bug, or should we file another?

Comment 4

[Matthew N. [:MattN]]

Either way is fine with me since it was added for self-support in the first place. Maybe a separate commit would be good though.

Comment 5

[Michael Cooper [:mythmon]]

I started to remove UITour.showHeartbeat, and I found that UITour provides some CSS for heartbeat that Shield uses. Because of this, I filed bug 1366006 to move that CSS over to Shield, and bug 1366005 to remove UITour.showHeartbeat which depends on it.

Comment 6

[Michael Cooper [:mythmon]]

Attachment: Bug 1361578 - Remove SelfSupport, since it is obsoleted by shield-recipe-client.

Review commit: https://reviewboard.mozilla.org/r/140722/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/140722/

Comment 7

[:Gijs (he/him)]

Comment on attachment 8869097 [details]
Bug 1361578 - Remove SelfSupport, since it is obsoleted by shield-recipe-client.

https://reviewboard.mozilla.org/r/140722/#review144552

r=me, without the changes to runtime json files.

Also note that the trypush is showing consistent xpcshell orange in browser/components/newtab/tests/xpcshell/test_NewTabSearchProvider.js , because it's trying to contact the location service... I have no idea why, off-hand, but obviously we shouldn't turn the tree orange when landing this. I don't see how it could be related to this patch, but equally the trypush was directly on top of an m-c rev that didn't have these oranges, as far as I can tell...

::: testing/runtimes/mochitest-browser-chrome-e10s.runtimes.json:2
(Diff revision 1)
>  {
> -  "excluded_test_average": 1310, 
> +  "excluded_test_average": 1310,

See comment for next file - I don't think this needs updating.

::: testing/runtimes/mochitest-browser-chrome.runtimes.json:2
(Diff revision 1)
>  {
> -  "excluded_test_average": 943, 
> +  "excluded_test_average": 943,

I don't think you need to update this file. We seem to be updating this in batches, cf. bug 1324047. I don't even know if removing all the spaces will break some kind of parser, so I'd prefer we don't touch it, or, if we really need to, only remove the entries we're removing here (which I don't think is strictly necessary).

Comment 8

[Michael Cooper [:mythmon]]

Comment on attachment 8869097 [details]
Bug 1361578 - Remove SelfSupport, since it is obsoleted by shield-recipe-client.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/140722/diff/1-2/

Comment 9

[Michael Cooper [:mythmon]]

Comment on attachment 8869097 [details]
Bug 1361578 - Remove SelfSupport, since it is obsoleted by shield-recipe-client.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/140722/diff/2-3/

Comment 10

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 9ccc52e14e98 -d c219c4a401b0: rebasing 396921:9ccc52e14e98 "Bug 1361578 - Remove SelfSupport, since it is obsoleted by shield-recipe-client. r=Gijs" (tip)
merging addon-sdk/source/python-lib/cuddlefish/prefs.py
merging addon-sdk/source/test/preferences/no-connections.json
merging browser/app/profile/firefox.js
merging layout/tools/reftest/reftest-preferences.js
merging testing/talos/talos/config.py
merging testing/xpcshell/head.js
warning: conflicts while merging addon-sdk/source/python-lib/cuddlefish/prefs.py! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 11

[Michael Cooper [:mythmon]]

I reverted the changes to the runtime json files. I hadn't meant to remove the spaces, just the test lines. My editor helpfully stripped the trailing spaces. I'll have to have a talk with it.

The test failure in browser/components/newtab/tests/xpcshell/test_NewTabSearchProvider.js was caused by a typo I made in testing/xpcshell/head.js. I had changed a "pref.setCharPref" into "ref.setCharPref". That section of the code is surrounded in try { ... } catch (e) {}, so it hid the error. A pref that is set a few lines later is prefs.setCharPref("browser.search.geoip.url", "https://%(server)s/geoip-dummy"). That explains the error pretty well, and in my local testing fixes the test.

Comment 12

[Michael Cooper [:mythmon]]

Comment on attachment 8869097 [details]
Bug 1361578 - Remove SelfSupport, since it is obsoleted by shield-recipe-client.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/140722/diff/3-4/

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5a8f2dcbeac0
Remove SelfSupport, since it is obsoleted by shield-recipe-client. r=Gijs

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/5a8f2dcbeac0