967970 - Set NSDisablePersistence to prevent disk leaks for non-Firefox branded builds

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mike Perry]

Attachment: Add plist property to always disable Saved Application State.

In https://trac.torproject.org/projects/tor/ticket/8987, we discovered that Tor Browser's window titles were leaking to the "~/Library/Saved Application State/org.mozilla.torbrowser.savedState/" directory. We ran some tests, and discovered that none of the other major browsers were leaking in this way, even in their non-private modes.

A pseudonymous contributor got to the bottom of this and found that Apple was actually blacklisting all of the browsers (including their own) from being written to this directory via hardcoded strings in the AppKit SDK.

Since we changed our branding string from org.mozilla.firefox to org.mozilla.torbrowser, we were not longer except from this, and our state got recorded.

It turns out the fix is to add the key '<key>NSDisablePersistence</key><true/>' to the plist file in the source tree. This causes even non-Firefox branded builds to be exempt from 'Saved Application State'.

Presumably this issue would also apply to Nightly and Aurora builds, and possibly Beta as well, but perhaps some or all of those brandings are also blacklisted by Apple.

Comment 1

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 8370481 [details] [diff] [review]
Add plist property to always disable Saved Application State.

This looks fine to me.  However I'd prefer that you change the syntax as follows, to match what already exists in our Info.plist files:

  <key>NSDisablePersistence</key>
  <true/>

Since we don't currently support Apple-style saved application state, I don't see how this can do any harm.  But we'll have to revisit this patch if and when we do decide to support it.

As your "pseudonymous contributor" says, this key is undocumented.  But by grepping for NSDisablePersistence in the /System/Library directory (on OS X 10.8.5), I found a bunch of Apple apps that use this key (in their Info.plist):

  loginwindow.app
  QuickLookUIHelper.app
  quicklookd.app
  quicklookd32.app

I also found a non-exported BOOL __NSEnablePersistentUI() method in the AppKit framework that allows the value of this key (in an app bundle's Info.plist), if it exists, to override any other setting.

Comment 2

[Steven Michaud [:smichaud] (Retired)]

> BOOL __NSEnablePersistentUI()

BOOL _NSEnablePersistentUI()

Comment 3

[Justin Dolske [:Dolske]]

Random thought: if we ever wanted to explicitly enable this (i.e., opt-in to OS X's application state stuff), we'd need a way to disable it on a per-Window basis. But for now disabling it for all flavors of Firefox-based apps with a big hammer is fine.

Comment 4

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #3)

I don't know a whole lot about Apple-style saved application state.  But that said ...

If we start supporting it, I suppose we'd need to dynamically turn it off globally whenever we're in private browsing mode, and turn it on again when we're not.  And if whether or not we're in private browsing mode is context-specific, we'd have to turn it off and on depending on the context.

As best I can tell, it's windows that can be "private" or not.  If so we shouldn't have any trouble.  But if some of the tabs in a window can be "private" and others not, it might get tricky.

Comment 5

[Steven Michaud [:smichaud] (Retired)]

Knowing Apple, we might have to use private APIs to get fine-grained dynamic control over Apple-style saved application state.  But we can cross that bridge when we come to it.  And if these private APIs are needed and available, Apple will surely be using them in its own apps (like Safari).

Comment 6

[(no longer active)]

(In reply to Steven Michaud from comment #4)
> As best I can tell, it's windows that can be "private" or not.  If so we
> shouldn't have any trouble.  But if some of the tabs in a window can be
> "private" and others not, it might get tricky.

Technically, each docshell can be in private or non-private mode, which theoretically means that you can put individual tabs into private mode even if the window itself is not, but there are a number of things which are currently broken in that model, and in Firefox we will never have that situation unless an add-on does that behind our backs.

Comment 7

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 8370481 [details] [diff] [review]
Add plist property to always disable Saved Application State.

Landed on mozilla-inbound with my suggested change:
https://hg.mozilla.org/integration/mozilla-inbound/rev/186096a5f6d0

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/186096a5f6d0