Closed Bug 1090147 Opened 8 years ago Closed 8 years ago

switch tab.attach() to use the new async Worker (e10s)

Categories

(Add-on SDK Graveyard :: General, defect, P1)

defect

Tracking

(e10sm4+)

RESOLVED FIXED
mozilla36
Tracking Status
e10s m4+ ---

People

(Reporter: zombie, Assigned: zombie)

References

Details

Attachments

(1 file)

No description provided.
Priority: -- → P1
i'm a bit sad that this requires a workaround. 

i hoped we would be able to keep a clean separation, all the content-touching/observing code in the worker-child.js, forwarding all the needed info to the parent.

unfortunately, when a tab gets closed, the message channel gets destroyed immediately, so by the time WorkerChild receives the "inner-window-destroyed" notifications, detaches, and tries to send the "detach" event to the parent, the communication fails.

the workaround is that worker-parent needs an out-of-band way to get notified, to ensure we send the "detach" event to the addon.
Assignee: nobody → tomica+amo
Status: NEW → ASSIGNED
Attachment #8512573 - Flags: review?(dtownsend+bugmail)
(In reply to Tomislav Jovanovic [:zombie] from comment #1)
> Created attachment 8512573 [details] [review]
> Link to Github pull-request: https://github.com/mozilla/addon-sdk/pull/1690
> 
> i'm a bit sad that this requires a workaround. 
> 
> i hoped we would be able to keep a clean separation, all the
> content-touching/observing code in the worker-child.js, forwarding all the
> needed info to the parent.
> 
> unfortunately, when a tab gets closed, the message channel gets destroyed
> immediately, so by the time WorkerChild receives the
> "inner-window-destroyed" notifications, detaches, and tries to send the
> "detach" event to the parent, the communication fails.

The browser message channel gets destroyed, the process message channel is still alive though. Does that help at all?

> the workaround is that worker-parent needs an out-of-band way to get
> notified, to ensure we send the "detach" event to the addon.

If I'm reading this right we rely on inner-window-destroyed being dispatched for a child process window in the parent process. I'm surprised that that happens. Am I missing something?
> The browser message channel gets destroyed, the process message channel is
> still alive though. Does that help at all?

yes, it's easy enough to use a cpmm/ppmm combo for this one thing. but that would result in two separate communication channels between worker-parent and -child, which leads to inevitable questions of refactoring (to use just the process channel), which leads to questions of performance (it's probably more optimal to use the browser channel for most of the communication), and is a whole Pandora's box that i don't feel like opening right now.


> If I'm reading this right we rely on inner-window-destroyed being dispatched
> for a child process window in the parent process. I'm surprised that that
> happens. Am I missing something?

yes, that is part of the shims implemented for (all) addons that forward some subset of observer notifications from the child to the parent process. for example, the page-mod module currently works by using those without too much changes (apart from the new async Worker).


using that shim for this is essentially punting on the whole issue described above, until we are at Stage II (removing dependance on CPOWs/shims), at which point i expect we will have a better understanding of the whole problem-space to judge the pros/cons better.


hope that sound reasonable?
Attachment #8512573 - Flags: review?(dtownsend+bugmail) → review+
Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/24f671dc24e49e127a71de975c8956cda1435057
bug 1090147 - switch tab.attach to use async Worker

https://github.com/mozilla/addon-sdk/commit/f7171f86dcc2fa8a0adb8bda859275ac1db3f3ea
Merge pull request #1690 from zombie/1090147-tab-attach

bug 1090147 - switch tab.attach to use async Worker, r=@Mossop
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Blocks: 1057107
You need to log in before you can comment on or make changes to this bug.