1355382 - "JavascriptException: TypeError: Components.classes['@mozilla.org/intl/localeservice;1'].getService(...).getAppLocaleAsLangTag is not a function" caused by bug 1347314

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

[Tracking Requested - why for this release]:

As I have seen today when I tried to run update tests for Firefox on the aurora channel with the test files from mozilla-central, those are completely busted due to changes in bug 1347314 which never got a review from any testing peer.

So in detail the changes as done bust our update tests when we are trying to run updates from version 54 to version 55, because no fallback case has been added.

The failure itself looks like:

TEST-UNEXPECTED-ERROR | test_fallback_update.py TestFallbackUpdate.test_update | JavascriptException: TypeError: Components.classes['@mozilla.org/intl/localeservice;1'].getService(...).getAppLocaleAsLangTag is not a function
stacktrace:
	execute_script @appinfo.py, line 36
	inline javascript, line 1
	src: "              return Components.classes["@mozilla.org/intl/localeservice;1"]"
	Stack:
	@appinfo.py:1:22
	@appinfo.py:0:2
	evaluate.sandbox/promise<@chrome://marionette/content/evaluate.js:152:13
	evaluate.sandbox@chrome://marionette/content/evaluate.js:104:17
	GeckoDriver.prototype.execute_@chrome://marionette/content/driver.js:893:29
	GeckoDriver.prototype.executeScript@chrome://marionette/content/driver.js:792:27
	TaskImpl_run@resource://gre/modules/Task.jsm:319:42
	TaskImpl@resource://gre/modules/Task.jsm:277:3
	asyncFunction@resource://gre/modules/Task.jsm:252:14
	Task_spawn@resource://gre/modules/Task.jsm:166:12
	TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:389:16
	TaskImpl_run@resource://gre/modules/Task.jsm:327:15
	TaskImpl@resource://gre/modules/Task.jsm:277:3
	asyncFunction@resource://gre/modules/Task.jsm:252:14
	Task_spawn@resource://gre/modules/Task.jsm:166:12
	Dispatcher.prototype.execute@chrome://marionette/content/dispatcher.js:118:13
	Dispatcher.prototype.onPacket@chrome://marionette/content/dispatcher.js:89:5
	_onJSONObjectReady/<@chrome://marionette/content/server.js -> resource://devtools/shared/transport/transport.js:483:11
	exports.makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/ThreadSafeDevToolsUtils.js:101:14
	exports.makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/ThreadSafeDevToolsUtils.js:101:14
Traceback (most recent call last):
  File "/Volumes/data/code/gecko/testing/marionette/harness/marionette_harness/marionette_test/testcases.py", line 147, in run
    self.setUp()
  File "/Volumes/data/code/gecko/testing/firefox-ui/tests/update/fallback/test_fallback_update.py", line 11, in setUp
    UpdateTestCase.setUp(self, is_fallback=True)
  File "/Volumes/data/code/gecko/testing/firefox-ui/harness/firefox_ui_harness/testcases.py", line 77, in setUp
    'build_pre': self.software_update.build_info,
  File "/Volumes/data/code/gecko/testing/marionette/puppeteer/firefox/firefox_puppeteer/api/software_update.py", line 224, in build_info
    'locale': self.app_info.locale,
  File "/Volumes/data/code/gecko/testing/marionette/puppeteer/firefox/firefox_puppeteer/api/appinfo.py", line 36, in locale
    """)
  File "/Volumes/data/code/gecko/testing/marionette/client/marionette_driver/marionette.py", line 1861, in execute_script
    rv = self._send_message("executeScript", body, key="value")
  File "/Volumes/data/code/gecko/testing/marionette/client/marionette_driver/decorators.py", line 23, in _
    return func(*args, **kwargs)
  File "/Volumes/data/code/gecko/testing/marionette/client/marionette_driver/marionette.py", line 735, in _send_message
    self._handle_error(err)
  File "/Volumes/data/code/gecko/testing/marionette/client/marionette_driver/marionette.py", line 768, in _handle_error
    raise errors.lookup(error)(message, stacktrace=stacktrace)
TEST-INFO took 691ms

So the changes to appinfo.py still need the old code in case of older Firefox versions are used. 

Zibi, can you please fix this? Thanks!

Comment 1

[Zibi Braniecki [:zbraniecki][:gandalf]]

Totally, apologies!

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Thanks! In case you want to test your patch please do the following:

1. Use your hg bookmark or git branch to run the update tests
2. Trigger the update tests via `mach firefox-ui-update --binary %path_to_aurora_nightly_build% testing/firefox-ui/tests/update/manifest.ini`
3. Also try a cross-channel update to a central nightly build by appending the option `--update-channel=nightlytest`

Comment 3

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: Bug 1355382 - Use ChromeRegistry in Marionette Puppet when LocaleService is not available.

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

Comment 4

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8856974 [details]
Bug 1355382 - Use ChromeRegistry in Marionette Puppet when LocaleService is not available.

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

Comment 5

[Zibi Braniecki [:zbraniecki][:gandalf]]

I took the liberty of not testing it separately since setting up an environment may be non-trivial and the patch seems to require very trivial change.

:whimboo, can you be my reviewer for this?

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 8856974 [details]
Bug 1355382 - Use ChromeRegistry in Marionette Puppet when LocaleService is not available.

https://reviewboard.mozilla.org/r/128882/#review131404

The patch makes sense, and should work fine. I just have one comment I would like to see addressed. r=me with this done.

::: testing/marionette/puppeteer/firefox/firefox_puppeteer/api/appinfo.py:33
(Diff revision 2)
>  
>      @property
>      def locale(self):
>          with self.marionette.using_context('chrome'):
> +            # LocaleService has been added in 55, use ChromeRegistry for
> +            # earlier releases.

Can you please move this comment into the execute script call, so that it is close to the code it is referring?

You can also add/change that the fallback code can be removed once Firefox 52esr is no longer supported. That's easier to parse in a couple of months as seeing 55 mentioned here.

Comment 7

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8856974 [details]
Bug 1355382 - Use ChromeRegistry in Marionette Puppet when LocaleService is not available.

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

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e8919d6caa25
Use ChromeRegistry in Marionette Puppet when LocaleService is not available. r=whimboo

Comment 9

[Iris Hsiao [:ihsiao]]

https://hg.mozilla.org/mozilla-central/rev/e8919d6caa25