815053 - social worker doesn't get new localStorage database in private browsing mode

Status: RESOLVED WONTFIX

(Firefox Graveyard :: SocialAPI, defect)

Description

[Mark Hammond [:markh] [:mhammond] [PTO until Sept 7]]

Attachment: social test demonstrating the problem

When you enter PB mode and enable social, the social worker has the same localStorage database as the worker when not in PB mode.

This almost certainly isn't a bug directly in social, and probably due to some of the ongoing "PB per window" work, but opening it as a social bug until more info is available.  This is only an issue on nightlies - Aurora and 17 are not impacted (and presumably beta also isn't).

I have a social test which demonstrates the problem, and I'm working on finding a regression range.

Comment 1

[Mark Hammond [:markh] [:mhammond] [PTO until Sept 7]]

It seems the bug did exist on Aurora until bug 813533 landed (specifically, 118338:e38abb2dab15) - but it seems the second attachment from that bug (which is backing out bug 795556!) didn't land on trunk, only on Aurora, hence the bug remains in nightly.  It seems bug 814275 is also a related regression, but the patch there doesn't fix this.

But all that confusion still leaves us in the position that per-window private browsing is going to mean that a single "shared worker" being used by both PB and non-PB windows is a problem.

I'm not sure where to go next on this...

Comment 2

[(no longer active)]

(In reply to Mark Hammond (:markh) from comment #1)
> It seems the bug did exist on Aurora until bug 813533 landed (specifically,
> 118338:e38abb2dab15) - but it seems the second attachment from that bug
> (which is backing out bug 795556!) didn't land on trunk, only on Aurora,
> hence the bug remains in nightly.  It seems bug 814275 is also a related
> regression, but the patch there doesn't fix this.

This is most likely a dupe of bug 814275, which we have a fix for (which I'll land shortly.)

> But all that confusion still leaves us in the position that per-window
> private browsing is going to mean that a single "shared worker" being used
> by both PB and non-PB windows is a problem.

Can you please explain what the higher level plan is which requires this implementation?  Thanks!

Comment 3

[Mark Hammond [:markh] [:mhammond] [PTO until Sept 7]]

(In reply to Ehsan Akhgari [:ehsan] from comment #2)
> (In reply to Mark Hammond (:markh) from comment #1)

> > But all that confusion still leaves us in the position that per-window
> > private browsing is going to mean that a single "shared worker" being used
> > by both PB and non-PB windows is a problem.
> 
> Can you please explain what the higher level plan is which requires this
> implementation?  Thanks!

The social component has a mocked-up implementation of a "Shared Worker", hosted in the hidden window, that all social content components use (eg, the sidebar gets a "port" to the worker used for communication between the 2.)

In per-window PB mode, a single shared worker isn't appropriate - a sidebar in PB mode will want to communicate with a shared worker also in PB mode (ie, that shares the same cookie jar), and ditto for a sidebar/worker not in PB mode.

I hope that answers the question you asked :)

Comment 4

[(no longer active)]

I see.  But what is the UX that we want to provide here?  Letting users log into two different accounts, one for non-private windows and one for private windows?  I've been thinking about this for the past while, and I think it probably makes the most sense to disable the social UI in private windows completely.  This is sort of sucky for permanent private browsing, so we might want to special case that, but that would be fine since in those types of sessions, we only have private windows, so the hidden window itself is also private.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

(In reply to Ehsan Akhgari [:ehsan] from comment #4)
> I see.  But what is the UX that we want to provide here?  Letting users log
> into two different accounts, one for non-private windows and one for private
> windows?  I've been thinking about this for the past while, and I think it
> probably makes the most sense to disable the social UI in private windows
> completely.

Yes, that's definitely the best first step.

Comment 6

[Shane Caraveo (:mixedpuppy)]

I think the fix for social, if not disabling social, would be to change getFrameWorkerHandle in FrameWorker.jsm to use a per-window workerCache for those windows that are in private browsing mode.  Upside is each private window could have separate accounts.  Downside is memory bloat when doing so.

Comment 7

[Shane Caraveo (:mixedpuppy)]

adding to the above, the worker iframe would be attached to the pb window rather than to the hidden window.

Comment 8

[(no longer active)]

(In reply to comment #7)
> adding to the above, the worker iframe would be attached to the pb window
> rather than to the hidden window.

/me likes this!

Comment 9

[Mark Hammond [:markh] [:mhammond] [PTO until Sept 7]]

Attachment: Slightly updated tests

FWIW, bug 814275 doesn't seem to have fixed this.  This new attachment has the same tests but is slightly cleaned up.

Comment 10

[:Gavin Sharp [email: [e-mail]]]

At this point I think this and bug 816202 are tracking the same thing - the hidden window never has its PB state set, and all of the worker code is tied to the hidden window.

Comment 11

[(no longer active)]

(In reply to comment #10)
> At this point I think this and bug 816202 are tracking the same thing - the
> hidden window never has its PB state set, and all of the worker code is tied to
> the hidden window.

Exactly.

Comment 12

[Shane Caraveo (:mixedpuppy)]

socialapi doesn't work in private windows, closing