799784 - Replace usages of nsIPrivateBrowsingService.autoStarted with PrivateBrowsingUtils.permanentPrivateBrowsing

Status: RESOLVED FIXED

(Firefox :: Private Browsing, defect)

Description

[(no longer active)]

This should be fairly straightforward with the API added in bug 799780.  Here's the list of current places where this is used:

http://mxr.mozilla.org/mozilla-central/search?string=autoStarted

Note that not all of those places need to be modified, we're basically trying to modify the consumers of the API.

Comment 1

[Andres Hernandez [:andreshm]]

Attachment: Patch v1

Just changed the consumers. I kept the nsPrivateBrowsingService::autoStarted value and tests related, is this going to be removed in another bug?

Comment 2

[(no longer active)]

(In reply to Andres Hernandez [:andreshm] from comment #1)
> Created attachment 674738 [details] [diff] [review]
> Patch v1
> 
> Just changed the consumers. I kept the nsPrivateBrowsingService::autoStarted
> value and tests related, is this going to be removed in another bug?

No, we're planning to just not build nsPrivateBrowsingService at all in per-window PB bugs.  That should implicitly take care of that!

Comment 3

[(no longer active)]

Comment on attachment 674738 [details] [diff] [review]
Patch v1

Review of attachment 674738 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great overall!  I just have a couple of nits.

::: browser/components/preferences/in-content/security.js
@@ +89,5 @@
>    {
>      var pref = document.getElementById("signon.rememberSignons");
>      var excepts = document.getElementById("passwordExceptions");
>  
> +    Components.utils.import("resource://gre/modules/PrivateBrowsingUtils.jsm");

Nit: please move this to the top of the file (same as in the other copy of security.js.

::: browser/components/sessionstore/src/nsSessionStartup.js
@@ +39,5 @@
>  Cu.import("resource://gre/modules/Services.jsm");
>  Cu.import("resource://gre/modules/TelemetryStopwatch.jsm");
>  
> +XPCOMUtils.defineLazyModuleGetter(this, "PrivateBrowsingUtils",
> +  "resource://gre/modules/PrivateBrowsingUtils.jsm");

Nit: please just use Cu.import, as this is going to be used in the initialization anyway.

Comment 4

[Andres Hernandez [:andreshm]]

Attachment: Patch v2

Comment 5

[(no longer active)]

Comment on attachment 674805 [details] [diff] [review]
Patch v2

Review of attachment 674805 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great, thanks!

Comment 6

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c90c5809e56e

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c90c5809e56e