If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

page-mods need a way to alert content-scripts when the add-on in unloaded

RESOLVED FIXED

Status

Add-on SDK
General
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: erikvold, Assigned: erikvold)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

page-mods need a way to alert content-scripts when the add-on in unloaded so that the page-mods can undo changes that they make to the page DOMs.

Use case https://github.com/rbudiharso/javascript-view

This add-on adds line numbers to the DOM (with my patch) which need to be remove on unload, when the add-on is disabled/uninstalled.
Blocks: 902217
Whiteboard: [good first bug]
is this even possible with the current jetpack and PageMod architecture? 

as far as i can tell, upon addon unloading, PageMods get destroyed, which means Workers get detached and destroyed, which means the sandbox in which content-script was executing gets destroyed.

even if we emit an "undo" event, will there by any listener alive to react to it?

or should PageMod wait for content-script to process the event, and delay the addon unloading? (surely not)

i don't see a proper (event-driven/async) way to do this. 

maybe call some (registered) function from the content-script synchronously, before destroying the sandbox? (hacky/ugly)
Flags: needinfo?(evold)
(In reply to Tomislav Jovanovic [:zombie] from comment #1)
> is this even possible with the current jetpack and PageMod architecture? 
> 
> as far as i can tell, upon addon unloading, PageMods get destroyed, which
> means Workers get detached and destroyed, which means the sandbox in which
> content-script was executing gets destroyed.
> 
> even if we emit an "undo" event, will there by any listener alive to react
> to it?
> 
> or should PageMod wait for content-script to process the event, and delay
> the addon unloading? (surely not)
> 
> i don't see a proper (event-driven/async) way to do this. 
> 
> maybe call some (registered) function from the content-script synchronously,
> before destroying the sandbox? (hacky/ugly)

The page-mod is alive as long as it isn't nuked I think, so it should be alive/around for 1sec after the unload (when we do the nuking https://github.com/mozilla/addon-sdk/blob/master/app-extension/bootstrap.js#L307 ), so an async call isn't awful here.  Anyhow the "async" emit (which I think may be sync atm) to the page-mod would occur during the "unload" addon event, so if in the e10s world where the page-mod destruction/nuking were to occur it would have to be async as well, so the page-mod still would have a window of opportunity to react to the event and do some cleaning.

Also, I don't think we need to emit the "unload" event to page-mods when the browser is being shutdown, at least for the moment.
Flags: needinfo?(evold)
Assignee: nobody → evold
OS: Mac OS X → All
Hardware: x86 → All
Blocks: 708192
Created attachment 8372940 [details] [review]
https://github.com/mozilla/addon-sdk/pull/1390
Attachment #8372940 - Flags: review?(rFobic)
Whiteboard: [good first bug]
Blocks: 792224
Attachment #8372940 - Flags: review?(rFobic) → review+

Comment 4

4 years ago
Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/06513f8341196eb7a4a4dd99c97c0140d19c5e58
Bug 964118 - page-mods need a way to alert content-scripts when the add-on in unloaded

https://github.com/mozilla/addon-sdk/commit/a0abc9cfa1db5ad8f1062b54a4becb4533de6120
Merge pull request #1390 from erikvold/964118

Bug 964118 - page-mods need a way to alert content-scripts when the add-on in unloaded r=@gozala

Updated

4 years ago
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Depends on: 972925

Comment 5

3 years ago
As of Iceweasel 31.1.0, the `detach` event doesn't appear to be fired to the content script when its associated page gets closed.

E.g.

// I'm content script
self.on("detach", function() {
    // this branch is never ever executed
});
You need to log in before you can comment on or make changes to this bug.