1419495 - Remove SelfSupport service

Status: RESOLVED FIXED

(Toolkit :: Telemetry, enhancement, P3)

Description

[Georg Fritzsche [:gfritzsche]]

The selfsupport service was presumably only used by about:healthreport and UITour.
Both those usages might have gone away.
Let's double-check this and if confirmed, remove selfsupport.

https://searchfox.org/mozilla-central/search?q=selfsupport&case=false&regexp=false&path=

Comment 1

[Georg Fritzsche [:gfritzsche]]

Alessio, do you know if this is still used with UITour?

Comment 2

[Alessio Placitelli [:Dexter]]

(In reply to Georg Fritzsche [:gfritzsche] from comment #1)
> Alessio, do you know if this is still used with UITour?

Nope, it's not. I remember now :) We had two "SelfSupport" objects before: the one that powers about:healthreport and the one that powered heartbeat. The latter was used by UITour and was removed by bug 1366005. We should be good to remove the former in this bug.

Comment 3

[Clément DAVID [:davidcl]]

Attachment: bug_1419495.patch

Hi, I am a new contributor (not a mozillian yet) and want to fix this bug. Could you please review the attached patch ?

Comment 4

[Alessio Placitelli [:Dexter]]

(In reply to c.david86 from comment #3)
> Created attachment 8933940 [details] [diff] [review]
> bug_1419495.patch
> 
> Hi, I am a new contributor (not a mozillian yet) and want to fix this bug.
> Could you please review the attached patch ?

Hi and welcome! I've assigned this bug to you and I'm going to add the review request flag to your patch. Note that, without this flag, we might not see the review request :( To request a reviewer, when you upload your next patch, select the question mark from the "review" field and write the name of the reviewer there ;) It should suggest you one that's relevant to the code you're touching!

Comment 5

[Alessio Placitelli [:Dexter]]

Comment on attachment 8933940 [details] [diff] [review]
bug_1419495.patch

Review of attachment 8933940 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good to me, thanks! Let me push that to try for you to make sure nothing breaks.

Comment 6

[Alessio Placitelli [:Dexter]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6da75c2e3bd31b032b43ce836feda2803bb5ddf1

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Dexter, looks like you made a mistake in the try build ;)

Comment 8

[Alessio Placitelli [:Dexter]]

(In reply to Sylvestre Ledru [:sylvestre] from comment #7)
> Dexter, looks like you made a mistake in the try build ;)

Ah, thanks Sylvestre! I received the notification and spawned a new one but I forgot to post the new link :)

https://treeherder.mozilla.org/#/jobs?repo=try&revision=22e524c27bb4acfdbd188818e32ebe7206e4ee7f

Comment 9

[Dão Gottwald [:dao]]

pushing to ssh://hg.mozilla.org/integration/mozilla-inbound/
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 12 changes to 12 files
remote: 
remote: WebIDL file dom/webidl/MozSelfSupport.webidl altered in changeset d3b11b99875c without DOM peer review
remote: WebIDL file dom/webidl/Window.webidl altered in changeset d3b11b99875c without DOM peer review
remote: 
remote: 
remote: 
remote: ************************** ERROR ****************************
remote: 
remote: Changes to WebIDL files in this repo require review from a DOM peer in the form of r=...
remote: This is to ensure that we behave responsibly with exposing new Web APIs. We appreciate your understanding..
remote: 
remote: *************************************************************
remote: 
remote: 
remote: transaction abort!
remote: rollback completed
remote: pretxnchangegroup.d_webidl hook failed
abort: push failed on remote

Comment 10

[Alessio Placitelli [:Dexter]]

Comment on attachment 8933940 [details] [diff] [review]
bug_1419495.patch

:smaug, this patch removes the MozSelfSupport webidl, as the about:healthreport page went away and that API has no consumers anymore (and it won't have consumers in the future).

Since this change requires the review from a DOM peer (commit hook says so!), would you mind reviewing it?

Comment 11

[Clément DAVID [:davidcl]]

(In reply to Alessio Placitelli [:Dexter] from comment #4)
> Hi and welcome! I've assigned this bug to you and I'm going to add the
> review request flag to your patch. Note that, without this flag, we might
> not see the review request :(

Thanks for the tip ! About the patch, the author name is 'Clément David' UTF-8 encoded whereas treeherder render it as "Clément David" (as ISO 8859 chars). Should I update the patch to use ISO 8859 (or plain ASCII) ?

Comment 12

[Dão Gottwald [:dao]]

(In reply to Clément DAVID [:davidcl] from comment #11)
> (In reply to Alessio Placitelli [:Dexter] from comment #4)
> > Hi and welcome! I've assigned this bug to you and I'm going to add the
> > review request flag to your patch. Note that, without this flag, we might
> > not see the review request :(
> 
> Thanks for the tip ! About the patch, the author name is 'Clément David'
> UTF-8 encoded whereas treeherder render it as "Clément David" (as ISO 8859
> chars). Should I update the patch to use ISO 8859 (or plain ASCII) ?

The encoding should be fine as-is. I believe this is a problem on Alessio's side, I can import your patch just fine and the user field reads "Clément David" as expected.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a334c14134d2
Remove SelfSupport service r=dexter r=smaug

Comment 14

[Raul Gurzau (:RaulG)]

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