959356 - org.mozilla.sysinfo.sysinfo's "architecture" property returns "x86" for 32-bit Firefox build running on Win64 OS

Status: VERIFIED FIXED

(Firefox Health Report Graveyard :: Client: Desktop, defect)

Description

[Chris Peterson [:cpeterson]]

I'm trying to determine what percentage of Firefox Windows users are running the 32-bit Firefox build on Win64 OS. The Mozilla Metrics team gave me some FHR data that claimed 99.9% of Firefox Windows users ran Win32 OS. In contrast, Valve's Steam user stats [1] claim 72% run Win64 (though Valve's gamers are not representative of average Firefox
users).

The FHR docs say org.mozilla.sysinfo.sysinfo's "architecture" property is the "OS architecture" [2], but AFAICT, "architecture" maps to NSPR's _PR_SI_ARCHITECTURE #define [3], which is a compile-time constant describing the target archicture of the Firefox build itself. So 32-bit Firefox running on Win64 will still return "x86", not "x86-64". 


[1] http://store.steampowered.com/hwsurvey/
[2] https://docs.services.mozilla.com/healthreport/index.html#org-mozilla-sysinfo-sysinfo
[3] https://mxr.mozilla.org/mozilla-central/source/nsprpub/pr/include/md/_winnt.h#36

Comment 1

[Benjamin Smedberg]

Yeah, this looks like a bug. We already have the current running arch of Firefox via geckoAppInfo.xpcomabi.

cpeterson, I can give estimated data via crash-stats, although if x86 Windows crashes more or less than x86-64 windows that could skew your stats somewhat.

Comment 2

[Benjamin Smedberg]

Note that we seem to know about this already, and nsSystemInfo exposes an "isWow64" property specifically for this, which we do expose in telemetry (see bug 838279). So telemetry, if the data is good enough, would tell you this also.

Comment 3

[Gregory Szorc [:gps]]

Attachment: Report isWow64 in Firefox Health Report

Previously, we weren't measuring whether the machine was 64-bit
properly. This change allows us to report actual CPU architecture (at
least on Windows).

In the future, we may wish to properly report actual CPU architecture on
all operating systems.

Comment 4

[Chris Peterson [:cpeterson]]

If you are going to add a new `isWow64` property to org.mozilla.sysinfo.sysinfo, you should update the docs to describe `isWow64` and clarify that the `architecture` field is not "OS architecture", as it is currently described.

https://docs.services.mozilla.com/healthreport/index.html#org-mozilla-sysinfo-sysinfo

Comment 5

[Gregory Szorc [:gps]]

I have every intention of updating the docs if this patch sticks.

That reminds me, we have in-tree docs now. I should move the content in docs.services into the tree.

Comment 6

[Benjamin Smedberg]

Comment on attachment 8361799 [details] [diff] [review]
Report isWow64 in Firefox Health Report

Since this is basically backward-compatible, should version be 1.1 instead of 2? I don't know if we have an actual versioning policy for any of this.

Comment 7

[Gregory Szorc [:gps]]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #6)
> Comment on attachment 8361799 [details] [diff] [review]
> Report isWow64 in Firefox Health Report
> 
> Since this is basically backward-compatible, should version be 1.1 instead
> of 2? I don't know if we have an actual versioning policy for any of this.

Versions are integers. When designed, we just wanted a mechanism that indicated any semantic change. We didn't want to worry about major vs minor foo.

Comment 8

[Gregory Szorc [:gps]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/22c4e8746c69

I added docs to the patch without review. We can always clarify the docs later if they aren't good enough.

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/22c4e8746c69

Comment 10

[Benjamin Smedberg]

This doesn't appear to work. Both jchen and I tried this against about:healthreport today and there is no isWow64 marking. I ran the following in the browser console though:

> si = Cc["@mozilla.org/system-info;1"].getService(Ci.nsIPropertyBag2);
XPCWrappedNative_NoHelper { QueryInterface: QueryInterface(), enumerator: Getter, getProperty: getProperty(), getPropertyAsInt32: getPropertyAsInt32(), getPropertyAsUint32: getPropertyAsUint32(), getPropertyAsInt64: getPropertyAsInt64(), getPropertyAsUint64: getPropertyAsUint64(), getPropertyAsDouble: getPropertyAsDouble(), getPropertyAsAString: getPropertyAsAString(), getPropertyAsACString: getPropertyAsACString(), 5 more… }
> si.get("isWow64")
true
> si.getProperty("isWow64")
true

I think this is because of the casing at isWOW64: "isWow64", where they both need to be isWow64.

Comment 11

[Benjamin Smedberg]

Marco please add to the current iteration.

Comment 12

[Marco Mucci [:MarcoM]]

Added to iteration 32.2

Comment 13

[Benjamin Smedberg]

Attachment: isWow64-959356

Comment 14

[Benjamin Smedberg]

Firefox product really wants these as reliable numbers, so I'm going to try and get this in for FF30. Marking tracking appropriately.

Comment 15

[Richard Newman [:rnewman]]

Comment on attachment 8424974 [details] [diff] [review]
isWow64-959356

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

Matches nsSystemInfo.cpp, so lgtm.

Comment 16

[Lukas Blakk [:lsblakk] use ?needinfo]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #14)
> Firefox product really wants these as reliable numbers, so I'm going to try
> and get this in for FF30. Marking tracking appropriately.

Will want to target this week (or early next) in that case as we're at Beta 6 tomorrow morning, next Monday's beta will be the last chance for this to land if low enough risk to take.

Comment 17

[Benjamin Smedberg]

https://hg.mozilla.org/integration/fx-team/rev/6399ede3f82f

Comment 18

[Benjamin Smedberg]

Comment on attachment 8424974 [details] [diff] [review]
isWow64-959356

[Approval Request Comment]
Bug caused by (feature/regressing bug #): This bug didn't work
User impact if declined: Need this to measure the impact of a possible Firefox for win64
Testing completed (on m-c, etc.): landed and verified via local testing
Risk to taking this patch (and alternatives if risky): low risk
String or IDL/UUID changes made by this patch: none

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6399ede3f82f

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/09da70ff07b8
https://hg.mozilla.org/releases/mozilla-beta/rev/ff1925aa6f85

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/ff1925aa6f85

Comment 22

[Petruta Horea [:phorea], Desktop QA]

Mozilla/5.0 (Windows NT 6.3; WOW64; rv:30.0) Gecko/20100101 Firefox/30.0 - Fx 30 beta 7
Mozilla/5.0 (Windows NT 6.3; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0 - 20140522004003 Aurora
Mozilla/5.0 (Windows NT 6.3; WOW64; rv:32.0) Gecko/20100101 Firefox/32.0 - 20140522030204 Nightly 

Verified that on Win 8.1 64-bit, the 32-bit Firefox builds are correctly marked with "isWow64": 1 in about:healthreport:

org.mozilla.sysinfo.sysinfo": {
        "_v": 2,
        "isWow64": 1,
        "architecture": "x86",
        "name": "Windows_NT",
        "version": "6.3"
      }

Is it correct that for 64-bit Firefox builds "isWow64" is 0? I found this for Nightly 2014-05-22 on Win 8.1 and Win 7 64-bit.

I mark this bug as verified and I will open a new one if a follow-up is needed.

Comment 23

[Benjamin Smedberg]

Yes, that is correct.