nsISessionStore uses private methods and is not extensible

RESOLVED FIXED in Firefox 3.1a2

Status

()

Firefox
Session Restore
--
enhancement
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: Mike Perry, Assigned: Simon Bünzli)

Tracking

(Blocks: 1 bug, {dev-doc-complete})

Trunk
Firefox 3.1a2
dev-doc-complete
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

9 years ago
The @mozilla.org/browser/sessionstore;1 component is very difficult to extend because it uses non-IDL published versions of all its calls internally. This makes it impossible for an addon to easily alter its behavior by reimplementing a couple methods. 

In my particular case, I need to prevent the session store from writing out tabs loaded during Tor mode in the Torbutton Firefox Extension (similar to the behavior planned in Bug 248970). Unfortunately, the component does not publish the function it uses to write the tabs to disk via its IDL. Worse, none of the published methods are used internally by the component itself, so reimplementing them by Contract ID hooking is not possible.

The end result is that Torbutton ultimately has to include a copy of the SessionStore for Firefox 2 and 3, keep it up to date, and determine the proper one to load at browser start. All of this for what amounts to about a 5 line diff to the actual code itself.

For further reference, the Torbutton bug filed by a developer encouraging me to explore alternatives (and my rationale for not being able to use any of them) is here: https://bugs.torproject.org/flyspray/index.php?do=details&id=722
(Assignee)

Comment 1

9 years ago
Created attachment 332722 [details] [diff] [review]
add sessionstore-state-(read|write) notifications

One possible solution for this issue would be allowing extensions to hook into the reading/writing of sessionstore.js through notifications. That's what this patch implements.

In case there's a better solution, I'd be glad to hear about it, though...

On a side note: The "sessionstore-state-read" notification would also replace the restore_prompt_uri pref which I'd like to drop once we use an error page instead of the prompt in bug 448976.
Attachment #332722 - Flags: review?(gavin.sharp)
(Assignee)

Updated

9 years ago
Attachment #332722 - Flags: review?(dietrich)
Comment on attachment 332722 [details] [diff] [review]
add sessionstore-state-(read|write) notifications

this seems like a reasonable approach for the pre-deserialization and post-serialization use-cases. the writing notification is testable via mochitest, so please add that to this patch. it will also serve as example for extension developers.
Attachment #332722 - Flags: review?(dietrich)
(Assignee)

Comment 3

9 years ago
Created attachment 334490 [details] [diff] [review]
includes a test
Assignee: nobody → zeniko
Attachment #332722 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #334490 - Flags: review?(dietrich)
Attachment #332722 - Flags: review?(gavin.sharp)
Comment on attachment 334490 [details] [diff] [review]
includes a test

r=me, thanks for adding the test.
Attachment #334490 - Flags: review?(dietrich) → review+
Keywords: checkin-needed
Hardware: PC → All
Pushed as 17138:181796ac1c8a.
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.1a2
(Assignee)

Comment 6

9 years ago
So, there are now two new notifications:
* sessionstore-state-read is dispatched right after data is read from sessionstore.js and before it's used internally
* sessionstore-state-write is dispatched right before the data is written to disk

In both cases, extensions can modify that data by getting/setting the notification's subject's data member (the subject is an nsISupportsString). They can either work on that string directly or eval it into a JS object and in the end toSource() the object again. Setting the data member to an empty string will either prevent session restoration (in the -read case) resp. it will prevent the file sessionstore.js from being (over)written.
Keywords: dev-doc-needed
(Assignee)

Updated

9 years ago
Blocks: 426045
(Assignee)

Updated

9 years ago
Blocks: 462702
Documented now.  See https://developer.mozilla.org/en/Observer_Notifications#Session_Store
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.