818800 - Remove the global private browsing service in per-window PB builds

Status: RESOLVED FIXED

(Firefox :: Private Browsing, defect)

Description

[(no longer active)]

This bug is about excluding everything in http://mxr.mozilla.org/mozilla-central/source/browser/components/privatebrowsing/src/ from per-window private browsing builds.

One thing that I'm not sure how to properly do is how to handle removed-files.in.  Basically we need everything associated with the global PB service to go away in per-window PB builds.  This means that we should add nsPrivateBrowsingService.js and nsPrivateBrowsingService.manifest to removed-files.in.  We can put that behind an #ifdef MOZ_PER_WINDOW_PRIVATE_BROWSING, so that's not a problem.  Should I also add nsIPrivateBrowsingService.xpt on that list?  Is it kosher to put this kind of stuff behind an #ifdef in removed-files.in?  Is there anything else that I need to watch out for?

One thing to note is that we're planning to use MOZ_PER_WINDOW_PRIVATE_BROWSING as a kill switch in case of a disaster.  If we need to flip that off when this gets to Aurora/Beta, can we just backout the change here and remove the removed-files.in entries and hope that everything will work correctly?

Comment 1

[Mike Hommey [:glandium]]

removed-files.in is so broken that i'm tempted to tell you to ignore it. Especially since all these files live in omni.ja anyways, and the removed-files.in contents would only be useful for flat or jar chrome format builds.

Comment 2

[Robert Strong (they/them - no direct email)]

btw: we no longer need removed-files.in for the vast majority of cases with Firefox 5 updating to anything greater than Firefox 5... it is only needed for Firefox 4 and below. If we could require Firefox 4 users to update to a specific release before updating to anything newer then this wouldn't be a problem.

I'll talk with a few people about this in the next day or two.

Comment 3

[(no longer active)]

Oh, I see.  Thanks guys!  Should I file a bug to investigate if we can remove the removed-files.in file?  :-)

Comment 4

[Robert Strong (they/them - no direct email)]

(In reply to Ehsan Akhgari [:ehsan] from comment #3)
> Oh, I see.  Thanks guys!  Should I file a bug to investigate if we can
> remove the removed-files.in file?  :-)
No. Update handles the vast majority of cases now but it doesn't handle all cases. Since requiring updates to a specific version for all platforms has been difficult to get buy in for I have started to take a different approach in bug 759469 but I haven't been able to get time to finish that up.

Comment 5

[(no longer active)]

(In reply to comment #4)
> (In reply to Ehsan Akhgari [:ehsan] from comment #3)
> > Oh, I see.  Thanks guys!  Should I file a bug to investigate if we can
> > remove the removed-files.in file?  :-)
> No. Update handles the vast majority of cases now but it doesn't handle all
> cases. Since requiring updates to a specific version for all platforms has been
> difficult to get buy in for I have started to take a different approach in bug
> 759469 but I haven't been able to get time to finish that up.

I see.  Thanks for the explanation!

Comment 6

[Erik Vold [:erikvold] (please needinfo? me)]

So it appears that pwpb UI has landed in Nightly and the feature itself as well, but the global service remains.  The plan is to remove it tho I see, but I'm wondering if there is some good way to tell that the global pb service should not be used in the meantime? which could be used after this bug has closed.

Basically I am looking for something/code that will tell me when I should no longer use nsIPrivateBrowsingService.  Something like MOZ_PER_WINDOW_PRIVATE_BROWSING but available at runtime.

Comment 7

[(no longer active)]

(In reply to Erik Vold [:erikvold] [:ztatic] from comment #6)
> So it appears that pwpb UI has landed in Nightly and the feature itself as
> well, but the global service remains.  The plan is to remove it tho I see,
> but I'm wondering if there is some good way to tell that the global pb
> service should not be used in the meantime? which could be used after this
> bug has closed.

I'm planning to remove it today.  :-)

> Basically I am looking for something/code that will tell me when I should no
> longer use nsIPrivateBrowsingService.  Something like
> MOZ_PER_WINDOW_PRIVATE_BROWSING but available at runtime.

The correct test would be:

  ("@mozilla.org/privatebrowsing;1" in Components.classes)

Comment 8

[(no longer active)]

Attachment: Patch (v1)

Comment 9

[(no longer active)]

I expect this to only fail pdf.js tests: https://tbpl.mozilla.org/?tree=Try&rev=113526f80a2e

Comment 10

[(no longer active)]

(In reply to comment #7)
> (In reply to Erik Vold [:erikvold] [:ztatic] from comment #6)
> > So it appears that pwpb UI has landed in Nightly and the feature itself as
> > well, but the global service remains.  The plan is to remove it tho I see,
> > but I'm wondering if there is some good way to tell that the global pb
> > service should not be used in the meantime? which could be used after this
> > bug has closed.
> 
> I'm planning to remove it today.  :-)

Looks like this did not happen yesterday.  There's a couple of more patches that need to get in before we can land the patch here.  Please see the dependency list of this bug.

Comment 11

[(no longer active)]

Attachment: Part 2: Remove the interface as well

We have now eliminated all of the dependencies on the interface too!

Comment 12

[Josh Matthews [:jdm]]

Comment on attachment 696526 [details] [diff] [review]
Part 2: Remove the interface as well

\o/

Send it through try for safety?

Comment 13

[(no longer active)]

Already have...

Comment 14

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0acf066babea

Comment 15

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/0acf066babea

Comment 16

[Ben Bucksch (:BenB)]

Ehsan, where's the developer doc for the new API, and the warning that the old API is removed?

I can't find anything at all on
https://developer.mozilla.org/en-US/docs/Firefox_20_for_developers
http://mxr.mozilla.org/comm-central/source/mozilla/netwerk/base/public/nsIPrivateBrowsingService.idl
http://mxr.mozilla.org/comm-central/source/mozilla/netwerk/base/public/nsIPrivateBrowsingServiceObsolete.idl
https://developer.mozilla.org/en-US/docs/XPCOM_Interface_Reference/nsIPrivateBrowsingService
https://developer.mozilla.org/en-US/docs/Supporting_private_browsing_mode

All of these pages, and others, should point to a page that explains how to use the new API. But I can't find anything.

Comment 17

[Josh Matthews [:jdm]]

I'll try to update those. Meanwhile: https://developer.mozilla.org/en-US/docs/Supporting_per-window_private_browsing and https://developer.mozilla.org/en-US/docs/Updating_addons_broken_by_private_browsing_changes are where it's at.