1496773 - Unable to run web-platform-tests in geckoview TestRunnerActivity

Status: RESOLVED FIXED

(Testing :: web-platform-tests, defect)

Description

[Geoff Brown [:gbrown]]

I think we should be able to run wpt in the TestRunnerActivity, but currently it fails:

gbrown@mozpad2:~/src$ ./mach -v wpt testing/web-platform/tests/infrastructure/ --package-name=org.mozilla.geckoview.test
 0:00.04 INFO Creating config file /home/gbrown/objdirs/x86/_tests/web-platform/wptrunner.local.ini
 0:00.04 INFO Creating directory /home/gbrown/objdirs/x86/_tests/web-platform/meta
 0:00.04 INFO Creating directory /home/gbrown/objdirs/x86/_tests/web-platform/mozilla/meta
 0:01.33 WARNING No generated manifest found
 0:01.33 INFO Recreating manifest /home/gbrown/objdirs/x86/_tests/web-platform/meta/MANIFEST.json
 1:51.31 INFO Recreating manifest /home/gbrown/objdirs/x86/_tests/web-platform/mozilla/meta/MANIFEST.json
 1:57.38 INFO Using 1 client processes
 1:57.50 INFO Starting http server on 192.168.0.6:8000
 1:57.51 INFO Starting http server on 192.168.0.6:8001
 1:57.57 INFO Starting https server on 192.168.0.6:8443
 1:58.05 SUITE_START: web-platform-test - running 45 tests
 1:58.05 INFO Running reftest tests
 1:58.06 ERROR Unsupported test type reftest for product fennec
 1:58.06 INFO Running wdspec tests
 1:58.06 ERROR Unsupported test type wdspec for product fennec
 1:58.06 INFO Running testharness tests
 1:58.10 INFO --certutil-binary not supplied; Firefox will not check certificates
 1:58.14 adb INFO Using adb 1.0.40
 1:58.51 adb INFO adbd running as root
 1:58.86 adb INFO su 0 supported
 1:59.19 adb INFO /system/bin/ls -1A supported
 1:59.37 adb INFO Native cp support: True
 1:59.54 adb INFO Native chmod -R support: True
 1:59.71 adb INFO Setting SELinux Permissive
 2:02.00 adb WARNING Ignoring attempt to chmod external storage
 2:08.74 INFO STDOUT: timed out waiting for profiles.ini
 2:12.10 INFO Application command: /home/gbrown/.mozbuild/android-sdk-linux/platform-tools/adb shell am start -a android.activity.MAIN -n org.mozilla.geckoview.test/org.mozilla.gecko.BrowserApp --es args '-no-remote -profile /sdcard/tests/profile --marionette about:blank' --es env0 MOZ_CRASHREPORTER=1 --es env1 R_LOG_VERBOSE=1 --es env2 MOZ_HIDE_RESULTS_TABLE=1 --es env3 STYLO_THREADS=1 --es env4 MOZ_LOG=signaling:3,mtransport:4,DataChannel:4,jsep:4,MediaPipelineFactory:4 --es env5 MOZ_CRASHREPORTER_SHUTDOWN=1 --es env6 R_LOG_DESTINATION=stderr --es env7 MOZ_DISABLE_NONLOCAL_CONNECTIONS=1 --es env8 MOZ_CRASHREPORTER_NO_REPORT=1 --es env9 MOZ_PROCESS_LOG=/tmp/tmpfsi1MRpidlog --es env10 R_LOG_LEVEL=6
 2:12.36 pid:23106 Full command: /home/gbrown/.mozbuild/android-sdk-linux/platform-tools/adb shell am start -a android.activity.MAIN -n org.mozilla.geckoview.test/org.mozilla.gecko.BrowserApp --es args '-no-remote -profile /sdcard/tests/profile --marionette about:blank' --es env0 MOZ_CRASHREPORTER=1 --es env1 R_LOG_VERBOSE=1 --es env2 MOZ_HIDE_RESULTS_TABLE=1 --es env3 STYLO_THREADS=1 --es env4 MOZ_LOG=signaling:3,mtransport:4,DataChannel:4,jsep:4,MediaPipelineFactory:4 --es env5 MOZ_CRASHREPORTER_SHUTDOWN=1 --es env6 R_LOG_DESTINATION=stderr --es env7 MOZ_DISABLE_NONLOCAL_CONNECTIONS=1 --es env8 MOZ_CRASHREPORTER_NO_REPORT=1 --es env9 MOZ_PROCESS_LOG=/tmp/tmpfsi1MRpidlog --es env10 R_LOG_LEVEL=6
pid:23106 Starting: Intent { act=android.activity.MAIN cmp=org.mozilla.geckoview.test/org.mozilla.gecko.BrowserApp (has extras) }
 2:12.36 INFO STDOUT: Starting: Intent { act=android.activity.MAIN cmp=org.mozilla.geckoview.test/org.mozilla.gecko.BrowserApp (has extras) }
 2:12.36 pid:23106 Error type 3
 2:12.36 pid:23106 Error: Activity class {org.mozilla.geckoview.test/org.mozilla.gecko.BrowserApp} does not exist.
 2:12.36 INFO STDOUT: Error type 3
 2:12.37 INFO STDOUT: Error: Activity class {org.mozilla.geckoview.test/org.mozilla.gecko.BrowserApp} does not exist.
 2:22.43 INFO STDOUT: timed out waiting for 'org.mozilla.geckoview.test' process to start


It looks like just a little launch plumbing is required.

Comment 1

[Geoff Brown [:gbrown]]

(In reply to Geoff Brown [:gbrown] from comment #0)
> It looks like just a little launch plumbing is required.

I added those changes and got TestRunnerActivity started, but can not run tests. It looks like marionette client cannot connect to the server.

Logcats from TestRunnerActivity runs are missing the customary "Marionette	INFO	Listening on port 2828" message.

Comment 2

[Geoff Brown [:gbrown]]

It looks like marionette is not included in the androidTest apk's omni.ja.

For instance, chrome/marionette/content/server.js is not in the omni.ja in the androidTest apk.

Comment 3

[Geoff Brown [:gbrown]]

Marionette *is* included in the androidTest apk's omni.ja.

For instance, chrome/marionette/content/server.js is in the omni.ja in the androidTest apk.

Comment 4

[Geoff Brown [:gbrown]]

Recent failure:

https://treeherder.mozilla.org/logviewer.html#?job_id=204955894&repo=try&lineNumber=1350

[task 2018-10-12T00:17:34.917Z] 00:17:34     INFO - adb launch_application: am start -W -n org.mozilla.geckoview.test/org.mozilla.geckoview.test.TestRunnerActivity -a android.intent.action.Main --es env9 MOZ_PROCESS_LOG=/tmp/tmpp57FT9pidlog --es env8 MOZ_CRASHREPORTER_NO_REPORT=1 --es args '-no-remote -profile /sdcard/tests/profile --marionette about:blank' --es env3 STYLO_THREADS=4 --es env2 MOZ_HIDE_RESULTS_TABLE=1 --es env1 R_LOG_VERBOSE=1 --es env0 MOZ_CRASHREPORTER=1 --es env7 MOZ_DISABLE_NONLOCAL_CONNECTIONS=1 --es env6 R_LOG_DESTINATION=stderr --es env5 MOZ_CRASHREPORTER_SHUTDOWN=1 --es env4 MOZ_LOG=signaling:3,mtransport:4,DataChannel:4,jsep:4,MediaPipelineFactory:4 --ez use_multiprocess True --es env10 R_LOG_LEVEL=6
[task 2018-10-12T00:17:36.041Z] 00:17:36    ERROR - adb get_process_list: 
[task 2018-10-12T00:17:36.145Z] 00:17:36     INFO - STDOUT: ZZZ org.mozilla.geckoview.test is running
[task 2018-10-12T00:17:36.256Z] 00:17:36     INFO - STDOUT: ZZZ org.mozilla.geckoview.test is running
[task 2018-10-12T00:17:36.259Z] 00:17:36     INFO - STDOUT: ZZZ DeviceRunner.start() okay
[task 2018-10-12T00:17:36.364Z] 00:17:36     INFO - Starting runner
[task 2018-10-12T00:17:36.383Z] 00:17:36     INFO - Connecting to Marionette on port 2828
[task 2018-10-12T00:17:36.384Z] 00:17:36     INFO - Waiting for Marionette connection
[task 2018-10-12T00:17:36.384Z] 00:17:36     INFO - STDOUT: protocol setup() - going to connect
[task 2018-10-12T00:19:36.389Z] 00:19:36  WARNING - Timed out waiting for browser to start

:jchen - Would you happen to know why the marionette server is not starting in TestRunnerActivity?

Comment 5

[David Burns :automatedtester]

Could this be that we are not processing the command line arg? We could set the marionette.enabled pref to true and see if that works.

Comment 6

[Geoff Brown [:gbrown]]

(In reply to David Burns :automatedtester from comment #5)
> Could this be that we are not processing the command line arg? We could set
> the marionette.enabled pref to true and see if that works.

Yes! That seems to be the trouble. If I set marionette.enabled=True, I can run wpt in TestRunnerActivity - wonderful!

Comment 7

[Geoff Brown [:gbrown]]

:jchen - New question then, why is the --marionette argument not getting through to gecko? See the launch command in comment 4. Also, I verified that GeckoThread sees the right args:

https://dxr.mozilla.org/mozilla-central/rev/c291143e24019097d087f9307e59b49facaf90cb/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoThread.java#483

RunGecko - args = org.mozilla.geckoview.test -greomni /data/app/org.mozilla.geckoview.test-1/base.apk -profile /sdcard/tests/profile -purgecaches -no-remote --marionette about:blank

Comment 8

[Geoff Brown [:gbrown]]

(In reply to Geoff Brown [:gbrown] from comment #6)
> (In reply to David Burns :automatedtester from comment #5)
> > Could this be that we are not processing the command line arg? We could set
> > the marionette.enabled pref to true and see if that works.
> 
> Yes! That seems to be the trouble. If I set marionette.enabled=True, I can
> run wpt in TestRunnerActivity - wonderful!

Sigh. That's not correct. I'm not sure what I did (was I mistakenly running in fennec?), but I still don't see marionette server initialization, even with marionette.enabled=True.

Comment 9

[Geoff Brown [:gbrown]]

I enabled debug logging. Now, in TestRunnerActivity, with marionette.enabled=True, I see:

10-12 22:58:08.720  2299  2315 I Gecko   : 1539381488720	Marionette	DEBUG	Received observer notification profile-after-change
10-12 22:58:08.730  2299  2315 I Gecko   : 1539381488730	Marionette	DEBUG	Received observer notification command-line-startup
10-12 22:58:08.730  2299  2315 I Gecko   : 1539381488730	Marionette	DEBUG	Received observer notification nsPref:changed
10-12 22:58:08.730  2299  2315 I Gecko   : 1539381488730	Marionette	DEBUG	Init aborted (running=false, enabled=true, finalUIStartup=false)

I think the trouble there is finalUIStartup=false, and I think that's because sessionstore-windows-restored has not been observed.

https://dxr.mozilla.org/mozilla-central/rev/c291143e24019097d087f9307e59b49facaf90cb/testing/marionette/components/marionette.js#425

https://dxr.mozilla.org/mozilla-central/rev/c291143e24019097d087f9307e59b49facaf90cb/testing/marionette/components/marionette.js#398

Comment 10

[David Burns :automatedtester]

(In reply to Geoff Brown [:gbrown] from comment #9)
> I enabled debug logging. Now, in TestRunnerActivity, with
> marionette.enabled=True, I see:
> 
> 10-12 22:58:08.720  2299  2315 I Gecko   : 1539381488720	Marionette	DEBUG
> Received observer notification profile-after-change
> 10-12 22:58:08.730  2299  2315 I Gecko   : 1539381488730	Marionette	DEBUG
> Received observer notification command-line-startup
> 10-12 22:58:08.730  2299  2315 I Gecko   : 1539381488730	Marionette	DEBUG
> Received observer notification nsPref:changed
> 10-12 22:58:08.730  2299  2315 I Gecko   : 1539381488730	Marionette	DEBUG
> Init aborted (running=false, enabled=true, finalUIStartup=false)
> 
> I think the trouble there is finalUIStartup=false, and I think that's
> because sessionstore-windows-restored has not been observed.
> 
> https://dxr.mozilla.org/mozilla-central/rev/
> c291143e24019097d087f9307e59b49facaf90cb/testing/marionette/components/
> marionette.js#425
> 
> https://dxr.mozilla.org/mozilla-central/rev/
> c291143e24019097d087f9307e59b49facaf90cb/testing/marionette/components/
> marionette.js#398

I wonder if this and https://bugzilla.mozilla.org/show_bug.cgi?id=1382162 are related or if sessionstore-windows-restored is more of a desktop thing...

Comment 11

[James Willcox (:snorp) ([e-mail]) (he/him)]

GeckoView doesn't use sessionstore, so it will never fire this event. Marionette needs to use something else to finish startup, I guess.

Comment 12

[David Burns :automatedtester]

(In reply to James Willcox (:snorp) ([e-mail]) from comment #11)
> GeckoView doesn't use sessionstore, so it will never fire this event.
> Marionette needs to use something else to finish startup, I guess.

Any recommendations for what we should wait for for Geckoview?

Comment 13

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to David Burns :automatedtester from comment #12)
> (In reply to James Willcox (:snorp) ([e-mail]) from comment #11)
> > GeckoView doesn't use sessionstore, so it will never fire this event.
> > Marionette needs to use something else to finish startup, I guess.
> 
> Any recommendations for what we should wait for for Geckoview?

I guess it depends on what you need. If this is a startup thing, 'profile-after-change' is probably adequate. You'd then listen for new 'navigator:geckoview' windows to be created.

Comment 14

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

(In reply to James Willcox (:snorp) ([e-mail]) from comment #13)
> I guess it depends on what you need. If this is a startup thing,
> 'profile-after-change' is probably adequate. You'd then listen for new
> 'navigator:geckoview' windows to be created.

`profile-after-change` is too early in the process, even `sessionstore-windows-restored` for Firefox so that we have to delay our initialization even more. Note that this was requested for reducing startup costs and increasing performance. See the following code:

https://dxr.mozilla.org/mozilla-central/rev/c291143e24019097d087f9307e59b49facaf90cb/testing/marionette/components/marionette.js#430-438

We might be able to make it different for Fennec but would clearly need a definite step during startup where it is allowed to start Marionette. As best this should be a observer notification. If `navigator:geckoview` gives that, wonderful. If you also have performance requirements for further delays, please also let us know.

Once we figured that out it would be great to get a Marionette bug created.

Comment 15

[Chris Peterson [:cpeterson]]

We don't run web-platform-tests on Fennec, but we'd like to run them on GeckoView, where possible:

Bitbar ARM32 (Moto G5)
Bitbar ARM64 (Pixel 2)
Packet x86-64

We don't need to test Android x86 if we are testing Android x86-64.

Comment 16

[Geoff Brown [:gbrown]]

https://treeherder.mozilla.org/#/jobs?repo=try&tier=1%2C2%2C3&revision=2601b55014aa4a18865ab3c9e83c38d6b93ff217

This patch:
 - enables marionette Trace logs
 - switches to geckoview TestRunnerActivity
 - sets finalUIStartup=True after toplevel-window-ready is observed (would need to make this conditional on android)

Now logcat shows:

11-29 23:43:17.010  2382  2397 I Gecko   : 1543534997010	Marionette	DEBUG	Received observer notification profile-after-change
11-29 23:43:17.040  2382  2397 I Gecko   : 1543534997040	Marionette	DEBUG	Received observer notification command-line-startup
11-29 23:43:17.040  2382  2397 I Gecko   : 1543534997040	Marionette	DEBUG	Received observer notification nsPref:changed
11-29 23:43:17.040  2382  2397 I Gecko   : 1543534997040	Marionette	DEBUG	Init aborted (running=false, enabled=true, finalUIStartup=false)
...
11-29 23:43:17.330  1311  1332 I ActivityManager: Displayed org.mozilla.geckoview.test/.TestRunnerActivity: +1s270ms
...
11-29 23:43:17.450  2382  2397 I Gecko   : 1543534997450	Marionette	DEBUG	Received observer notification toplevel-window-ready
11-29 23:43:17.450  2382  2397 I Gecko   : 1543534997450	Marionette	DEBUG	Waiting for delayed startup...
...
11-29 23:43:18.020  2382  2397 I Gecko   : 1543534998020	Marionette	INFO	Listening on port 2828
11-29 23:43:18.020  2382  2397 I Gecko   : 1543534998020	Marionette	DEBUG	Remote service is active
11-29 23:43:18.030  2440  2455 D GeckoViewStartup: observe: app-startup
11-29 23:43:18.060  2440  2455 D GeckoViewConsole: enabled = true
11-29 23:43:18.060  2440  2455 D GeckoThread: State changed to RUNNING
11-29 23:43:18.060  2382  2397 I Gecko   : 1543534998060	Marionette	DEBUG	Accepted connection 0 from 127.0.0.1:46850
11-29 23:43:18.070  2382  2397 I Gecko   : 1543534998070	Marionette	DEBUG	Closed connection 0
11-29 23:43:18.070  2382  2397 I Gecko   : 1543534998070	Marionette	DEBUG	Accepted connection 1 from 127.0.0.1:46851
11-29 23:43:18.080  2382  2397 I Gecko   : 1543534998080	Marionette	DEBUG	Accepted connection 2 from 127.0.0.1:46852
11-29 23:43:18.080  2382  2397 I Gecko   : 1543534998080	Marionette	DEBUG	Closed connection 1
11-29 23:43:18.080  2382  2397 I Gecko   : 1543534998080	Marionette	TRACE	2 -> [0,1,"WebDriver:NewSession",{"strictFileInteractability":true}]

Then no more marionette activity until timeout:

11-29 23:49:17.590  2382  2397 I Gecko   : 1543535357590	Marionette	DEBUG	Closed connection 2


That seems better, but...what are we waiting for?

Comment 17

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

(In reply to Geoff Brown [:gbrown] from comment #16)
>  - sets finalUIStartup=True after toplevel-window-ready is observed (would
> need to make this conditional on android)

You simply delay a check to a later time inside of `NewSession`. I'm fairly sure that the initial window is not correctly getting finished loading. Maybe this is the reason why no "sessionstore-windows-restored" notification is sent (bug 1382162).

Comment 18

[Geoff Brown [:gbrown]]

(In reply to Geoff Brown [:gbrown] from comment #16)
> That seems better, but...what are we waiting for?

The client is waiting for WebDriver:NewSession to complete.

The server is failing with

[JavaScript Error: "TypeError: WeakMap key must be an object, got undefined" {file: "chrome://marionette/content/driver.js" line: 1320}]

that's in GeckoDriver.prototype.updateIdForBrowser() - I suppose browser.permanentKey == undefined.

Comment 19

[David Burns :automatedtester]

(In reply to Geoff Brown [:gbrown] from comment #18)
> (In reply to Geoff Brown [:gbrown] from comment #16)
> > That seems better, but...what are we waiting for?
> 
> The client is waiting for WebDriver:NewSession to complete.
> 
> The server is failing with
> 
> [JavaScript Error: "TypeError: WeakMap key must be an object, got undefined"
> {file: "chrome://marionette/content/driver.js" line: 1320}]
> 
> that's in GeckoDriver.prototype.updateIdForBrowser() - I suppose
> browser.permanentKey == undefined.

This looks like it might be related to the window work that ato recently did. We were this might be an issue so perhaps he can have a look in January.

Comment 20

[Geoff Brown [:gbrown]]

Finally got tests running in geckoview:

https://treeherder.mozilla.org/#/jobs?repo=try&tier=1%2C2%2C3&revision=68fc466f18ad11aad415caba3978fc44b1f2085e

The various hacks are inappropriate for check-in, but this at least identifies the issues.

Comment 21

[Geoff Brown [:gbrown]]

Attachment: switch to geckoview

We cannot check this in until the hacks are addressed and the tests greened up again, but let's review this bit now anyway.

Changing the target apk is all that's needed to get the geckoview test app installed and run. You can see that happening in my try run:

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=216927454&repo=try&lineNumber=22
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=216927454&repo=try&lineNumber=1340

We run geckoview in e10s mode, so there's a slight change for that too.

At the same time, I'd like to reduce the number of test chunks, to reduce overhead (it takes 2 to 3 minutes per task to get the emulator started, packages installed, etc). Most chunks still complete in 30 minutes or less, which seems reasonable. Chunk divisions can affect greening, so this seems like a good time to make changes.

Comment 22

[Geoff Brown [:gbrown]]

Attachment: hacks I needed to get marionette to work with geckoview

I certainly don't want to check this in, but it demonstrates the issues. Hopefully ato or whimboo can provide better solutions.

Comment 23

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Comment on attachment 9031526 [details] [diff] [review]
switch to geckoview

Looks good.

Just so I'm clear, this patch in effect prevents wpt from running against Fennec, yes? If that's so, I wonder if it'd be worth making it possible to run either?

Incidentally, do you know if it's possible to distinguish Fennec from geckotestview via mozinfo, for annotating expectations?

Comment 24

[David Bolter [:davidb] (NeedInfo me for attention)]

Geoff do you know who we can hand this bug off to (assignee)?

Comment 25

[Geoff Brown [:gbrown]]

(In reply to David Bolter [:davidb] (NeedInfo me for attention) from comment #24)
> Geoff do you know who we can hand this bug off to (assignee)?

I don't know exactly. The remaining work is in marionette. Maybe :ato, or :whimboo?

Comment 26

[Geoff Brown [:gbrown]]

Attachment: switch to geckoview

> Just so I'm clear, this patch in effect prevents wpt from running against
> Fennec, yes? If that's so, I wonder if it'd be worth making it possible to
> run either?

It should be okay to run either. The hacks patch will need to be generalized. 'mach wpt' still runs fennec by default and can run geckoview with --package. In continuous integration, a simple change to the apk configuration switches between fennec/geckoview. In the earlier patch I had hard-coded e10s; that's corrected here to derive e10s-ness from the package name.

> Incidentally, do you know if it's possible to distinguish Fennec from 
> geckotestview via mozinfo, for annotating expectations?

One strategy is to use e10s to approximate: android && e10s => geckoview / android && !e10s => fennec. (geckoview can run non-e10s, but I believe there are no plans to actually use that.)

Bug 1511133 recently added isFennec for mochitest.

Comment 27

[James Graham [:jgraham]]

Attachment: Bug 1496773 - Get marionette working with geckoview,

Geckoview has a different window type and several differences in the available properties, so some
changes are needed to ensure that marionette is able to find the windows. Patch originally by
[e-mail].

Comment 28

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Comment on attachment 9032003 [details] [diff] [review]
switch to geckoview

lgtm

Comment 29

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

So what is the product id or app name for builds running under geckoview? Are those still promoted with the Fennec ID?

Comment 30

[James Graham [:jgraham]]

AIUI there isn't one; geckoview isn't an app and so doesn't have app id. An app using geckoview will have one, but we don't want to depend on that.

Comment 31

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

Ok, so basically any app integrating geckoview will actually have it's own id. As such that is not helpful.

I wonder if there is anything under `Services.app` or similar which identifies geckoview.

Comment 32

[Andreas Tolfsen ❲:ato❳]

(In reply to David Burns :automatedtester from comment #19)
> (In reply to Geoff Brown [:gbrown] from comment #18)
> > (In reply to Geoff Brown [:gbrown] from comment #16)
> > > That seems better, but...what are we waiting for?
> > 
> > The client is waiting for WebDriver:NewSession to complete.
> > 
> > The server is failing with
> > 
> > [JavaScript Error: "TypeError: WeakMap key must be an object, got undefined"
> > {file: "chrome://marionette/content/driver.js" line: 1320}]
> > 
> > that's in GeckoDriver.prototype.updateIdForBrowser() - I suppose
> > browser.permanentKey == undefined.
> 
> This looks like it might be related to the window work that ato recently
> did. We were this might be an issue so perhaps he can have a look in January.

I only touched window manipulation, e.g. resizing/moving windows.
updateIdForBrowser is super-ancient, but is more than likely related
to what gbrown mentions about the permanentKey not being populated
on GeckoView.

In the review I agree with nalexander’s call that we should uplift
a patch to always populate the permanentKey on GeckoView based
<browser>s with this so that we can tell them apart.

Comment 33

[Vicky Chin [:vchin]]

Once we have marionette support for geckoview, it will also work with the Reference Browser?

Comment 34

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

(In reply to Andreas Tolfsen ⦗:ato⦘ from comment #32)

In the review I agree with nalexander’s call that we should uplift
a patch to always populate the permanentKey on GeckoView based
<browser>s with this so that we can tell them apart.

Nick, has someone already started to work on getting permanentKey populated in GeckoView? It feels like a blocker for us.

Comment 35

[Nick Alexander :nalexander [he/him]]

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #34)

(In reply to Andreas Tolfsen ⦗:ato⦘ from comment #32)

In the review I agree with nalexander’s call that we should uplift
a patch to always populate the permanentKey on GeckoView based
<browser>s with this so that we can tell them apart.

Nick, has someone already started to work on getting permanentKey populated in GeckoView? It feels like a blocker for us.

Not that I am aware of, so I filed Bug 1520226 to track this work. I expect that if you insert

browser.permanentKey = {};

around https://searchfox.org/mozilla-central/source/mobile/android/chrome/geckoview/geckoview.js#381 this will Just Work.

Comment 36

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1496773 - Part 1: Feature test and accommodate navigator:geckoview inside Marionette. r?ato

This lays the foundation for Marionette to work inside
GeckoView-consuming Apps.

Depends on D17577

Comment 37

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1496773 - Part 2: Route Marionette startup via new "marionette-startup-requested" notification. r?ato

This just adds a layer of indirection between existing startup paths and a
new startup path that GeckoView(-consumers) will exploit.

Depends on D17578

Comment 38

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1524673 - Make Marionette part of remote debugging within GeckoView. r?snorp,whimboo

Depends on D17579

Comment 39

[Nick Alexander :nalexander [he/him]]

Hi folks! The attached patch sequence splits the existing sequence into parts, applying the review comments and the suggested approach for a new startup notification.

./mach try fuzzy -q "'marionette | 'opt-web-platform" is at https://treeherder.mozilla.org/#/jobs?repo=try&revision=56c59b40ac731cd17542bea81210754846ec4d79

With some small tweaks, ./mach -v wpt testing/web-platform/tests/infrastructure/assumptions/allowed-to-play.html --package-name=org.mozilla.{gecko.test,geckoview_example} definitely invokes Marionette and does some things, but it totally does not work because these apps don't support multiple tabs. E.g., in logs:

01-24 22:59:17.789  5372  5387 I Gecko   : 1548370757789	Marionette	DEBUG	2 -> [0,3,"WebDriver:GetWindowHandle",{}]
01-24 22:59:17.790  5372  5387 I Gecko   : 1548370757790	Marionette	DEBUG	2 <- [1,3,null,{"value":"2147483649"}]
01-24 22:59:17.801  5372  5387 I Gecko   : 1548370757801	Marionette	DEBUG	2 -> [0,4,"WebDriver:ExecuteScript",{"script":"document.title = 'TestRunnerManager-1';","newSandbox":false,"args":[],"filename":"testing/web-platform/tests/tools/wptrunner/wptrunner/executors/executormarionette.py","sandbox":null,"line":61}]
01-24 22:59:17.805  5427  5442 D GeckoViewContent[C]: handleEvent: DOMTitleChanged
01-24 22:59:17.805  5372  5387 I Gecko   : 1548370757805	Marionette	DEBUG	2 <- [1,4,null,{"value":null}]
01-24 22:59:17.808  5372  5387 I Gecko   : 1548370757808	Marionette	DEBUG	2 -> [0,5,"WebDriver:SetTimeouts",{"script":45000}]
01-24 22:59:17.809  5372  5387 I Gecko   : 1548370757809	Marionette	DEBUG	2 <- [1,5,null,{"value":null}]
01-24 22:59:17.813  5372  5387 I Gecko   : 1548370757813	Marionette	DEBUG	2 -> [0,6,"WebDriver:GetWindowHandles",{}]
01-24 22:59:17.813  5372  5387 I Gecko   : 1548370757813	Marionette	DEBUG	2 <- [1,6,null,["2147483649"]]
01-24 22:59:17.816  5372  5387 I Gecko   : 1548370757816	Marionette	DEBUG	2 -> [0,7,"WebDriver:SwitchToWindow",{"focus":true,"name":"2147483649"}]
01-24 22:59:17.817  5372  5387 I Gecko   : 1548370757817	Marionette	DEBUG	2 <- [1,7,null,{"value":null}]
01-24 22:59:17.827  5372  5387 I Gecko   : 1548370757827	Marionette	DEBUG	2 -> [0,8,"WebDriver:ExecuteScript",{"script":"window.open('about:blank', '4f8ac1fb-bdc2-46c8-ab84-9328dfaf2b04', 'noopener')","ne ... ,"filename":"testing/web-platform/tests/tools/wptrunner/wptrunner/executors/executormarionette.py","sandbox":null,"line":61}]
01-24 22:59:17.832  5372  5387 D GeckoViewNavigation: handleOpenUri: uri=about:blank where=3 flags=0
01-24 22:59:17.834  5372  5387 I Gecko   : 1548370757834	Marionette	DEBUG	2 <- [1,8,null,{"value":null}]
01-24 22:59:17.835  5372  5387 D GeckoViewNavigation: handleNewSession: uri=about:blank where=3 flags=0
01-24 22:59:17.924  5372  5414 D EGL_emulation: eglCreateContext: 0xe129c080: maj 2 min 0 rcv 2
01-24 22:59:17.926  5372  5414 D EGL_emulation: eglMakeCurrent: 0xe129c080: ver 2 0 (tinfo 0xe2fd2760)
01-24 22:59:17.951  5372  5414 I GeckoConsole: OpenGL compositor Initialized Succesfully.
01-24 22:59:17.951  5372  5414 I GeckoConsole: Version: OpenGL ES 2.0 (OpenGL ES 3.0 SwiftShader 4.0.0.1)
01-24 22:59:17.951  5372  5414 I GeckoConsole: Vendor: Google (Google Inc.)
01-24 22:59:17.951  5372  5414 I GeckoConsole: Renderer: Android Emulator OpenGL ES Translator (Google SwiftShader)
01-24 22:59:17.951  5372  5414 I GeckoConsole: FBO Texture Target: TEXTURE_2D
01-24 22:59:17.963  5372  5387 I Gecko   : 1548370757962	Marionette	TRACE	Received observer notification toplevel-window-ready
01-24 22:59:17.974  5372  5387 I Gecko   : 1548370757974	Marionette	DEBUG	2 -> [0,9,"WebDriver:GetWindowHandles",{}]
01-24 22:59:17.975  5372  5387 I Gecko   : 1548370757975	Marionette	DEBUG	2 <- [1,9,null,["2147483649"]]

Observe that the second call to GetWindowHandles returns the same set as the first. It's not clear that any of our GeckoView-based browsers handle window.open in the way that wpt tests require, but I'd like to make progress on that in a separate ticket.

Comment 40

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

That sounds great. Please note that I could start working on the further integration once bug 1520226 has been fixed.

Comment 41

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

(In reply to Nick Alexander :nalexander [he/him] from comment #39)

01-24 22:59:17.827 5372 5387 I Gecko : 1548370757827 Marionette DEBUG 2 -> [0,8,"WebDriver:ExecuteScript",{"script":"window.open('about:blank', '4f8ac1fb-bdc2-46c8-ab84-9328dfaf2b04', 'noopener')","ne ... ,"filename":"testing/web-platform/tests/tools/wptrunner/wptrunner/executors/executormarionette.py","sandbox":null,"line":61}]
01-24 22:59:17.832 5372 5387 D GeckoViewNavigation: handleOpenUri: uri=about:blank where=3 flags=0
01-24 22:59:17.834 5372 5387 I Gecko : 1548370757834 Marionette DEBUG 2 <- [1,8,null,{"value":null}]
01-24 22:59:17.835 5372 5387 D GeckoViewNavigation: handleNewSession: uri=about:blank where=3 flags=0
01-24 22:59:17.924 5372 5414 D EGL_emulation: eglCreateContext: 0xe129c080: maj 2 min 0 rcv 2
01-24 22:59:17.926 5372 5414 D EGL_emulation: eglMakeCurrent: 0xe129c080: ver 2 0 (tinfo 0xe2fd2760)
01-24 22:59:17.951 5372 5414 I GeckoConsole: OpenGL compositor Initialized Succesfully.
01-24 22:59:17.951 5372 5414 I GeckoConsole: Version: OpenGL ES 2.0 (OpenGL ES 3.0 SwiftShader 4.0.0.1)
01-24 22:59:17.951 5372 5414 I GeckoConsole: Vendor: Google (Google Inc.)
01-24 22:59:17.951 5372 5414 I GeckoConsole: Renderer: Android Emulator OpenGL ES Translator (Google SwiftShader)
01-24 22:59:17.951 5372 5414 I GeckoConsole: FBO Texture Target: TEXTURE_2D
01-24 22:59:17.963 5372 5387 I Gecko : 1548370757962 Marionette TRACE Received observer notification toplevel-window-ready
01-24 22:59:17.974 5372 5387 I Gecko : 1548370757974 Marionette DEBUG 2 -> [0,9,"WebDriver:GetWindowHandles",{}]
01-24 22:59:17.975 5372 5387 I Gecko : 1548370757975 Marionette DEBUG 2 <- [1,9,null,["2147483649"]]

Observe that the second call to `GetWindowHandles` returns the same set as the first.  It's not clear that _any_ of our GeckoView-based browsers handle `window.open` in the way that wpt tests require, but I'd like to make progress on that in a separate ticket.

Note that the execute script call returns too early, means before the toplevel-window-ready notification has been received. This could cause race conditions, also because the parent process might not have been informed about the new tab. Basically you should use self.marionette.open(type="tab") to open a new tab, which will take care of all of this. I recently added this via bug 1504756.

Comment 42

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1496773 - Part 3: Make Marionette startup wait for toplevel-window-ready. r?whimboo

Depends on D17579

Comment 43

[Nick Alexander :nalexander [he/him]]

Note that the execute script call returns too early, means before the toplevel-window-ready notification has been received.

whimboo: I flagged you to review a patch that ensure this doesn't happen. It's not clear to me that it's necessary, but you can address that.

Comment 44

[Nick Alexander :nalexander [he/him]]

Comment on attachment 9038984 [details]
Bug 1524673 - Make Marionette part of remote debugging within GeckoView. r?snorp,whimboo

Spun out into Bug 1524673.

Comment 45

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

(In reply to Nick Alexander :nalexander [he/him] from comment #43)

Note that the execute script call returns too early, means before the toplevel-window-ready notification has been received.

whimboo: I flagged you to review a patch that ensure this doesn't happen. It's not clear to me that it's necessary, but you can address that.

Oh I reviewed the patch before seeing this comment. I think that we don't need it because that case happens later and not during startup of Marionette. The underlying problem will be fixed when we can land a fix for bug 1522790.

Comment 46

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d1654e2c218e
Part 1: Feature test and accommodate navigator:geckoview inside Marionette. r=ato
https://hg.mozilla.org/integration/autoland/rev/5ef017963ce3
Part 2: Route Marionette startup via new "marionette-startup-requested" notification. r=ato

Comment 47

[Phabricator Automation]

Comment on attachment 9038984 [details]
Bug 1524673 - Make Marionette part of remote debugging within GeckoView. r?snorp,whimboo

Revision D17580 was moved to bug 1524673. Setting attachment 9038984 [details] to obsolete.

Comment 48

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

https://hg.mozilla.org/mozilla-central/rev/d1654e2c218e
https://hg.mozilla.org/mozilla-central/rev/5ef017963ce3