1372662 - Don't copy SessionStore when creating a window with noopener

Status: RESOLVED FIXED

(Core :: Window Management, enhancement)

Description

[Nika Layzell [:nika] (ni? for response)]

We need this right now in order to implement bug 1370971 according to spec, but we would like to not need to do this (see https://github.com/whatwg/html/issues/2681).

Let's implement this behind a pref so we can decide what we want.

Comment 1

[Nika Layzell [:nika] (ni? for response)]

Attachment: Copy sessionStorage across processes when opening windows

This patch implements the feature in the simplest way I could think of, which is
using the copy of sessionStorage in sessionstore.

Ideally we'd have a better solution for this.

Doing a try run before I ask for review.

MozReview-Commit-ID: 6f3PMyZSAkd

Comment 2

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 1: Update the browser_noopener test to also test sessionstorage copying

MozReview-Commit-ID: lvLAfuZYFm

Comment 3

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 2: Add an API to desktop SessionStore to copy SessionStorage between browsers

This was the easiest way I could think of to copy SessionStorage between browsers across processes right now. Baku, I was told you might know things about the future of LocalStorage and SessionStorage, and perhaps you might have an idea of a better/easier way to do this?

Holding off on asking for review on the remaining parts of this bug until I get feedback :).

MozReview-Commit-ID: 2Hfw3SWcLAY

Comment 4

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 3: Use SessionStore to copy SessionStorage in CreateWindowInDifferentProcess

MozReview-Commit-ID: AulvjpdDU5G

Comment 5

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 4: Enable copying SessionStorage in CreateWindowInDifferentProcess by default

MozReview-Commit-ID: 53U4XvrH0E1

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8878143 [details] [diff] [review]
Part 1: Update the browser_noopener test to also test sessionstorage copying

r=me, I think.  I assume the test at least fails without these changes and passes with.  ;)

Comment 7

[Nika Layzell [:nika] (ni? for response)]

This actually doesn't work very well (I mess up the loading semantics in part 3, and that means that if we open too many windows at once you can race and fail to open some of them after my changes).

I can probably adapt this patch to make it work (I'd have to probably start the SessionStore flush on the child side before sending the request, and then read it on the parent side, shoving propagating some reference to the SessionStorage information through the Js code which triggers the initial load). It would be awesome if there was a better way to do this, or if we had plans to change SessionStorage already which would make my life easier.

Comment 8

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 1: Never copy sessionstorage into noopener windows

According to the comments in https://github.com/whatwg/html/issues/2681 it looks like this is what the spec is going toward.

Comment 9

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 2: Add wpt tests for not copying sessionstorage into noopener windows

I'm also hoping that this will cover the request for a test in the linked whatwg/html bug.

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8901967 [details] [diff] [review]
Part 1: Never copy sessionstorage into noopener windows

Looks rather simple :)

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8901968 [details] [diff] [review]
Part 2: Add wpt tests for not copying sessionstorage into noopener windows

BroadcastChannel is so nice.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/eeb9b3c58699
Part 1: Never copy sessionstorage into noopener windows, r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/2af34cba4d4c
Part 2: Add wpt tests for not copying sessionstorage into noopener windows, r=smaug

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/94d6cf31353d
Part 3: Include manifest changes for WPT on a CLOSED TREE, a=bustage

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/eeb9b3c58699
https://hg.mozilla.org/mozilla-central/rev/2af34cba4d4c
https://hg.mozilla.org/mozilla-central/rev/94d6cf31353d