Add support for getting locale from content process (or forbid it and fix callers)

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: starkov.egor, Assigned: cjones)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

Reporter

Description

9 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.19) Gecko/2010031218 Firefox/3.5
Build Identifier: 

When loading http://bash.org.ru/rss using Yummy extension I get assertion from this code:
NS_IMETHODIMP
nsChromeRegistryContent::GetSelectedLocale(const nsACString& aPackage,
                                           nsACString& aLocale)
{
  CONTENT_NOT_IMPLEMENTED();
}

chrome/src/nsChromeRegistryContent.cpp +245

Also this assertion was mentioned here https://bugzilla.mozilla.org/show_bug.cgi?id=536273#c18, when loading gmail.com

Reproducible: Always

Comment 1

9 years ago
Stack wanted, we need to fix the caller I think.

Comment 3

9 years ago
Why is *that* running in the content process!?
(In reply to comment #3)
> Why is *that* running in the content process!?

That code is not running in the content process
(In reply to comment #2)
> It is coming from:
> http://mxr.mozilla.org/mobile-browser/source/chrome/content/preferences.js#132

That was last assumption that I've heard from Egor... but now he is away somewhere deep in Siberia :).

Probably it is easier wait for 2 weeks, or install yummy and reproduce it.
Posted file call stack
I can reproduce this with test-ipc.xul running the following 2 test cases:

1.
<!DOCTYPE HTML PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "DTD/xhtml1-strict.dtd">
<html>
  <head></head>
  <body></body>
</html>

2. test.js has one simple statement 'var a = 1;'
<html>
<head></head>
<body>
  <script type="text/javascript" src="test.js"></script>
</body>
</html>

Comment 8

9 years ago
Is this still happening?
Yes, I still see it with test-ipx.cul
Status: UNCONFIRMED → NEW
Ever confirmed: true
Morphing this bug into a TODO item.

In addition to the caller in bug 617543, we also have

0 anonymous() ["file:///home/cjones/mozilla/ff-dbg/dist/bin/components/nsHandlerService.js":173]
    currentLocale = undefined
    chromeRegistry = [xpconnect wrapped nsIXULChromeRegistry @ 0x2957f80 (native @ 0x279b010)]
    this = [object Object]
1 anonymous() ["file:///home/cjones/mozilla/ff-dbg/dist/bin/components/nsHandlerService.js":206]
    version = undefined
    this = [object Object]
2 HS__updateDB() ["file:///home/cjones/mozilla/ff-dbg/dist/bin/components/nsHandlerService.js":146]
    defaultHandlersVersion = undefined
    this = [object Object]
3 HS__init() ["file:///home/cjones/mozilla/ff-dbg/dist/bin/components/nsHandlerService.js":141]
    this = [object Object]
4 HandlerService() ["file:///home/cjones/mozilla/ff-dbg/dist/bin/components/nsHandlerService.js":115]
    this = [object Object]
5 anonymous(iid = {53f0ad17-ec62-46a1-adbc-efccc06babcd}, outer = null) ["resource://gre/modules/XPCOMUtils.jsm":242]
    this = [object Object]

The call in bug 617543 seems legitimate, except that the architecture for fetching dictionaries is wrong (would require fs access from child process).  I don't know whether this caller is legitimate.

Informing the content processes of the locale is easy, and possibly orthogonal to fixing these callers.  For now, I'm going to weaken this assertion to unblock bug 615386, but leave in a FIXME.
Summary: e10s: Assertion in nsChromeRegistryContent::GetSelectedLocale while loading page → Add support for getting locale from content process (or forbid it and fix callers)
Patch is just to unblock bug 615386.  (Plus we were already ignoring this assertion.)
Attachment #496370 - Flags: review?(josh)
Attachment #496370 - Flags: review?(josh) → review+
This patch is a stop-gap measure to satisfy the two callers of GetSelectedLocale("global") I know of.  In both cases, the callers seem to be using GetSelectedLocale("global") to mean, "get some locale close to the user's that I can use for stuff."

In both cases though, we'll eventually need to fix the callers, I suspect.  We definitely need to fix the spellchecker, and I don't know what nsHandlerService is but I bet we need to fix that too ;).

feedback? here because I'm way out of my element.  Verified that this passes the assertion checks in reftest-ipc.
Attachment #496430 - Flags: feedback?(benjamin)
Attachment #496430 - Flags: feedback?(benjamin) → review?(benjamin)
Comment on attachment 496430 [details] [diff] [review]
Forward the selected locale to content process to satisfy callers that want "some locale"

Oops wrong flags.
Attachment #496430 - Flags: superreview?(benjamin)
Attachment #496430 - Flags: review?(josh)
Attachment #496430 - Flags: review?(benjamin)
Comment on attachment 496430 [details] [diff] [review]
Forward the selected locale to content process to satisfy callers that want "some locale"

Looks good to me.
Attachment #496430 - Flags: review?(josh) → review+
FTR, this bug indirectly blocks bug 623613.
Comment on attachment 496430 [details] [diff] [review]
Forward the selected locale to content process to satisfy callers that want "some locale"

Hrmph. In some ways I'd almost prefer to simply return NS_ERROR_NOT_IMPLEMENTED, rather than returning data which will sometimes but not always be correct. r=me either way, I guess.
Attachment #496430 - Flags: superreview?(benjamin) → superreview+
Comment on attachment 496430 [details] [diff] [review]
Forward the selected locale to content process to satisfy callers that want "some locale"

This should remove the constant errors we get in Fennec when loading web pages
Attachment #496430 - Flags: approval2.0? → approval2.0+
http://hg.mozilla.org/mozilla-central/rev/95f32675752f
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Assignee: nobody → jones.chris.g
You need to log in before you can comment on or make changes to this bug.