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

RESOLVED FIXED in Firefox 55

Status

()

P1
normal
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: mythmon, Assigned: mythmon)

Tracking

Trunk
Firefox 55
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [photon-performance])

Attachments

(1 attachment)

(Assignee)

Description

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

Updated

2 years ago
Blocks: 1360874
Yay! Can UITour.showHeartbeat also be removed or is that used by SHIELD?

Comment 2

2 years ago
(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

Updated

2 years ago
Blocks: 1363155
Flags: qe-verify?
Priority: -- → P2
Whiteboard: [photon-performance]
(Assignee)

Comment 3

2 years ago
(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?
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.
(Assignee)

Updated

2 years ago
Blocks: 1366005
(Assignee)

Comment 5

2 years ago
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.
Assignee: nobody → mcooper
Status: NEW → ASSIGNED
Priority: P2 → P1
Comment hidden (mozreview-request)

Comment 7

2 years ago
mozreview-review
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).
Attachment #8869097 - Flags: review?(gijskruitbosch+bugs) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 10

2 years ago
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)
(Assignee)

Comment 11

2 years ago
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 hidden (mozreview-request)

Comment 13

2 years ago
Pushed by mcooper@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5a8f2dcbeac0
Remove SelfSupport, since it is obsoleted by shield-recipe-client. r=Gijs

Comment 14

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/5a8f2dcbeac0
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Iteration: --- → 55.6 - May 29
Flags: qe-verify? → qe-verify-
You need to log in before you can comment on or make changes to this bug.