Closed Bug 959356 Opened 9 years ago Closed 8 years ago

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

Categories

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

All
Windows 8.1
defect
Not set
normal

Tracking

(firefox29 wontfix, firefox30+ verified, firefox31+ verified, firefox32 verified)

VERIFIED FIXED
Firefox 32
Tracking Status
firefox29 --- wontfix
firefox30 + verified
firefox31 + verified
firefox32 --- verified

People

(Reporter: cpeterson, Assigned: benjamin)

References

Details

(Whiteboard: p=1 s=it-32c-31a-30b.2 [qa!])

Attachments

(2 files)

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
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.
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.
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.
Attachment #8361799 - Flags: review?(benjamin)
Assignee: nobody → gps
Status: NEW → ASSIGNED
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
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 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.
Attachment #8361799 - Flags: review?(benjamin) → review+
(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.
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.
https://hg.mozilla.org/mozilla-central/rev/22c4e8746c69
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Marco please add to the current iteration.
Assignee: gps → benjamin
Flags: needinfo?(mmucci)
Flags: firefox-backlog+
Whiteboard: p=1 [qa+]
Target Milestone: Firefox 29 → Firefox 32
Added to iteration 32.2
Status: REOPENED → ASSIGNED
Flags: needinfo?(mmucci)
Whiteboard: p=1 [qa+] → p=1 s=it-32c-31a-30b.2 [qa+]
Attached patch isWow64-959356Splinter Review
Attachment #8424974 - Flags: review?(rnewman)
Firefox product really wants these as reliable numbers, so I'm going to try and get this in for FF30. Marking tracking appropriately.
Comment on attachment 8424974 [details] [diff] [review]
isWow64-959356

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

Matches nsSystemInfo.cpp, so lgtm.
Attachment #8424974 - Flags: review?(rnewman) → review+
(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 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
Attachment #8424974 - Flags: approval-mozilla-beta?
Attachment #8424974 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/6399ede3f82f
Status: ASSIGNED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Attachment #8424974 - Flags: approval-mozilla-beta?
Attachment #8424974 - Flags: approval-mozilla-beta+
Attachment #8424974 - Flags: approval-mozilla-aurora?
Attachment #8424974 - Flags: approval-mozilla-aurora+
QA Contact: petruta.rasa
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.
Status: RESOLVED → VERIFIED
Whiteboard: p=1 s=it-32c-31a-30b.2 [qa+] → p=1 s=it-32c-31a-30b.2 [qa!]
Yes, that is correct.
Product: Firefox Health Report → Firefox Health Report Graveyard
You need to log in before you can comment on or make changes to this bug.