Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove the storage mutex #342

Merged
merged 2 commits into from
Dec 15, 2015
Merged

Remove the storage mutex #342

merged 2 commits into from
Dec 15, 2015

Conversation

foolip
Copy link
Member

@foolip foolip commented Nov 16, 2015

No description provided.

@@ -96713,18 +96552,6 @@ dictionary <dfn>StorageEventInit</dfn> : <span>EventInit</span> {
initialised to null. It represents the <code>Storage</code> object that was affected.</p>



<h4>Threads</h4>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we keep this section and point out that there can be racing now that the protection against that is gone? (If we end up specifying something later this can be amended, but it seems good to articulate somewhere the guarantees the specification text offers.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a stern warning for the racy document.cookies API, and thought I would do something similar for localStorage once I figure out how to express the "sync at event loop" thing. Do you think we also need to keep this section, or the "Serialisability of script execution" section?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either is probably fine. I suspect that long term we should rewrite the storage APIs on top of some primitives that define the synchronization.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that adding a stern warning + example similar to the coookies one (or maybe just a reference to the cookies example) would be good.

@domenic domenic added the removal/deprecation Removing or deprecating a feature label Nov 25, 2015
be thought of as completely serialising the execution of all scripts in all <span data-x="browsing
context">browsing contexts</span>.</p>

<p class="note">The <code
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this section is still valuable. You can just remove the note. Right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@foolip foolip force-pushed the rm-storage-mutex branch 3 times, most recently from 6c901e5 to bb89fa1 Compare December 15, 2015 14:31
@domenic
Copy link
Member

domenic commented Dec 15, 2015

This LGTM after a rebase.

Also rename NavigatorStorageUtils to NavigatorCookies.

Fixes #334
Add stern warnings for both document.cookies and localStorage.

Fixes #335
@foolip
Copy link
Member Author

foolip commented Dec 15, 2015

Rebased to resolve conflicts with eec9646 (the removed algorithm released the storage mutex)

@domenic domenic merged commit 1b918cf into master Dec 15, 2015
@foolip foolip deleted the rm-storage-mutex branch December 15, 2015 19:03
annevk pushed a commit that referenced this pull request Dec 16, 2015
@domenic domenic mentioned this pull request Jun 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
removal/deprecation Removing or deprecating a feature
Development

Successfully merging this pull request may close these issues.

3 participants