462832 - (mozpb) Enable private browsing support for all of the Mozilla-based applications

Status: RESOLVED WORKSFORME

(Firefox :: Private Browsing, defect)

Description

[(no longer active)]

Based on IRC discussions with mconnor, we want to make our private browsing implementation (bug 248970) available to all Mozilla-based applications.  There are (at least) two approaches here: moving the private browsing component to netwerk/ or toolkit/.  The former seems like a better choice because the interface already lives in netwerk/.

This will happen after landing the private browsing code on trunk, and hopefully before the release of Firefox 3.1 (and Gecko 1.9.1).

Comment 1

[(no longer active)]

mconnor: is this something we want to do for Beta 3 or Beta 4?  I had a half-done patch before bug 476463, but with that fix it's obsolete.  I need to be able to prioritize this correctly so that it doesn't get missed for 1.9.1, and doesn't interfere with the rest of my PB work.

Comment 2

[Mike Connor [:mconnor]]

Definitely not b3, I think this is something we should do on trunk now, and once that's done, figure out if it's low-risk enough to move in b4.

Comment 3

[(no longer active)]

Attachment: Patch (v1)

This patch moves the private browsing service core from browser/ to toolkit/, so that other Mozilla-based applications can use it.

Most of this patch deals with actually moving the code from browser/components/privatebrowsing to toolkit/components/privatebrowsing.  I have used hg cp and hg mv in order to provide a readable git style patch to make the review less painful.

The other part of the patch adds a new interface called nsIPrivateBrowsingSession, which is the interface that applications _may_ decide to implement if they want to provide custom private browsing session handling (like we do in Firefox, for example, by closing down everything and opening about:privatebrowsing).  All of the Firefox specific code in nsPrivateBrowsingService has been ported to nsPrivateBrowsingSession, which now lives in browser.

On the test side, unit tests which require browser-specific stuff remain in browser/, and all the others are ported to toolkit/.  I have also added a few tests for nsIPrivateBrowsingSession to cover all the possible cases in which this interface may get called (starting and stopping in normal mode, autostart mode, and with exiting the app without explicitly leaving the PB mode.)

Functionality-wise, this patch doesn't change anything in Firefox, and all the existing unit tests pass with this.

Because of the size of this patch, and its effect on the rest of my work on the PB stuff, I appreciate if you can review this sooner rather than later, even though I know that reviewing this may take some time because of the size of the patch.  Thanks!

Comment 4

[(no longer active)]

Marcia, Henrik: since this is a very big change, I have prepared a try server build with my patch, and it would be great if you can give it some serious test and report if you find any bugs.  Thanks!

<https://build.mozilla.org/tryserver-builds/2009-03-07_09:[e-mail]-try-fb8a209e9eb/>

Comment 5

[(no longer active)]

Attachment: Patch (v1.1)

Small fix to the unit tests.

Comment 6

[(no longer active)]

Attachment: Patch (v1.2)

Oops, also make sure that this doesn't overwrite the fix from bug 480854.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #4)
> Marcia, Henrik: since this is a very big change, I have prepared a try server
> build with my patch, and it would be great if you can give it some serious test
> and report if you find any bugs.  Thanks!
> 
> <https://build.mozilla.org/tryserver-builds/2009-03-07_09:[e-mail]-try-fb8a209e9eb/>

What tests should be done against this build? All major aspects where we have manual tests on Litmus?

Comment 8

[(no longer active)]

(In reply to comment #7)
> What tests should be done against this build? All major aspects where we have
> manual tests on Litmus?

Yes, if possible.  Since all automated tests pass with this patch, ensuring that all manual tests do pass as well will be a requirement in evaluating the safety of this patch.  Thanks!

Comment 9

[TommyBee (No E-mail)]

Any update on this?  It would be nice to see this feature extend to Camino, SeaMonkey, etc.

Comment 10

[Michael Kohler [:mkohler]]

This is a mass change. Every comment has "assigned-to-new" in it.

I didn't look through the bugs, so I'm sorry if I change a bug which shouldn't be changed. But I guess these bugs are just bugs that were once assigned and people forgot to change the Status back when unassigning.

Comment 11

[Patrick Dreier]

Please resolve this. Mozilla Firefox i have this function.

Comment 12

[(no longer active)]

This bug is about making it possible for Mozilla-based applications other than Firefox to support private browsing.

Comment 13

[Ian Melven :imelven]

Attachment: WIP - updated patch that builds against recent m-c

I updated this patch to apply cleanly and build against a current m-c, i'm sticking it in here as a WIP. I'm working on this bug with the hope of using it to get private browsing support in Fennec (see bug 582244). The next steps i'll be taking with this patch are getting the private browsing tests in /browser and /toolkit to all pass.

Comment 14

[(no longer active)]

(In reply to Ian Melven :imelven from comment #13)
> Created attachment 571169 [details] [diff] [review]
> WIP - updated patch that builds against recent m-c
> 
> I updated this patch to apply cleanly and build against a current m-c, i'm
> sticking it in here as a WIP. I'm working on this bug with the hope of using
> it to get private browsing support in Fennec (see bug 582244). The next
> steps i'll be taking with this patch are getting the private browsing tests
> in /browser and /toolkit to all pass.

Friendly ping to see if anything happened here since this patch.  :-)

Comment 15

[Ian Melven :imelven]

(In reply to Ehsan Akhgari [:ehsan] from comment #14)
> (In reply to Ian Melven :imelven from comment #13)
> > Created attachment 571169 [details] [diff] [review]
> > WIP - updated patch that builds against recent m-c
> > 
> > I updated this patch to apply cleanly and build against a current m-c, i'm
> > sticking it in here as a WIP. I'm working on this bug with the hope of using
> > it to get private browsing support in Fennec (see bug 582244). The next
> > steps i'll be taking with this patch are getting the private browsing tests
> > in /browser and /toolkit to all pass.
> 
> Friendly ping to see if anything happened here since this patch.  :-)

nope :( i've been working on bug 341604 (IFRAME sandbox) instead when i have time to do coding between security reviews. i still want to pick this back up when IFRAME sandbox moves along to the review/iterate stage - but in the meantime anyone else is welcome to have at it - i'd be happy to pass on the info/background that i can :)

Comment 16

[(no longer active)]

If you're willing to mentor somebody through this, I can see if Josh knows any contributors who will be willing to work on it.  :-)

(Note that I will definitely be available to help them as well!)

Comment 17

[Ian Melven :imelven]

(In reply to Ehsan Akhgari [:ehsan] from comment #16)
> If you're willing to mentor somebody through this, I can see if Josh knows
> any contributors who will be willing to work on it.  :-)
> 
> (Note that I will definitely be available to help them as well!)

i would totally be willing to mentor someone as well as i can :)

Comment 18

[(no longer active)]

Great!

Josh, do you happen to know somebody who's willing to work on this?  It may not be a good choice as a first bug, not because of the complexity of the work, but because it's not exactly going to be fixed with the smallest patch ever.  :-)

Comment 19

[Ian Melven :imelven]

Just a note that it may not make sense for a contributor to work on this bug at this time, since Josh's work in bug 463027 also moves private browsing to platform where other Mozilla products can use it as well as making private browsing per window.

Comment 20

[Ian Melven :imelven]

Removing (In reply to Ian Melven :imelven from comment #19)
> Just a note that it may not make sense for a contributor to work on this bug
> at this time, since Josh's work in bug 463027 also moves private browsing to
> platform where other Mozilla products can use it as well as making private
> browsing per window.

removing [mentor=imelven][lang=js] for this reason.

Comment 21

[Abhishek Madan[:abhishekmadan]]

Private Browsing is available on all versions of Firefox across all its platforms