1352343 - Bind LocaleService to react to OSPreferences' `intl:system-locales-changed`

Assignee

Description

•

7 years ago

Android is at the moment the only platform where do locale renegotiation on system locales change, but we'll want all platforms to do this.

We're going to clean up how we retrieve OS locales (bug 1337078) and hopefully trigger this event, so the other side of the equation is to make LocaleService listen to it.

Zibi Braniecki [:zbraniecki][:gandalf]

Assignee

Updated

•

7 years ago

Assignee: nobody → gandalf

Status: NEW → ASSIGNED

Zibi Braniecki [:zbraniecki][:gandalf]

Assignee

Updated

•

7 years ago

Depends on: 1337078

Comment hidden (mozreview-request)

Review commit: https://reviewboard.mozilla.org/r/125412/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/125412/

Makoto Kato [:m_kato] (PTO until 5/6)

Updated

•

7 years ago

Priority: -- → P3

Zibi Braniecki [:zbraniecki][:gandalf]

Assignee

Updated

•

7 years ago

Blocks: 1346877

Richard Newman [:rnewman]

Updated

•

7 years ago

Updated

•

7 years ago

Blocks: 1337078

No longer depends on: 1337078

Comment hidden (mozreview-request)

Comment on attachment 8853323 [details]
Bug 1352343 - Bind LocaleService to react to OSPreferences `intl:system-locales-changed`.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/125412/diff/2-3/

Jonathan Kew [:jfkthame]

Comment 4

•

7 years ago

mozreview-review

Comment on attachment 8853323 [details]
Bug 1352343 - Bind LocaleService to react to OSPreferences `intl:system-locales-changed`.

https://reviewboard.mozilla.org/r/125412/#review193136

This looks OK, but while reading it over, I stumbled across something that I think you should consider changing at the same time (it relates directly to the patch here). Namely, I think the method LocaleService::OnRequestedLocalesChanged is misnamed, and makes it unnecessarily confusing to follow the interdependencies here.

We have a notification intl:requested-locales-changed which is used to inform any interested observers that they might need to update locale-dependent stuff. The confusion here arises because OnRequestedLocalesChanged sounds like a method called in -response- to this notification (the "On..." name), but in fact it is responsible for -sending- it, not responding to it.

So I think LocaleService::OnRequestedLocalesChanged would be better without the "On" prefix, or perhaps even renamed to something like LocalePreferencesChanged, as it is used within the locale service to handle changes in prefs that affect locale negotiation; it then -sends- the intl:requested-locales-changed message to inform the world that a change has happened.

Attachment #8853323 - Flags: review?(jfkthame) → review+

Zibi Braniecki [:zbraniecki][:gandalf]

Assignee

Comment 5

•

7 years ago

Thank you! Would you be ok with me changing the name in this patch?

Flags: needinfo?(jfkthame)

Jonathan Kew [:jfkthame]

Comment 6

•

7 years ago

Yes, fine with me; it should be a simple global replacement, with no change in behavior.

Flags: needinfo?(jfkthame)

Comment hidden (mozreview-request)

Review commit: https://reviewboard.mozilla.org/r/188082/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/188082/

Comment hidden (mozreview-request)

Comment on attachment 8853323 [details]
Bug 1352343 - Bind LocaleService to react to OSPreferences `intl:system-locales-changed`.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/125412/diff/3-4/

Zibi Braniecki [:zbraniecki][:gandalf]

Assignee

Comment 9

•

7 years ago

mozreview-review

Comment on attachment 8917071 [details]
Bug 1352343 - Rename LocaleService::On* methods to LocaleService::*.

https://reviewboard.mozilla.org/r/188082/#review193234

r+ as per jfkthame's comment

Attachment #8917071 - Flags: review+

Jonathan Kew [:jfkthame]

Comment 10

•

7 years ago

mozreview-review

Comment on attachment 8917071 [details]
Bug 1352343 - Rename LocaleService::On* methods to LocaleService::*.

https://reviewboard.mozilla.org/r/188082/#review193282

Attachment #8917071 - Flags: review?(jfkthame) → review+

Pulsebot

Comment 11

•

7 years ago

Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0699fcfd990f
Bind LocaleService to react to OSPreferences `intl:system-locales-changed`. r=jfkthame
https://hg.mozilla.org/integration/autoland/rev/279dc744683e
Rename LocaleService::On* methods to LocaleService::*. r=jfkthame

Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)

Comment 12

•

7 years ago

Backed out for build bustage:

https://hg.mozilla.org/integration/autoland/rev/89761af55e553fe583370bf5e0101ce80a3c3520
https://hg.mozilla.org/integration/autoland/rev/15751b4cd7c9c3d710d817b4b4eb211c5d91e5fe

Push with bustage: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=279dc744683e449887790e910283980e64c0a2c8&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Build log: https://treeherder.mozilla.org/logviewer.html#?job_id=136017526&repo=autoland

[task 2017-10-10T19:58:55.135Z] 19:58:55     INFO - ==================== 48 passed, 3 skipped in 18.37 seconds =====================
[task 2017-10-10T19:59:30.389Z] 19:59:30     INFO - Exception in thread ProcessReader:
[task 2017-10-10T19:59:30.389Z] 19:59:30     INFO - Traceback (most recent call last):
[task 2017-10-10T19:59:30.389Z] 19:59:30     INFO -   File "/usr/lib/python2.7/threading.py", line 810, in __bootstrap_inner
[task 2017-10-10T19:59:30.389Z] 19:59:30     INFO -     self.run()
[task 2017-10-10T19:59:30.390Z] 19:59:30     INFO -   File "/usr/lib/python2.7/threading.py", line 763, in run
[task 2017-10-10T19:59:30.390Z] 19:59:30     INFO -     self.__target(*self.__args, **self.__kwargs)
[task 2017-10-10T19:59:30.390Z] 19:59:30     INFO -   File "/builds/worker/workspace/build/src/testing/mozbase/mozprocess/mozprocess/processhandler.py", line 985, in _read
[task 2017-10-10T19:59:30.390Z] 19:59:30     INFO -     callback(line.rstrip())
[task 2017-10-10T19:59:30.390Z] 19:59:30     INFO -   File "/builds/worker/workspace/build/src/testing/mozbase/mozprocess/mozprocess/processhandler.py", line 903, in __call__
[task 2017-10-10T19:59:30.390Z] 19:59:30     INFO -     e(*args, **kwargs)
[task 2017-10-10T19:59:30.391Z] 19:59:30     INFO -   File "/builds/worker/workspace/build/src/python/mach_commands.py", line 209, in _line_handler
[task 2017-10-10T19:59:30.391Z] 19:59:30     INFO -     if 'FAILED' in line.rsplit(' ', 1)[-1]:
[task 2017-10-10T19:59:30.391Z] 19:59:30     INFO - UnicodeDecodeError: 'ascii' codec can't decode byte 0xe2 in position 88: ordinal not in range(128)
[task 2017-10-10T19:59:31.373Z] 19:59:31     INFO - /builds/worker/workspace/build/src/testing/xpcshell/selftest.py
[task 2017-10-10T19:59:31.374Z] 19:59:31     INFO - ============================= test session starts ==============================
[task 2017-10-10T19:59:31.374Z] 19:59:31     INFO - platform linux2 -- Python 2.7.10, pytest-3.1.3, py-1.4.34, pluggy-0.4.0 -- /builds/worker/workspace/build/src/obj-firefox/_virtualenv/bin/python
[task 2017-10-10T19:59:31.374Z] 19:59:31     INFO - cachedir: ../.cache
[task 2017-10-10T19:59:31.374Z] 19:59:31     INFO - rootdir: /builds/worker/workspace/build/src, inifile:
[task 2017-10-10T19:59:31.375Z] 19:59:31     INFO - collecting ... collected 55 items
[task 2017-10-10T19:59:31.375Z] 19:59:31     INFO - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testAddTaskRunNextTest PASSED
[task 2017-10-10T19:59:31.375Z] 19:59:31  WARNING - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testAddTaskSkip TEST-UNEXPECTED-FAIL
[task 2017-10-10T19:59:31.376Z] 19:59:31  WARNING - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testAddTaskSkipAll TEST-UNEXPECTED-FAIL
[task 2017-10-10T19:59:31.376Z] 19:59:31     INFO - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testAddTaskStackTrace PASSED
[task 2017-10-10T19:59:31.376Z] 19:59:31     INFO - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testAddTaskTestFailureInside PASSED
[task 2017-10-10T19:59:31.377Z] 19:59:31  WARNING - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testAddTaskTestMultiple TEST-UNEXPECTED-FAIL
[task 2017-10-10T19:59:31.377Z] 19:59:31     INFO - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testAddTaskTestRejected PASSED
[task 2017-10-10T19:59:31.377Z] 19:59:31  WARNING - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testAddTaskTestSingle TEST-UNEXPECTED-FAIL
[task 2017-10-10T19:59:31.378Z] 19:59:31     INFO - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testAddTestFailing PASSED
[task 2017-10-10T19:59:31.378Z] 19:59:31  WARNING - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testAddTestSimple TEST-UNEXPECTED-FAIL
[task 2017-10-10T19:59:31.378Z] 19:59:31     INFO - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testAddTestUncaughtRejection PASSED
[task 2017-10-10T19:59:31.379Z] 19:59:31     INFO - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testAddTestUncaughtRejectionJSM PASSED
[task 2017-10-10T19:59:31.379Z] 19:59:31     INFO - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testAssertStack PASSED
[task 2017-10-10T19:59:31.379Z] 19:59:31     INFO - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testAsyncCleanup PASSED
[task 2017-10-10T19:59:31.379Z] 19:59:31  WARNING - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testChild TEST-UNEXPECTED-FAIL
[task 2017-10-10T19:59:31.380Z] 19:59:31     INFO - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testChildFail PASSED
[task 2017-10-10T19:59:31.380Z] 19:59:31     INFO - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testChildHang PASSED
[task 2017-10-10T19:59:31.380Z] 19:59:31  WARNING - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testChildMozinfo TEST-UNEXPECTED-FAIL
[task 2017-10-10T19:59:31.381Z] 19:59:31  WARNING - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testChildPass TEST-UNEXPECTED-FAIL
[task 2017-10-10T19:59:31.381Z] 19:59:31  WARNING - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testDoPrintWhenVerboseExplicit TEST-UNEXPECTED-FAIL
[task 2017-10-10T19:59:31.381Z] 19:59:31  WARNING - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testDoPrintWhenVerboseInManifest TEST-UNEXPECTED-FAIL
[task 2017-10-10T19:59:31.382Z] 19:59:31  WARNING - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testDoPrintWhenVerboseNotExplicit TEST-UNEXPECTED-FAIL
[task 2017-10-10T19:59:31.382Z] 19:59:31     INFO - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testDoReportForeignObject PASSED
[task 2017-10-10T19:59:31.382Z] 19:59:31     INFO - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testDoReportNonSyntaxError PASSED
[task 2017-10-10T19:59:31.383Z] 19:59:31     INFO - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testDoReportRefError PASSED
[task 2017-10-10T19:59:31.383Z] 19:59:31     INFO - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testDoReportSyntaxError PASSED
[task 2017-10-10T19:59:31.383Z] 19:59:31     INFO - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testDoThrowForeignObject PASSED
[task 2017-10-10T19:59:31.383Z] 19:59:31     INFO - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testDoThrowString PASSED
[task 2017-10-10T19:59:31.384Z] 19:59:31     INFO - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testFail PASSED
[task 2017-10-10T19:59:31.384Z] 19:59:31     INFO - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testHangingTimeout <- ../../../../../usr/lib/python2.7/unittest/case.py SKIPPED
[task 2017-10-10T19:59:31.384Z] 19:59:31  WARNING - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testKnownFail TEST-UNEXPECTED-FAIL
[task 2017-10-10T19:59:31.385Z] 19:59:31  WARNING - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testLogCorrectFileName TEST-UNEXPECTED-FAIL
[task 2017-10-10T19:59:31.385Z] 19:59:31     INFO - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testMissingHeadFile PASSED
[task 2017-10-10T19:59:31.385Z] 19:59:31  WARNING - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testMozinfo TEST-UNEXPECTED-FAIL
[task 2017-10-10T19:59:31.386Z] 19:59:31  WARNING - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testNoRunTestAddTask TEST-UNEXPECTED-FAIL
[task 2017-10-10T19:59:31.386Z] 19:59:31     INFO - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testNoRunTestAddTaskFail PASSED
[task 2017-10-10T19:59:31.386Z] 19:59:31  WARNING - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testNoRunTestAddTaskMultiple TEST-UNEXPECTED-FAIL
[task 2017-10-10T19:59:31.387Z] 19:59:31  WARNING - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testNoRunTestAddTest TEST-UNEXPECTED-FAIL
[task 2017-10-10T19:59:31.387Z] 19:59:31  WARNING - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testNoRunTestAddTestAddTask TEST-UNEXPECTED-FAIL
[task 2017-10-10T19:59:31.387Z] 19:59:31     INFO - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testNoRunTestAddTestFail PASSED
[task 2017-10-10T19:59:31.388Z] 19:59:31  WARNING - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testNoRunTestEmptyTest TEST-UNEXPECTED-FAIL
[task 2017-10-10T19:59:31.388Z] 19:59:31  WARNING - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testNotSkipForAddTask TEST-UNEXPECTED-FAIL
[task 2017-10-10T19:59:31.388Z] 19:59:31  WARNING - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testNotSkipForAddTest TEST-UNEXPECTED-FAIL
[task 2017-10-10T19:59:31.389Z] 19:59:31  WARNING - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testPass TEST-UNEXPECTED-FAIL
[task 2017-10-10T19:59:31.389Z] 19:59:31  WARNING - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testPassFail TEST-UNEXPECTED-FAIL
[task 2017-10-10T19:59:31.389Z] 19:59:31  WARNING - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testRandomExecution TEST-UNEXPECTED-FAIL
[task 2017-10-10T19:59:31.390Z] 19:59:31     INFO - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testReturnNonzero PASSED
[task 2017-10-10T19:59:31.390Z] 19:59:31     INFO - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testSkip PASSED
[task 2017-10-10T19:59:31.390Z] 19:59:31  WARNING - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testSkipForAddTask TEST-UNEXPECTED-FAIL
[task 2017-10-10T19:59:31.391Z] 19:59:31  WARNING - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testSkipForAddTest TEST-UNEXPECTED-FAIL
[task 2017-10-10T19:59:31.391Z] 19:59:31     INFO - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testSyntaxError PASSED
[task 2017-10-10T19:59:31.391Z] 19:59:31     INFO - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testUncaughtRejection PASSED
[task 2017-10-10T19:59:31.392Z] 19:59:31     INFO - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testUncaughtRejectionJSM PASSED
[task 2017-10-10T19:59:31.392Z] 19:59:31  WARNING - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testUnexpectedPass TEST-UNEXPECTED-FAIL
[task 2017-10-10T19:59:31.392Z] 19:59:31  WARNING - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testUnicodeInAssertMethods TEST-UNEXPECTED-FAIL
[task 2017-10-10T19:59:31.393Z] 19:59:31     INFO - =================================== FAILURES ===================================
[task 2017-10-10T19:59:31.393Z] 19:59:31     INFO - ______________________ XPCShellTestsTests.testAddTaskSkip ______________________
[task 2017-10-10T19:59:31.393Z] 19:59:31     INFO - self = <selftest.XPCShellTestsTests testMethod=testAddTaskSkip>
[task 2017-10-10T19:59:31.394Z] 19:59:31     INFO -     def testAddTaskSkip(self):
[task 2017-10-10T19:59:31.394Z] 19:59:31     INFO -         self.writeFile("test_tasks_skip.js", ADD_TASK_SKIP)
[task 2017-10-10T19:59:31.394Z] 19:59:31     INFO -         self.writeManifest(["test_tasks_skip.js"])
[task 2017-10-10T19:59:31.394Z] 19:59:31     INFO - >       self.assertTestResult(True)
[task 2017-10-10T19:59:31.395Z] 19:59:31     INFO - ../testing/xpcshell/selftest.py:1068:
[task 2017-10-10T19:59:31.395Z] 19:59:31     INFO - _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
[task 2017-10-10T19:59:31.395Z] 19:59:31     INFO - ../testing/xpcshell/selftest.py:527: in assertTestResult
[task 2017-10-10T19:59:31.396Z] 19:59:31     INFO -     """ % ("passed" if expected else "failed", self.log.getvalue()))
[task 2017-10-10T19:59:31.396Z] 19:59:31     INFO - E   AssertionError: Tests should have passed, log:
[task 2017-10-10T19:59:31.396Z] 19:59:31     INFO - E   ========
[task 2017-10-10T19:59:31.396Z] 19:59:31     INFO - E   MOZ_NODE_PATH environment variable not set. Tests requiring http/2 will fail.
[task 2017-10-10T19:59:31.397Z] 19:59:31     INFO - E   Running tests sequentially.
[task 2017-10-10T19:59:31.397Z] 19:59:31     INFO - E   SUITE-START | Running 1 tests
[task 2017-10-10T19:59:31.397Z] 19:59:31     INFO - E   TEST-START | test_tasks_skip.js
[task 2017-10-10T19:59:31.398Z] 19:59:31  WARNING - E   TEST-UNEXPECTED-FAIL | test_tasks_skip.js | xpcshell return code: -11
[task 2017-10-10T19:59:31.398Z] 19:59:31     INFO - E   TEST-INFO took 434ms
[task 2017-10-10T19:59:31.398Z] 19:59:31     INFO - E   >>>>>>>
[task 2017-10-10T19:59:31.399Z] 19:59:31     INFO - E   PID 36830 | [36830, Main Thread] WARNING: Couldn't get the user appdata directory. Crash events may not be produced.: file /builds/worker/workspace/build/src/toolkit/crashreporter/nsExceptionHandler.cpp, line 2872
[task 2017-10-10T19:59:31.399Z] 19:59:31     INFO - E   (xpcshell/head.js) | test MAIN run_test pending (1)
[task 2017-10-10T19:59:31.399Z] 19:59:31     INFO - E   (xpcshell/head.js) | test run_next_test 0 pending (2)
[task 2017-10-10T19:59:31.399Z] 19:59:31     INFO - E   (xpcshell/head.js) | test MAIN run_test finished (2)
[task 2017-10-10T19:59:31.400Z] 19:59:31     INFO - E   running event loop
[task 2017-10-10T19:59:31.400Z] 19:59:31     INFO - E   PID 36830 | Couldn't convert chrome URL: chrome://branding/locale/brand.properties
[task 2017-10-10T19:59:31.400Z] 19:59:31     INFO - E   PID 36830 | [36830, Main Thread] WARNING: Could not get the program name for a cubeb stream.: 'NS_SUCCEEDED(rv)', file /builds/worker/workspace/build/src/dom/media/CubebUtils.cpp, line 370
[task 2017-10-10T19:59:31.401Z] 19:59:31     INFO - E   test_tasks_skip.js | Starting skipMeNot1
[task 2017-10-10T19:59:31.401Z] 19:59:31     INFO - E   (xpcshell/head.js) | test skipMeNot1 pending (2)
[task 2017-10-10T19:59:31.401Z] 19:59:31     INFO - E   TEST-PASS | test_tasks_skip.js | skipMeNot1 - [skipMeNot1 : 3] Well well well. - true == true
[task 2017-10-10T19:59:31.402Z] 19:59:31     INFO - E   (xpcshell/head.js) | test run_next_test 0 finished (2)
[task 2017-10-10T19:59:31.402Z] 19:59:31     INFO - E   (xpcshell/head.js) | test run_next_test 1 pending (2)
[task 2017-10-10T19:59:31.402Z] 19:59:31     INFO - E   (xpcshell/head.js) | test skipMeNot1 finished (2)
[task 2017-10-10T19:59:31.403Z] 19:59:31     INFO - E   "CONSOLE_MESSAGE: (info) No chrome package registered for chrome://branding/locale/brand.properties"
[task 2017-10-10T19:59:31.403Z] 19:59:31     INFO - E   TEST-SKIP | test_tasks_skip.js | skipMe1 - skipMe1 skipped because the following conditions were met: (explicitly skipped.)
[task 2017-10-10T19:59:31.403Z] 19:59:31     INFO - E   (xpcshell/head.js) | test run_next_test pending (2)
[task 2017-10-10T19:59:31.404Z] 19:59:31     INFO - E   (xpcshell/head.js) | test run_next_test 1 finished (2)
[task 2017-10-10T19:59:31.404Z] 19:59:31     INFO - E   (xpcshell/head.js) | test run_next_test 2 pending (2)
[task 2017-10-10T19:59:31.404Z] 19:59:31     INFO - E   (xpcshell/head.js) | test skipMe1 finished (2)
[task 2017-10-10T19:59:31.404Z] 19:59:31     INFO - E   (xpcshell/head.js) | test run_next_test finished (1)
[task 2017-10-10T19:59:31.405Z] 19:59:31     INFO - E   exiting test
[task 2017-10-10T19:59:31.405Z] 19:59:31     INFO - E   test_tasks_skip.js | Starting skipMeNot2
[task 2017-10-10T19:59:31.405Z] 19:59:31     INFO - E   (xpcshell/head.js) | test skipMeNot2 pending (1)
[task 2017-10-10T19:59:31.406Z] 19:59:31     INFO - E   TEST-PASS | test_tasks_skip.js | skipMeNot2 - [skipMeNot2 : 11] Well well well. - true == true
[task 2017-10-10T19:59:31.406Z] 19:59:31     INFO - E   (xpcshell/head.js) | test run_next_test 2 finished (1)
[task 2017-10-10T19:59:31.406Z] 19:59:31     INFO - E   exiting test
[task 2017-10-10T19:59:31.406Z] 19:59:31     INFO - E   (xpcshell/head.js) | test run_next_test 3 pending (1)
[task 2017-10-10T19:59:31.407Z] 19:59:31     INFO - E   (xpcshell/head.js) | test skipMeNot2 finished (1)
[task 2017-10-10T19:59:31.407Z] 19:59:31     INFO - E   exiting test
[task 2017-10-10T19:59:31.408Z] 19:59:31     INFO - E   test_tasks_skip.js | Starting skipMeNot3
[task 2017-10-10T19:59:31.408Z] 19:59:31     INFO - E   (xpcshell/head.js) | test skipMeNot3 pending (1)
[task 2017-10-10T19:59:31.408Z] 19:59:31     INFO - E   TEST-PASS | test_tasks_skip.js | skipMeNot3 - [skipMeNot3 : 15] Well well well. - true == true
[task 2017-10-10T19:59:31.408Z] 19:59:31     INFO - E   (xpcshell/head.js) | test run_next_test 3 finished (1)
[task 2017-10-10T19:59:31.409Z] 19:59:31     INFO - E   exiting test
[task 2017-10-10T19:59:31.409Z] 19:59:31     INFO - E   (xpcshell/head.js) | test run_next_test 4 pending (1)
[task 2017-10-10T19:59:31.409Z] 19:59:31     INFO - E   (xpcshell/head.js) | test skipMeNot3 finished (1)
[task 2017-10-10T19:59:31.409Z] 19:59:31     INFO - E   exiting test
[task 2017-10-10T19:59:31.410Z] 19:59:31     INFO - E   TEST-SKIP | test_tasks_skip.js | skipMe2 - skipMe2 skipped because the following conditions were met: (explicitly skipped.)
[task 2017-10-10T19:59:31.410Z] 19:59:31     INFO - E   (xpcshell/head.js) | test run_next_test pending (1)
[task 2017-10-10T19:59:31.410Z] 19:59:31     INFO - E   (xpcshell/head.js) | test run_next_test 4 finished (1)
[task 2017-10-10T19:59:31.410Z] 19:59:31     INFO - E   exiting test
[task 2017-10-10T19:59:31.411Z] 19:59:31     INFO - E   (xpcshell/head.js) | test run_next_test 5 pending (1)
[task 2017-10-10T19:59:31.411Z] 19:59:31     INFO - E   (xpcshell/head.js) | test skipMe2 finished (1)
[task 2017-10-10T19:59:31.411Z] 19:59:31     INFO - E   exiting test
[task 2017-10-10T19:59:31.412Z] 19:59:31     INFO - E   (xpcshell/head.js) | test run_next_test finished (0)
[task 2017-10-10T19:59:31.412Z] 19:59:31     INFO - E   (xpcshell/head.js) | test run_next_test 5 finished (-1)
[task 2017-10-10T19:59:31.412Z] 19:59:31     INFO - E   PID 36830 | \x07[36830, Main Thread] ###!!! ASSERTION: Using observer service after XPCOM shutdown!: 'Error', file /builds/worker/workspace/build/src/xpcom/ds/nsObserverService.cpp, line 243
[task 2017-10-10T19:59:31.412Z] 19:59:31     INFO - E   PID 36830 | #01: mozilla::intl::LocaleService::~LocaleService [intl/locale/LocaleService.cpp:196]
[task 2017-10-10T19:59:31.413Z] 19:59:31     INFO - E   PID 36830 | #02: mozilla::intl::LocaleService::~LocaleService [intl/locale/LocaleService.cpp:201]
[task 2017-10-10T19:59:31.413Z] 19:59:31     INFO - E   PID 36830 | #03: mozilla::intl::LocaleService::Release [intl/locale/LocaleService.cpp:41]
[task 2017-10-10T19:59:31.413Z] 19:59:31     INFO - E   PID 36830 | #04: mozilla::KillClearOnShutdown [xpcom/base/ClearOnShutdown.cpp:38]
[task 2017-10-10T19:59:31.414Z] 19:59:31     INFO - E   PID 36830 | #05: mozilla::ShutdownXPCOM [xpcom/build/XPCOMInit.cpp:939]
[task 2017-10-10T19:59:31.414Z] 19:59:31     INFO - E   PID 36830 | #06: XRE_XPCShellMain [js/xpconnect/src/XPCShellImpl.cpp:1414]
[task 2017-10-10T19:59:31.414Z] 19:59:31     INFO - E   PID 36830 | #07: main [mfbt/UniquePtr.h:340]
[task 2017-10-10T19:59:31.414Z] 19:59:31     INFO - E   PID 36830 | #08: libc.so.6 + 0x1ed1d
[task 2017-10-10T19:59:31.415Z] 19:59:31     INFO - E   PID 36830 | #09: _start
[task 2017-10-10T19:59:31.415Z] 19:59:31     INFO - E   PID 36830 | [36830, Main Thread] ###!!! ASSERTION: Using observer service after XPCOM shutdown!: 'Error', file /builds/worker/workspace/build/src/xpcom/ds/nsObserverService.cpp, line 243
[task 2017-10-10T19:59:31.415Z] 19:59:31     INFO - E   PID 36830 | Hit MOZ_CRASH() at /builds/worker/workspace/build/src/memory/mozalloc/mozalloc_abort.cpp:33
[task 2017-10-10T19:59:31.416Z] 19:59:31     INFO - E   PID 36830 | ExceptionHandler::GenerateDump cloned child 36851
[task 2017-10-10T19:59:31.416Z] 19:59:31     INFO - E   PID 36830 | ExceptionHandler::SendContinueSignalToChild sent continue signal to child
[task 2017-10-10T19:59:31.416Z] 19:59:31     INFO - E   PID 36830 | ExceptionHandler::WaitForContinueSignal waiting for continue signal...
[task 2017-10-10T19:59:31.416Z] 19:59:31     INFO - E   <<<<<<<
[task 2017-10-10T19:59:31.417Z] 19:59:31     INFO - E   mozcrash Copy/paste: /builds/worker/workspace/build/tools/breakpad/linux64/minidump_stackwalk /tmp/xpc-other-8ojiaI/159d7a9b-2343-0717-3d36-620ed07f0b00.dmp /builds/worker/workspace/build/src/obj-firefox/dist/crashreporter-symbols
[task 2017-10-10T19:59:31.417Z] 19:59:31     INFO - E   mozcrash Saved minidump as /builds/worker/minidumps/159d7a9b-2343-0717-3d36-620ed07f0b00.dmp
[task 2017-10-10T19:59:31.417Z] 19:59:31     INFO - E   mozcrash Saved app info as /builds/worker/minidumps/159d7a9b-2343-0717-3d36-620ed07f0b00.extra
[task 2017-10-10T19:59:31.418Z] 19:59:31     INFO - E   INFO | Result summary:
[task 2017-10-10T19:59:31.418Z] 19:59:31     INFO - E   INFO | Passed: 0
[task 2017-10-10T19:59:31.418Z] 19:59:31     INFO - E   INFO | Failed: 1
[task 2017-10-10T19:59:31.418Z] 19:59:31     INFO - E   INFO | Todo: 0
[task 2017-10-10T19:59:31.419Z] 19:59:31     INFO - E   INFO | Retried: 0
[task 2017-10-10T19:59:31.419Z] 19:59:31     INFO - E   SUITE-END | took 0s
[task 2017-10-10T19:59:31.419Z] 19:59:31     INFO - E
[task 2017-10-10T19:59:31.419Z] 19:59:31     INFO - E   ========

Flags: needinfo?(gandalf)

Zibi Braniecki [:zbraniecki][:gandalf]

Assignee

Comment 13

•

7 years ago

weeeird bug.

Debugging and so far I only got that it's caused by the first part of the patch (not the renames) and the stack is sth like:

 0:44.55 E   PID 5147 | \x07[5147, Main Thread] ###!!! ASSERTION: Using observer service after XPCOM shutdown!: 'Error', file /projects/mozilla-unified/xpcom/ds/nsObserverService.cpp, line 243
 0:44.55 E   PID 5147 | #01: nsObserverService::RemoveObserver(nsIObserver*, char const*) (/projects/mozilla-unified/xpcom/ds/nsObserverService.cpp:243 (discriminator 3))
 0:44.55 E   PID 5147 | #02: nsCOMPtr<nsIObserverService>::~nsCOMPtr() (/projects/mozilla-unified/obj-x86_64-pc-linux-gnu/dist/include/nsCOMPtr.h:424)
 0:44.55 E   PID 5147 | #03: operator delete(void*) (/projects/mozilla-unified/obj-x86_64-pc-linux-gnu/dist/include/mozilla/mozalloc.h:230)
 0:44.55 E   PID 5147 | #04: mozilla::intl::LocaleService::Release() (/projects/mozilla-unified/intl/locale/LocaleService.cpp:41 (discriminator 3))
 0:44.55 E   PID 5147 | #05: mozilla::ClearOnShutdown_Internal::PointerClearer<mozilla::StaticRefPtr<mozilla::intl::LocaleService> >::Shutdown() (/projects/mozilla-unified/obj-x86_64-pc-linux-gnu/dist/include/mozilla/ClearOnShutdown.h:83)
 0:44.55 E   PID 5147 | #06: mozilla::KillClearOnShutdown(mozilla::ShutdownPhase) (/projects/mozilla-unified/xpcom/base/ClearOnShutdown.cpp:38)
 0:44.55 E   PID 5147 | #07: mozilla::ShutdownXPCOM(nsIServiceManager*) (/projects/mozilla-unified/xpcom/build/XPCOMInit.cpp:939)
 0:44.55 E   PID 5147 | #08: NS_ShutdownXPCOM (/projects/mozilla-unified/xpcom/build/XPCOMInit.cpp:823)
 0:44.55 E   PID 5147 | #09: XRE_XPCShellMain(int, char**, char**, XREShellData const*) (/projects/mozilla-unified/js/xpconnect/src/XPCShellImpl.cpp:1413)
 0:44.55 E   PID 5147 | #10: mozilla::BootstrapImpl::XRE_XPCShellMain(int, char**, char**, XREShellData const*) (/projects/mozilla-unified/toolkit/xre/Bootstrap.cpp:58)
 0:44.55 E   PID 5147 | #11: main (/projects/mozilla-unified/js/xpconnect/shell/xpcshell.cpp:68)
 0:44.55 E   PID 5147 | #12: __libc_start_main (/usr/lib/libc.so.6)
 0:44.55 E   PID 5147 | #13: _start (/projects/mozilla-unified/obj-x86_64-pc-linux-gnu/dist/bin/xpcshell)
 0:44.55 E   PID 5147 | #14: ??? (???:???)
 0:44.55 E   PID 5147 | [5147, Main Thread] ###!!! ASSERTION: Using observer service after XPCOM shutdown!: 'Error', file /projects/mozilla-unified/xpcom/ds/nsObserverService.cpp, line 243
 0:44.55 E   PID 5147 | Hit MOZ_CRASH() at /projects/mozilla-unified/memory/mozalloc/mozalloc_abort.cpp:33
 0:44.55 E   PID 5147 | ExceptionHandler::GenerateDump cloned child 5162

Zibi Braniecki [:zbraniecki][:gandalf]

Assignee

Comment 14

•

7 years ago

Ok, I think it all makes sense.

The LocaleService::~LocaleService is called after XPCOM shutdown.

The question is how to workaround it. My first guess is that maybe we can AddObserver as a weak reference and that means it'll get autoremoved?

Jonathan, can you advise here?

Flags: needinfo?(gandalf) → needinfo?(jfkthame)

Jonathan Kew [:jfkthame]

Comment 15

•

7 years ago

It looks to me like a fix here would be to ensure the LocaleService is cleared in a slightly earlier phase of shutdown, by changing

     ClearOnShutdown(&sInstance);

to

     ClearOnShutdown(&sInstance, ShutdownPhase::Shutdown);

(the default being ShutdownPhase::ShutdownFinal). This change should mean it'll be done before the observer service has been told to shut itself down. (From inspection of the sequence of things done by ShutdownXPCOM.)

I'd suggest pushing a linux64-debug try run with that change, and see if it resolves things; I believe it will.

Flags: needinfo?(jfkthame)

Zibi Braniecki [:zbraniecki][:gandalf]

Assignee

Comment 16

•

7 years ago

Unfortunately, it doesn't seem to fix things :(

I applied your change, and ran `./mach python-test testing/xpcshell/selftest.py` and ended up with the same crashes in tests.

Flags: needinfo?(jfkthame)

Zibi Braniecki [:zbraniecki][:gandalf]

Assignee

Comment 17

•

7 years ago

hmm, but switching the AddObserver to `ownsWeak=true` fixes them. This patch seems to work:

```
--- a/intl/locale/LocaleService.cpp
+++ b/intl/locale/LocaleService.cpp
@@ -180,10 +180,10 @@ LocaleService::GetInstance()
 
       nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
       if (obs) {
-        obs->AddObserver(sInstance, INTL_SYSTEM_LOCALES_CHANGED, false);
+        obs->AddObserver(sInstance, INTL_SYSTEM_LOCALES_CHANGED, true);
       }
     }
-    ClearOnShutdown(&sInstance);
+    ClearOnShutdown(&sInstance, ShutdownPhase::Shutdown);
   }
   return sInstance;
 }
@@ -195,7 +195,7 @@ LocaleService::~LocaleService()
 
     nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
     if (obs) {
-      obs->RemoveObserver(static_cast<nsIObserver*>(this), INTL_SYSTEM_LOCALES_CHANGED);
+      obs->RemoveObserver(this, INTL_SYSTEM_LOCALES_CHANGED);
     }
   }
 }
```

I'll run it on try, but keeping the NI on, since I don't know if this is the right way to fix it, or just hide the symptoms.

Comment hidden (mozreview-request)

Comment on attachment 8853323 [details]
Bug 1352343 - Bind LocaleService to react to OSPreferences `intl:system-locales-changed`.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/125412/diff/4-5/

Comment hidden (mozreview-request)

Comment on attachment 8917071 [details]
Bug 1352343 - Rename LocaleService::On* methods to LocaleService::*.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/188082/diff/1-2/

Zibi Braniecki [:zbraniecki][:gandalf]

Assignee

Comment 20

•

7 years ago

The try looks green if you ignore the oranges - https://treeherder.mozilla.org/#/jobs?repo=try&revision=764f6b11c704&selectedJob=136285972 :)

Will wait for Jonathan to check my logic. My understanding of the nsIObserverService weak logic is fuzzy, and MDN doc specifically says "In most cases, you should use false"[0], so I'd like to get a confirmation that we're a special snowflake.

[0] https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIObserverService

Jonathan Kew [:jfkthame]

Comment 21

•

7 years ago

Ah, I think I get it... my original suggestion didn't help, because although it makes us clear the sInstance variable earlier, the observer service itself is still holding a strong reference, and so the LocaleService doesn't actually get closed down at that time. So it only goes away when the observer service finally releases its reference - which is too late to avoid the problem here.

Giving the observer service a weak reference means that the LocaleService really will get released at ShutdownPhase::Shutdown, and then we're all good. It's OK to use a weak reference here, AFAICS -- we're a special snowflake because we're a singleton that is held (strongly) in a static variable until shutdown anyway.

Flags: needinfo?(jfkthame)

Pulsebot

Comment 22

•

7 years ago

Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/59839bfbf574
Bind LocaleService to react to OSPreferences `intl:system-locales-changed`. r=jfkthame
https://hg.mozilla.org/integration/autoland/rev/8d79e0a2538d
Rename LocaleService::On* methods to LocaleService::*. r=jfkthame

Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)

Comment 23

•

7 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/59839bfbf574
https://hg.mozilla.org/mozilla-central/rev/8d79e0a2538d

Status: ASSIGNED → RESOLVED

Closed: 7 years ago

status-firefox58: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla58

Bug 1352343 - Bind LocaleService to react to OSPreferences `intl:system-locales-changed`. 7 years ago Zibi Braniecki [:zbraniecki][:gandalf] 59 bytes, text/x-review-board-request	jfkthame : review+	Details
Bug 1352343 - Rename LocaleService::On* methods to LocaleService::*. 7 years ago Zibi Braniecki [:zbraniecki][:gandalf] 59 bytes, text/x-review-board-request	jfkthame : review+	Details