Closed Bug 1426154 Opened 2 years ago Closed 2 years ago

Use targetted message manager to communciate with content browsers


(Testing :: Marionette, enhancement)

Version 3
Not set


(firefox59 fixed)

Tracking Status
firefox59 --- fixed


(Reporter: ato, Assigned: ato)


(Blocks 1 open bug)



(3 files, 1 obsolete file)

Marionette currently uses a message manager broadcaster to communicate
with content browsers.  This is hugely wasteful and fragile,
especially because we use the content browser’s outerWindowID as
the marker for which frame script picks up the message because it
changes when the browser suffers a remoteness change.

We should instead use a targetted
message manager.  The window refactoring work in
makes this possible.

One problem the window refactoring does not take care of is the
Marionette:listenersAttached and this.curBrowser.pendingCommands queue.
I believe we still need some version of this, in case the content
browser disappears whilst processing a message we have sent it.
We are still liable to see this despite improving the tracking of
browsers and windows.
Btw the framescript could listen for the `unload` event on the tabchildglobal to send the parent a message about its unload. That should help us to better handle such situations.
We could perhaps use the unload event to tell chrome to delay, or
more likely re-send, the last message.  Provided it fires consistently,
of course.
I tested that locally for a patch on a different bug, and it was working fine. Only sending the data to the parent process wasn't working that well at that time.
It looks like we might be able to employ a targetted message
manager for communicating directly with content frames without the
window tracking refactoring.  It might make sense to reverse the
relationship and try to land this first.
Assignee: nobody → ato
No longer depends on: marionette-window-tracking
Blocks: 1157192
Great to see the progress on this bug Andreas! I hope that this will already fix the multi-process issues which I have seen on bug 1422769.
Blocks: 1422769
Taskcluster had some kind of major cockup yesterday that ruined
the test run.  Triggered a new one.
Comment on attachment 8941180 [details]
Bug 1426154 - Use targetted IPC messages to talk to content frames.
Attachment #8941180 - Flags: review?(dburns) → review+
Comment on attachment 8941181 [details]
Bug 1426154 - Sort message listeners lexicographically.
Attachment #8941181 - Flags: review?(dburns) → review+
Comment on attachment 8941182 [details]
Bug 1426154 - Refactor Marionette content frame script registration.
Attachment #8941182 - Flags: review?(dburns) → review+
Comment on attachment 8941183 [details]
Bug 1426154 - Rename Marionette:deleteSession to Marionette:Deregister.
Attachment #8941183 - Flags: review?(dburns) → review+
Attachment #8941183 - Attachment is obsolete: true
Pushed by
Use targetted IPC messages to talk to content frames. r=automatedtester
Sort message listeners lexicographically. r=automatedtester
Refactor Marionette content frame script registration. r=automatedtester
You need to log in before you can comment on or make changes to this bug.