Last Comment Bug 766088 - Content scripts are kept "alive" when going back/forward in history
: Content scripts are kept "alive" when going back/forward in history
Status: RESOLVED FIXED
:
Product: Add-on SDK
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All All
P1 normal with 1 vote (vote)
: 1.11
Assigned To: Alexandre Poirot [:ochameau]
:
:
Mentors:
Depends on:
Blocks: 693345
  Show dependency treegraph
 
Reported: 2012-06-19 06:40 PDT by Alexandre Poirot [:ochameau]
Modified: 2012-09-19 08:13 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Pull request 469 (165 bytes, text/html)
2012-06-19 10:28 PDT, Alexandre Poirot [:ochameau]
evold: review+
Details

Description User image Alexandre Poirot [:ochameau] 2012-06-19 06:40:41 PDT
For now, content scripts were only destroyed/detached when the matched document was removed from bfcache, so that it will still be alive and working if we get back to this document. (And it won't be created twice: one on page loading, and another time when we browse back to this document in history)
This choice introduce a quite important issue. When the user goes back or forth in history, the content script will still "work" on websites that don't necessary match page-mod rules. By working I mean that the script can still fire timeouts, interval, messages, events and eventually listen to some DOM events.

I think that we will have to introduce some specifics in order to handle nicely the history usecase. We do want to keep content script around until the document is removed from bfcache, mainly to avoid cases where we would apply a content twice to the same document instance. For ex, when we go back and immediatly forth in history.
So we want to keep it "around", but like documents in bfcache, we do want to freeze the content script so that timeouts and events are temporarily disabled.
It will allow to pause any computation made by the content script if the matched document is hidden and has the side effect to avoid any access to other documents in history!
We may want to be extra safe about this particular point. We may want to ensure that content script can only have access to inner window object. So that it will never to able to get access to other windows in history.

Last but not least, it would allow us to offer a way better support in page-mod for content script that need to have specific behavior between page being hidden/showed from history, and, page being destroyed (tab/window close).
Comment 1 User image Alexandre Poirot [:ochameau] 2012-06-19 10:28:16 PDT
Created attachment 634493 [details]
Pull request 469
Comment 2 User image Alexandre Poirot [:ochameau] 2012-06-19 10:42:58 PDT
This patch introduce 3 new events on content script side: detach, pageshow, pagehide.
- pagehide is fired when the user open a link, or go back/forward in history. The related document is kept in bfcache and may still be shown later on.
- pageshow: is fired when the related document is shown again. i.e. when the user go back or forward in history and open up this document again.
- detach: is fired when the document is destroyed. i.e. removed from bfcache. The content script is totally destroyed so that it should stop any possible activity, as the related document is destroyed too and won't be available anymore. It mainly happens when the tab/window is closed.

pageshow and pagehide are dispatched on addon side too.
You can listen to these events with:
self.on("pageshow", function onPageShow() { ... }); // from content script
worker.on("pagehide", function onPageHide() { ... }); // from addon module

Then, this patch freeze the content script on pagehide and unfreeze it on pageshow.
It basically reproduce what is done for the document itself: all setTimeout and setInterval are disabled during the freeze and restored on unfreeze. All DOM events should be disabled as well, but at first sight, there is no need to do that, the platform does this for us. It isn't done for timers as we are *not* using document ones. Finally we only have to disable message/event API (postMessage/emit).
I've decided in this patch to print warning messages when the addon try to send messages to a frozen content script, but we may decided to either throw exception, do nothing, or keep message until content script is alive again?
Comment 3 User image John Nagle 2012-08-08 21:45:05 PDT
The description of this in the release notes, at 

https://wiki.mozilla.org/Labs/Jetpack/Release_Notes/1.9

says

"Content scripts are only destroyed/detached when the matched document is removed from the bfcache, so that it will still be alive and working if we get back to this document. (And it won't be created twice: once on page loading, and another time when we browse back to this document in history).

This choice introduce a quite important issue. When the user goes back or forth in history, the content script will still "work" on websites that don't necessary match page-mod rules. Meaning that the script can still fire timeouts, interval, messages, events and eventually listen to some DOM events. "

I thought the plan was that this would PREVENT content scripts from being applied to web sites that don't match the page-mode rules.  Are the release notes wrong?
Comment 4 User image John Nagle 2012-08-08 21:53:21 PDT
Did this go into SDK 1.9?    

And it's listed as a block for 

https://bugzilla.mozilla.org/show_bug.cgi?id=693345

which it was supposed to fix.  What's the status here?
Comment 5 User image Alexandre Poirot [:ochameau] 2012-08-20 04:45:58 PDT
(In reply to John Nagle from comment #3)
> I thought the plan was that this would PREVENT content scripts from being
> applied to web sites that don't match the page-mode rules.  Are the release
> notes wrong?

It didn't went into 1.9. It is listed in "Known issues" list. So that it describe the current broken behavior.
Comment 6 User image Alexandre Poirot [:ochameau] 2012-08-20 04:53:00 PDT
Comment on attachment 634493 [details]
Pull request 469

Erik, I was wondering if you were handling this corner case in scriptish?
What's your thoughts about such approach?
Comment 7 User image John Nagle 2012-09-18 09:58:17 PDT
Is this fixed in SDK 1.10?
Comment 8 User image [github robot] 2012-09-18 12:33:16 PDT
Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/847d8fa873a257006fe5bb9917acfef36046eb5d
Bug 766088: freeze content scripts when the page is hidden in history.

https://github.com/mozilla/addon-sdk/commit/6d65a6e17cb46ae8ff51cf5fceed24f82ef5ec1e
Merge pull request #469 from ochameau/bug/766088-freeze-content-scripts

Fix Bug 766088: freeze content scripts when the page is hidden in history. r=@erikvold
Comment 9 User image Alexandre Poirot [:ochameau] 2012-09-19 08:13:07 PDT
(In reply to John Nagle from comment #7)
> Is this fixed in SDK 1.10?

Unfortunately, no. But a bunch of page-mod/content scripts improvement are going to be shipped in 1.11 release. You can get an overview of them here:
  http://test.techno-barje.fr/post/2012/09/16/1.11-page-mod-release/

Note You need to log in before you can comment on or make changes to this bug.