New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 821290 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner: ----
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug



Sign in to add a comment

WarmupWindowsLocales in sandbox/win/src/target_services.cc can now call GetUserDefaultLocaleName directly

Reported by bo...@mozilla.com, Mar 13 2018

Issue description

UserAgent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/64.0.3282.186 Safari/537.36

Steps to reproduce the problem:
GetUserDefaultLocaleName was not available on Windows XP, so it is loaded and called at runtime in WarmupWindowsLocales.

It can now be called directly as Windows XP is no longer supported.

What is the expected behavior?

What went wrong?
In the Firefox copy of this code we've had a small number of crashes related to the dynamic calling of this.

Did this work before? N/A 

Chrome version: 64.0.3282.186  Channel: n/a
OS Version: 6.1 (Windows 7, Windows Server 2008 R2)
Flash Version:
 
Labels: Needs-Triage-M64
Components: Internals>Sandbox
Labels: Triaged-ET TE-NeedsTriageHelp
The issue seems to be out of TE-scope as it is related to .cc files and code. Hence, adding label TE-NeedsTriageHelp for further investigation from dev team.

Thanks...!!
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 18 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/97d0c581ae51cb117d875b2a44f435a9e8f63ff0

commit 97d0c581ae51cb117d875b2a44f435a9e8f63ff0
Author: Bob Owen <bowen@mozilla.com>
Date: Wed Apr 18 16:40:04 2018

Remove dynamic load and call for GetUserDefaultLocaleName

This was only required because it is not available on Windows XP, which is no
longer supported.

R=forshaw@chromium.org

Bug:  821290 
Change-Id: Ie5fba51391b25f7a1a01549f67a054c00aefa50f
Reviewed-on: https://chromium-review.googlesource.com/1012119
Reviewed-by: James Forshaw <forshaw@chromium.org>
Commit-Queue: James Forshaw <forshaw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551715}
[modify] https://crrev.com/97d0c581ae51cb117d875b2a44f435a9e8f63ff0/AUTHORS
[modify] https://crrev.com/97d0c581ae51cb117d875b2a44f435a9e8f63ff0/sandbox/win/src/target_services.cc

Comment 5 by wfh@chromium.org, May 15 2018

Cc: liamjm@chromium.org
hmm I thought this was needed for CSRSS warmup? liam? if not, please go ahead and mark this bug fixed.

Comment 6 by liamjm@chromium.org, May 15 2018

This is correct. It was only loaded dynamically because of WinXP. This approach can be removed, as per the commit.

I am interested to know more about why it failed.
bowen@mozilla.com: can you comment on this?

Comment 7 by liamjm@chromium.org, May 15 2018

Status: Fixed (was: Unconfirmed)

Comment 8 by bo...@mozilla.com, May 16 2018

I'm afraid I couldn't see why this was failing.

Possibly interference from third party DLLs, although no concrete evidence.
James also noticed that it was passing the wrong length, but the problem appeared to be as we called the function (with an invalid pointer, possibly returned from GetProcAddress), so I don't see how that could have caused our crashes.

Sign in to add a comment