Closed Bug 1372662 Opened 3 years ago Closed 3 years ago
Don't copy Session
Store when creating a window with noopener
1.38 KB, patch
|Details | Diff | Splinter Review|
2.60 KB, patch
|Details | Diff | Splinter Review|
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.
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
Attachment #8878143 - Flags: review?(bzbarsky)
Attachment #8877751 - Attachment is obsolete: true
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 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. ;)
Attachment #8878143 - Flags: review?(bzbarsky) → review+
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.
According to the comments in https://github.com/whatwg/html/issues/2681 it looks like this is what the spec is going toward.
Attachment #8901967 - Flags: review?(bugs)
Attachment #8878143 - Attachment is obsolete: true
I'm also hoping that this will cover the request for a test in the linked whatwg/html bug.
Summary: Copy SessionStorage into windows created with CreateWindowInOtherProcess → Don't copy SessionStore when creating a window with noopener
Comment on attachment 8901967 [details] [diff] [review] Part 1: Never copy sessionstorage into noopener windows Looks rather simple :)
Attachment #8901967 - Flags: review?(bugs) → review+
Comment on attachment 8901968 [details] [diff] [review] Part 2: Add wpt tests for not copying sessionstorage into noopener windows BroadcastChannel is so nice.
Attachment #8901968 - Flags: review?(bugs) → review+
Pushed by email@example.com: 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
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/94d6cf31353d Part 3: Include manifest changes for WPT on a CLOSED TREE, a=bustage
You need to log in before you can comment on or make changes to this bug.