Currently we have DISPLAY_SCALING_OSX, DISPLAY_SCALING_MSWIN, DISPLAY_SCALING_LINUX. We should just use DISPLAY_SCALING and do a proper per-OS breakdown as usual.

This patch builds fine but when running it I can't find the new histogram in about:telemetry...

:Dolske are you still using this histogram ?

It has been a while since this bug was filed. This is useful, as OS breakdowns can be done server-side in better ways. However, the users/owners of these histograms should decide if we can just replace them.

Comment on attachment 8848479 [details] Bug 1121966 - Change DISPLAY_SCALING_<OS> histograms to a single https://reviewboard.mozilla.org/r/121376/#review123866 If these histograms are not used anymore, we should just remove them. If they are still moved and its ok to change them into one histogram, then we can improve this a bit. ::: toolkit/components/telemetry/histogram-whitelists.json:208 (Diff revision 1) > "DEVTOOLS_WEBIDE_REMOTE_CONNECTION_RESULT", > "DEVTOOLS_WEBIDE_SIMULATOR_CONNECTION_RESULT", > "DEVTOOLS_WEBIDE_TIME_ACTIVE_SECONDS", > "DEVTOOLS_WEBIDE_USB_CONNECTION_RESULT", > "DEVTOOLS_WEBIDE_WIFI_CONNECTION_RESULT", > - "DISPLAY_SCALING_LINUX", > + "DISPLAY_SCALING", We should not add anything to the whitelist here. Instead we should set proper attributes for the histogram.

The data is still useful, and I have no objection to removing the OS-specific probes.

(In reply to Georg Fritzsche [:gfritzsche] [away March 24 - April 2] from comment #5) > We should not add anything to the whitelist here. > Instead we should set proper attributes for the histogram. I pushed those changes :).

Chris, can you take a look?

Comment on attachment 8848479 [details] Bug 1121966 - Change DISPLAY_SCALING_<OS> histograms to a single https://reviewboard.mozilla.org/r/121376/#review134860 Does it pass lint? ::: browser/components/nsBrowserGlue.js:875 (Diff revision 2) > - case "linux": > - SCALING_PROBE_NAME = "DISPLAY_SCALING_LINUX"; > - break; > - } > - if (SCALING_PROBE_NAME) { > let scaling = aWindow.devicePixelRatio * 100; Isn't the indentation off here?

Comment on attachment 8848479 [details] Bug 1121966 - Change DISPLAY_SCALING_<OS> histograms to a single https://reviewboard.mozilla.org/r/121376/#review134860 ``` 0 fly@Minotoor ~/workspace/mozilla-central $ ./mach lint browser/components/nsBrowserGlue.js ✖ 0 problems (0 errors, 0 warnings) ``` But you are right there was an indentation problem there :).

Comment on attachment 8848479 [details] Bug 1121966 - Change DISPLAY_SCALING_<OS> histograms to a single https://reviewboard.mozilla.org/r/121376/#review134910

Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5c16d58e5f23 Change DISPLAY_SCALING_<OS> histograms to a single r=chutten