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 navigator.yieldForStorageUpdates() due to lack of implementation #341

Closed
wants to merge 1 commit into from

Conversation

foolip
Copy link
Member

@foolip foolip commented Nov 14, 2015

Also rename NavigatorStorageUtils to NavigatorCookies.

Fixes #334

@annevk
Copy link
Member

annevk commented Nov 16, 2015

Would it perhaps be better to couple this with the removal of the storage mutex? Otherwise we might forget to add advice/warnings back.

@foolip
Copy link
Member Author

foolip commented Nov 16, 2015

What do you mean should be added back? AFAICT, the only overlap is in the "Serialisability of script execution" section, which should be fully removed or perhaps replaced with warnings, as noted in #335 (comment)

@annevk
Copy link
Member

annevk commented Nov 16, 2015

The "A call to X is implied" phrases seem like they might return in some way. E.g., if we adopt the model from @inexorabletash in that issue thread.

@foolip
Copy link
Member Author

foolip commented Nov 16, 2015

OK, so "A call to the navigator.yieldForStorageUpdates() method is implied when this method is invoked" corresponds to the "Release the storage mutex" step. It would be nice to remove those two things together, I suppose.

Also rename NavigatorStorageUtils to NavigatorCookies.

Fixes #334
@foolip
Copy link
Member Author

foolip commented Nov 16, 2015

Not wanting to spend time untangling these issues, I'm doing both in #342

@foolip foolip closed this Nov 16, 2015
@foolip foolip deleted the rm-yieldForStorageUpdates branch November 16, 2015 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants