815807 - Make Marionette load later in B2G startup cycle, except when --load-early is passed

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Jonathan Griffin (:jgriffin)]

In order to allow us to land bug 800138, which allows Marionette to work remotely on pandas, we need to load Marionette later in the startup process.  However, this breaks WebAPI tests on emulators.

Therefore, we need to optionally allow Marionette to preserve it's current early-loading behavior on B2G.  We can add a --load-early argument to facilitate this.

Comment 1

[Jonathan Griffin (:jgriffin)]

In order to implement this without breaking anything on TBPL, I'll have to land three patches in sequence:

1 - gecko patch which adds --load-early to the Marionette Python client
2 - a mozharness script change to add --load-early to the webapi tests
3 - gecko patch which causes the Marionette server to act differently when --load-early is passed

Comment 2

[Jonathan Griffin (:jgriffin)]

Attachment: Part 1: add --load-early arg to Marionette,

This will be slightly bitrotted by bug 790817, but I'll unbitrot it before landing if needed.  This patch has no effect until parts 2 and 3 land (not yet submitted).

Comment 3

[Jonathan Griffin (:jgriffin)]

Attachment: Part 2: Add --load-early to mozharness marionette script for emulator testing,

This is the mozharness script change; it can't land until part 1 has propagated to all the trees, in order to avoid breaking things in TPBL

Comment 4

[Jonathan Griffin (:jgriffin)]

Comment on attachment 686225 [details] [diff] [review]
Part 1: add --load-early arg to Marionette,

Blargh, bzexport unset the review flag.

Comment 5

[Andrew Halberstadt [:ahal]]

Comment on attachment 686225 [details] [diff] [review]
Part 1: add --load-early arg to Marionette,

Review of attachment 686225 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/marionette/client/marionette/emulator.py
@@ +401,5 @@
> +            self.wait_for_system_message(marionette)
> +
> +    def restart_b2g(self):
> +        print 'restarting B2G'
> +        self.dm.shellCheckOutput(['stop', 'b2g'])

We should probably start using mozb2g to get all of the b2g specific stuff out of here, but I won't block on that since this is time sensitive. I'm guilty of just adding the install_busybox method ;)

Comment 6

[Jonathan Griffin (:jgriffin)]

(In reply to Andrew Halberstadt [:ahal] from comment #5)
> Comment on attachment 686225 [details] [diff] [review]
> Part 1: add --load-early arg to Marionette,
> 
> Review of attachment 686225 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: testing/marionette/client/marionette/emulator.py
> @@ +401,5 @@
> > +            self.wait_for_system_message(marionette)
> > +
> > +    def restart_b2g(self):
> > +        print 'restarting B2G'
> > +        self.dm.shellCheckOutput(['stop', 'b2g'])
> 
> We should probably start using mozb2g to get all of the b2g specific stuff
> out of here, but I won't block on that since this is time sensitive. I'm
> guilty of just adding the install_busybox method ;)

I agree, after all the fires are out we should clean up a lot of things.  I'll file a bug for this if I can't find an existing one.

Comment 7

[Jonathan Griffin (:jgriffin)]

Bug 816236 filed for getting Marionette to use mozdevice/mozb2g.

Comment 8

[Jonathan Griffin (:jgriffin)]

Part 1: https://hg.mozilla.org/integration/mozilla-inbound/rev/60a1cb7e8b79

Comment 9

[Jonathan Griffin (:jgriffin)]

Attachment: Part 1: add --load-eary arg to Marionette

Unbitrotted; carry r+ forward.

Comment 10

[Jonathan Griffin (:jgriffin)]

Attachment: Part 1a: temporarily enable some code until Part 2 lands,

I realized I needed to tweak some code to prevent breakage until part 2 lands, so here it is.

Comment 11

[Jonathan Griffin (:jgriffin)]

Part 1a: https://hg.mozilla.org/integration/mozilla-inbound/rev/2c3ca2156b00

Comment 12

[Jonathan Griffin (:jgriffin)]

Attachment: Part 1b: fix typo

And I realized when I merged Part 1 with inbound, on which bug 790817 landed, I made a typo which broke Marionette on TBPL.  Here's the fix.  :sigh:

https://hg.mozilla.org/integration/mozilla-inbound/rev/702ff40d1447

Comment 13

[Jonathan Griffin (:jgriffin)]

Attachment: Part 3: Load marionette early in the B2G startup process if relevant pref is set, otherwise load it late,

This patch takes some of past's work from bug 800138, along with patch 1 on this bug to load marionette early only if a pref is set.  Also does some cleanup of related code on the Python side.  Tested locally and it works fine.

Comment 14

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/60a1cb7e8b79
https://hg.mozilla.org/mozilla-central/rev/2c3ca2156b00
https://hg.mozilla.org/mozilla-central/rev/702ff40d1447

Comment 15

[Andrew Halberstadt [:ahal]]

Comment on attachment 686343 [details] [diff] [review]
Part 3: Load marionette early in the B2G startup process if relevant pref is set, otherwise load it late,

Review of attachment 686343 [details] [diff] [review]:
-----------------------------------------------------------------

Heh, I have no idea why any of this is needed. r+

Do you think in the future we could pass in the actual event to wait for via pref? That would make things a bit more flexible in case we find ourselves needing to wait for yet another event, or the events change.

Comment 16

[Jonathan Griffin (:jgriffin)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/5b81398a1ea2
https://hg.mozilla.org/releases/mozilla-aurora/rev/7cbfbb0bcb89
https://hg.mozilla.org/releases/mozilla-aurora/rev/ba6628781ca1

Comment 17

[Jonathan Griffin (:jgriffin)]

https://hg.mozilla.org/releases/mozilla-beta/rev/db23aec39973
https://hg.mozilla.org/releases/mozilla-beta/rev/ed3420ea3e63
https://hg.mozilla.org/releases/mozilla-beta/rev/c63bc3273e92

Comment 18

[Jonathan Griffin (:jgriffin)]

Part 2: http://hg.mozilla.org/build/mozharness/rev/ff8bda110d82

Comment 19

[Jonathan Griffin (:jgriffin)]

Pushed part 3 to try: https://tbpl.mozilla.org/?tree=Try&rev=c714fc81f334

Comment 20

[Jonathan Griffin (:jgriffin)]

(In reply to Jonathan Griffin (:jgriffin) from comment #19)
> Pushed part 3 to try: https://tbpl.mozilla.org/?tree=Try&rev=c714fc81f334

This patch breaks mochitest.  The reason is that when we restart B2G after installing the test profile, we never get a 'system-message-listener-ready' event, so Marionette never gets initialized.  I'm not quite sure why this is.  The workaround is to add the loadearly pref to the test profile, for which I'll attach a patch.

We won't be able to run mochitests on pandas until we find a better way to deal with this, but I guess that's a fight for another day.

Comment 21

[Jonathan Griffin (:jgriffin)]

Attachment: Part 3: Load marionette early in the B2G startup process if relevant pref is set, otherwise load it late

This revision fixes bustage with mochitest and reftest.

Comment 22

[Jonathan Griffin (:jgriffin)]

Pushed new version of Part 3 to try: https://tbpl.mozilla.org/?tree=Try&rev=bb5f83086e20

Comment 23

[Jonathan Griffin (:jgriffin)]

(In reply to Andrew Halberstadt [:ahal] from comment #15)
> Comment on attachment 686343 [details] [diff] [review]
> Part 3: Load marionette early in the B2G startup process if relevant pref is
> set, otherwise load it late,
> 
> Review of attachment 686343 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Heh, I have no idea why any of this is needed. r+
> 
> Do you think in the future we could pass in the actual event to wait for via
> pref? That would make things a bit more flexible in case we find ourselves
> needing to wait for yet another event, or the events change.

I wouldn't want to have to pass in a pref with an event name in it, since it would force the Python client to be more closely coupled with the JS bits, which would mean that version mistmaches between client and server (which are quite common in the wild and usually harmless) could end up being bad.  We'd also have to propagate a change like that to the JS client, so I think I'm in favor of leaving things as-is.

Comment 24

[Andrew Halberstadt [:ahal]]

(In reply to Jonathan Griffin (:jgriffin) from comment #23)
> I wouldn't want to have to pass in a pref with an event name in it, since it
> would force the Python client to be more closely coupled with the JS bits,
> which would mean that version mistmaches between client and server (which
> are quite common in the wild and usually harmless) could end up being bad. 
> We'd also have to propagate a change like that to the JS client, so I think
> I'm in favor of leaving things as-is.

Oops, I shouldn't have said pref. I meant command line argument (i.e instead of --load-early, --load-event <event name>). Anyway, not important for now at least.

Comment 25

[Andrew Halberstadt [:ahal]]

Comment on attachment 686930 [details] [diff] [review]
Part 3: Load marionette early in the B2G startup process if relevant pref is set, otherwise load it late

Review of attachment 686930 [details] [diff] [review]:
-----------------------------------------------------------------

Lgtm

Comment 26

[Jonathan Griffin (:jgriffin)]

Part 3:  https://hg.mozilla.org/integration/mozilla-inbound/rev/9252522aab90

I'll also take what's left of bug 800138 and make a Part 4.

Comment 27

[Jonathan Griffin (:jgriffin)]

Part 3:
https://hg.mozilla.org/releases/mozilla-aurora/rev/c1f26b5ca7bd
https://hg.mozilla.org/releases/mozilla-beta/rev/6cc72f5e8dc3

Comment 28

[Jonathan Griffin (:jgriffin)]

Attachment: Part 4: Let Marionette listen for non-local connections,

This contains the last gecko bits of bug 800138 that haven't been landed as parts of other patches.  The gaia PR for bug 800138 will have to land before we can close that bug.

Comment 29

[Jonathan Griffin (:jgriffin)]

Part 4 pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=71dbeb2731c1

Comment 30

[Jonathan Griffin (:jgriffin)]

Backed out part 3 due to non-B2G bustage: https://hg.mozilla.org/integration/mozilla-inbound/rev/d6529aa77182

Comment 31

[Jonathan Griffin (:jgriffin)]

(In reply to Jonathan Griffin (:jgriffin) from comment #30)
> Backed out part 3 due to non-B2G bustage:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/d6529aa77182

Also:
https://hg.mozilla.org/releases/mozilla-aurora/rev/77a4e53cc826
https://hg.mozilla.org/releases/mozilla-beta/rev/cae8d0d59cbd

Comment 32

[Jonathan Griffin (:jgriffin)]

Attachment: Part 3: Load marionette early in the B2G startup process if relevant pref is set, otherwise load it late

Carry r+ forward.  Fix desktop breakage by continuing to load Marionette early in non-B2G platforms.

Pushed to try:
https://tbpl.mozilla.org/?tree=Try&rev=b2a3ab386389
https://tbpl.mozilla.org/?tree=Try&rev=60b37c2de1b5

Comment 33

[Mozilla RelEng Bot]

Try run for b2a3ab386389 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=b2a3ab386389
Results (out of 4 total builds):
    success: 4
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-b2a3ab386389

Comment 34

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

per aki's mtg w/jgrffin, this is blocking-basecamp, hence nominating.

Comment 35

[Mozilla RelEng Bot]

Try run for 60b37c2de1b5 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=60b37c2de1b5
Results (out of 12 total builds):
    success: 11
    warnings: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-60b37c2de1b5

Comment 36

[Jonathan Griffin (:jgriffin)]

Part 3: https://hg.mozilla.org/integration/mozilla-inbound/rev/1b70ac0124fe
Part 4: https://hg.mozilla.org/integration/mozilla-inbound/rev/41be1ff63431

Comment 37

[Jonathan Griffin (:jgriffin)]

(In reply to Jonathan Griffin (:jgriffin) from comment #36)
> Part 3: https://hg.mozilla.org/integration/mozilla-inbound/rev/1b70ac0124fe
> Part 4: https://hg.mozilla.org/integration/mozilla-inbound/rev/41be1ff63431

I have no idea why this reset the blocking-basecamp flag.

Comment 38

[Jonathan Griffin (:jgriffin)]

aurora:
https://hg.mozilla.org/releases/mozilla-aurora/rev/f92aed392073
https://hg.mozilla.org/releases/mozilla-aurora/rev/f3c6c6bce22c

beta:
https://hg.mozilla.org/releases/mozilla-beta/rev/e2c93c571158
https://hg.mozilla.org/releases/mozilla-beta/rev/ef6cedb88804

Comment 39

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1b70ac0124fe
https://hg.mozilla.org/mozilla-central/rev/41be1ff63431

Comment 40

[Jonathan Griffin (:jgriffin)]

All patches have landed.