Closed Bug 1534635 Opened 5 years ago Closed 5 years ago

Send custom "marionette-startup-requested" observer notification to start Marionette in Fennec

Categories

(Firefox for Android Graveyard :: General, enhancement, P1)

enhancement

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 1530979

People

(Reporter: whimboo, Unassigned)

References

Details

As of now Marionette listens for a couple of startup observer notifications, and has to wait until the startup recorder has finished recording startup scripts:

https://searchfox.org/mozilla-central/rev/dbddac86aadf1d4871fb350bbe66db43728a9f81/testing/marionette/components/marionette.js#449-456

James or Nick, where would be the best place to send this notification?

Flags: needinfo?(snorp)
Flags: needinfo?(nalexander)

So you want to know when geckoview is finished starting, right? I think "browser-delayed-startup-finished"[1] is probably fine.

[1] https://searchfox.org/mozilla-central/rev/8ff2cd0a27e3764d9540abdc5a66b2fb1e4e9644/mobile/android/chrome/geckoview/geckoview.js#467

Flags: needinfo?(snorp)

No, this is for Fennec and not Geckoview.

Flags: needinfo?(snorp)

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

No, this is for Fennec and not Geckoview.

Ah, ok. Well Fennec does the same thing. https://searchfox.org/mozilla-central/rev/8ff2cd0a27e3764d9540abdc5a66b2fb1e4e9644/mobile/android/chrome/content/browser.js#523

Flags: needinfo?(snorp)

Ok, just to wrap up the question you asked in comment 1 which I missed to reply. We don't want to listen for a specific observer notification but we want to let application send a specific notification for Marionette. As the topic of the bug mentions it will be "marionette-startup-requested".

In case of Firefox we delay that to after all the startup scripts have been loaded. The code you referenced in the last comment shows some more scripts to be run via initLater(). Would it hurt the startup time when we send the notification too early? Should we maybe move it to after browser-idle-startup-tasks-finished?

Also for GeckoView we do it here, and I wonder if that may also be too early:

https://searchfox.org/mozilla-central/rev/8ff2cd0a27e3764d9540abdc5a66b2fb1e4e9644/mobile/android/modules/geckoview/GeckoViewRemoteDebugger.jsm#67

Flags: needinfo?(snorp)

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

Ok, just to wrap up the question you asked in comment 1 which I missed to reply. We don't want to listen for a specific observer notification but we want to let application send a specific notification for Marionette. As the topic of the bug mentions it will be "marionette-startup-requested".

Ah, ok, I'm following now.

In case of Firefox we delay that to after all the startup scripts have been loaded. The code you referenced in the last comment shows some more scripts to be run via initLater(). Would it hurt the startup time when we send the notification too early? Should we maybe move it to after browser-idle-startup-tasks-finished?

It depends on where we're measuring, but yeah, it could potentially hurt startup perf. AFAIK we usually cut off "startup" for Fennec/GV to be "chrome document loaded", which would happen before this.

Also for GeckoView we do it here, and I wonder if that may also be too early:

https://searchfox.org/mozilla-central/rev/8ff2cd0a27e3764d9540abdc5a66b2fb1e4e9644/mobile/android/modules/geckoview/GeckoViewRemoteDebugger.jsm#67

That's also probably fine since it's deferred to a future iteration, but it may also be worthwhile it to use the InitLater stuff there.

Flags: needinfo?(snorp)

Thank you for the information James! I had a try to get this implemented for Fennec and noticed that right now we also depend on the sessionstore-windows-restored notification, and would try to initialize Marionette twice.

To not have a temporary workaround just for Fennec it would be good to get Firefox, Fennec, and maybe GeckoView patched in the same patch series. That said I will do it on bug 1530979.

Status: NEW → RESOLVED
Closed: 5 years ago
Flags: needinfo?(nalexander)
Resolution: --- → DUPLICATE
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.