Closed Bug 576507 Opened 14 years ago Closed 14 years ago

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

Categories

(Core :: IPC, defect)

Product:
Core
Component:
IPC
Platform:
All
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: starkov.egor, Assigned: cjones)

References

Details

Attachments

(3 files)

call stack
9.75 KB, text/plain
Details
Temporarily weaken an assertion until a plan is made
1.23 KB, patch
jdm
: review+
Details | Diff | Splinter Review
Forward the selected locale to content process to satisfy callers that want "some locale"
8.16 KB, patch
jdm
: review+
benjamin
: superreview+
mfinkle
: approval2.0+
Details | Diff | Splinter Review
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
Stack wanted, we need to fix the caller I think.
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.
Attached 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>
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
Closed: 14 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.

Attachment

General

Creator:
Created:
Updated:
Size: