Closed Bug 1430077 Opened 2 years ago Closed 2 years ago

Separate removal of message listeners from session state clearing

Categories

(Testing :: Marionette, enhancement, P1)

Version 3
enhancement

Tracking

(firefox59 fixed)

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: ato, Assigned: ato)

References

Details

Attachments

(4 files)

With the window tracking refactoring we want to let message listeners
live for the duration of the Marionette lifetime rather than the
duration of the session.  To make this happen we need to separate
the removal of message listeners from clearing the session state.
Assignee: nobody → ato
Comment on attachment 8942162 [details]
Bug 1430077 - Broadcast Marionette:Deregister once.

https://reviewboard.mozilla.org/r/212434/#review218524
Attachment #8942162 - Flags: review?(hskupin) → review+
Comment on attachment 8942163 [details]
Bug 1430077 - Remove global outerWindowID in frame script.

https://reviewboard.mozilla.org/r/212436/#review218526
Attachment #8942163 - Flags: review?(hskupin) → review+
Comment on attachment 8942164 [details]
Bug 1430077 - Correct safety checks on Marionette:Register reply.

https://reviewboard.mozilla.org/r/212438/#review218528
Attachment #8942164 - Flags: review?(hskupin) → review+
Comment on attachment 8942165 [details]
Bug 1430077 - Separate clearing session state from deregistering listeners.

https://reviewboard.mozilla.org/r/212440/#review218530

::: testing/marionette/driver.js:2770
(Diff revision 1)
>  GeckoDriver.prototype.deleteSession = function() {
>    if (this.curBrowser !== null) {
>      // frame scripts can be safely reused
>      Preferences.set(CONTENT_LISTENER_PREF, false);
>  
> +    globalMessageManager.broadcastAsyncMessage("Marionette:Session:Clear");

You meant `Marionette:Session:Delete` here, right?

Also this should be sent after `Marionette:Deregister`.
Attachment #8942165 - Flags: review?(hskupin) → review-
Comment on attachment 8942165 [details]
Bug 1430077 - Separate clearing session state from deregistering listeners.

https://reviewboard.mozilla.org/r/212440/#review218530

> You meant `Marionette:Session:Delete` here, right?
> 
> Also this should be sent after `Marionette:Deregister`.

I do mean that, yes.

It can’t be sent after Deregister because that removes the
Session:Delete listener.
[Mass update] Setting P1 as currently being worked on.
Priority: -- → P1
Comment on attachment 8942165 [details]
Bug 1430077 - Separate clearing session state from deregistering listeners.

https://reviewboard.mozilla.org/r/212440/#review218574
Attachment #8942165 - Flags: review?(hskupin) → review+
Pushed by atolfsen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/989f852dc1a8
Broadcast Marionette:Deregister once. r=whimboo
https://hg.mozilla.org/integration/autoland/rev/8c33350d63a2
Remove global outerWindowID in frame script. r=whimboo
https://hg.mozilla.org/integration/autoland/rev/5b775719d432
Correct safety checks on Marionette:Register reply. r=whimboo
https://hg.mozilla.org/integration/autoland/rev/659526d528ff
Separate clearing session state from deregistering listeners. r=whimboo
You need to log in before you can comment on or make changes to this bug.