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

Split filesystem impl, fixes https://github.com/mozilla/thimble.webmaker.org/issues/1150 #476

Merged
merged 1 commit into from
Oct 9, 2015

Conversation

humphd
Copy link

@humphd humphd commented Oct 9, 2015

This was an attempt to move all the Blob URL caching into BracketsFiler, but it turns out to be impossible. I've made a bunch of good changes though, so I want to take the rest of it. A few of the things you'll find in it:

  • separation of Blob URL preloading into its own module (FileSystemCache)
  • removal of async versions of URL lookup
  • decoding of paths if you use BracketsFiler directly vs. via Brackets API
  • optional file consistency check on writes, so we don't hit https://github.com/mozilla/thimble.webmaker.org/issues/1150
  • I've made the rewriters to be properly async with setTimeout to yield control to the main thread more often. Since almost none of the code is async, a really big web page could block the main thread for a long time. This will do it in chunks.

@@ -142,7 +143,7 @@ define(function (require, exports, module) {

deferred.always(function() {
// Preload BlobURLs for all assets in the filesystem
BlobUtils.preload(root, function(err) {
FileSystemCache.refresh(function(err) {

Choose a reason for hiding this comment

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

This is awesome! I no longer need to figure out a root to pass in!

@gideonthomas
Copy link

This is a great patch, it will really fix a lot of issues! A few questions, r=me with that!

…sync

Refactor BlobUtils.preload() into separate module

Undoing some of my work to fix circular deps

Yield to main thread during rewriting so we don't block UI for sync work

Review fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants