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

Add file handling functionality to Blob object #10851

Closed
izgzhen opened this issue Apr 26, 2016 · 3 comments
Closed

Add file handling functionality to Blob object #10851

izgzhen opened this issue Apr 26, 2016 · 3 comments

Comments

@izgzhen
Copy link
Contributor

@izgzhen izgzhen commented Apr 26, 2016

NOTE: This is an issue assigned to myself

Spec: https://w3c.github.io/FileAPI/#blob

As noted before, current Blob object can only refer to in-memory data (DataSlice), and the FileReader will also only operate in this way. In order to support reading from file system, another internal data structure might need to be added to the Blob. It might be a file handler created at the time of object construction.

About snapshot state:

If a File object is a reference to a byte sequence originating from a file on disk, then its snapshot state should be set to the state of the file on disk at the time the File object is created.

Note: This is a non-trivial requirement to implement for user agents, and is thus not a must but a should [RFC2119]. User agents should endeavor to have a File object’s snapshot state set to the state of the underlying storage on disk at the time the reference is taken. If the file is modified on disk following the time a reference has been taken, the File's snapshot state will differ from the state of the underlying storage. User agents may use modification time stamps and other mechanisms to maintain snapshot state, but this is left as an implementation detail.

Working at branching here

bors-servo added a commit that referenced this issue Apr 27, 2016
Fixing TODOs related to file input and File API

Fixing problems I met when trying to resolve #10851, but most of changes here are simple fixes.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10873)
<!-- Reviewable:end -->
@Manishearth
Copy link
Member

@Manishearth Manishearth commented Apr 29, 2016

As mentioned in email, we need to be careful about process isolation here. The script task should never be reading files directly, nor should it be possible for the script task to ask another process to read an arbitrary file for it. Instead, the script task should ask the resource task to spawn a file dialog, and the resource task should return some handle through which the script task can refer to the file.

@izgzhen
Copy link
Contributor Author

@izgzhen izgzhen commented Apr 29, 2016

@Manishearth Thanks for guidance, I will see what I can do here

bors-servo added a commit that referenced this issue May 2, 2016
Fixes related to File API

Fixing problems I met when trying to resolve #10851, but most of changes here are simple fixes.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10873)
<!-- Reviewable:end -->
@izgzhen
Copy link
Contributor Author

@izgzhen izgzhen commented May 2, 2016

@Manishearth Can we reopen this? ...

@Manishearth Manishearth reopened this May 2, 2016
@izgzhen izgzhen mentioned this issue May 5, 2016
1 of 1 task complete
@izgzhen izgzhen mentioned this issue May 17, 2016
6 of 6 tasks complete
bors-servo added a commit that referenced this issue May 26, 2016
Add file backend support for Blob and related

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy --faster` does not report any errors
- [x] These changes fix #10851, related to #11131
- [x] These changes do not require tests because the implementation is partial and can't work alone

1. Add new backend to `Blob` and a `BlobImpl` struct to abstract multiple backends
2. Rewrite most interfaces of `Blob` to accommodate the change
3. Change the `read` behaviour of `FileReader`, considering the case when blob is file-backed and not cached

The design is still immature, welcome comments!

- [x] I used `DOMRefCell` to cache the bytes in `BlobImpl`, is it sound?
- [x] The interfaces (like `BlobImpl::get_bytes`) handle requests in a default-to-empty way when the inner `DataSlice` is not cached. It might be possible to handle this condition better.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11221)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue May 26, 2016
Add file backend support for Blob and related

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy --faster` does not report any errors
- [x] These changes fix #10851, related to #11131
- [x] These changes do not require tests because the implementation is partial and can't work alone

1. Add new backend to `Blob` and a `BlobImpl` struct to abstract multiple backends
2. Rewrite most interfaces of `Blob` to accommodate the change
3. Change the `read` behaviour of `FileReader`, considering the case when blob is file-backed and not cached

The design is still immature, welcome comments!

- [x] I used `DOMRefCell` to cache the bytes in `BlobImpl`, is it sound?
- [x] The interfaces (like `BlobImpl::get_bytes`) handle requests in a default-to-empty way when the inner `DataSlice` is not cached. It might be possible to handle this condition better.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11221)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Jun 1, 2016
Add file backend support for Blob and related

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy --faster` does not report any errors
- [x] These changes fix #10851, related to #11131
- [x] These changes do not require tests because the implementation is partial and can't work alone

1. Add new backend to `Blob` and a `BlobImpl` struct to abstract multiple backends
2. Rewrite most interfaces of `Blob` to accommodate the change
3. Change the `read` behaviour of `FileReader`, considering the case when blob is file-backed and not cached

The design is still immature, welcome comments!

- [x] I used `DOMRefCell` to cache the bytes in `BlobImpl`, is it sound?
- [x] The interfaces (like `BlobImpl::get_bytes`) handle requests in a default-to-empty way when the inner `DataSlice` is not cached. It might be possible to handle this condition better.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11221)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Jun 1, 2016
Add file backend support for Blob and related

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy --faster` does not report any errors
- [x] These changes fix #10851, related to #11131
- [x] These changes do not require tests because the implementation is partial and can't work alone

1. Add new backend to `Blob` and a `BlobImpl` struct to abstract multiple backends
2. Rewrite most interfaces of `Blob` to accommodate the change
3. Change the `read` behaviour of `FileReader`, considering the case when blob is file-backed and not cached

The design is still immature, welcome comments!

- [x] I used `DOMRefCell` to cache the bytes in `BlobImpl`, is it sound?
- [x] The interfaces (like `BlobImpl::get_bytes`) handle requests in a default-to-empty way when the inner `DataSlice` is not cached. It might be possible to handle this condition better.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11221)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Jun 1, 2016
Add file backend support for Blob and related

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy --faster` does not report any errors
- [x] These changes fix #10851, related to #11131
- [x] These changes do not require tests because the implementation is partial and can't work alone

1. Add new backend to `Blob` and a `BlobImpl` struct to abstract multiple backends
2. Rewrite most interfaces of `Blob` to accommodate the change
3. Change the `read` behaviour of `FileReader`, considering the case when blob is file-backed and not cached

The design is still immature, welcome comments!

- [x] I used `DOMRefCell` to cache the bytes in `BlobImpl`, is it sound?
- [x] The interfaces (like `BlobImpl::get_bytes`) handle requests in a default-to-empty way when the inner `DataSlice` is not cached. It might be possible to handle this condition better.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11221)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Jun 1, 2016
Add file backend support for Blob and related

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy --faster` does not report any errors
- [x] These changes fix #10851, related to #11131
- [x] These changes do not require tests because the implementation is partial and can't work alone

1. Add new backend to `Blob` and a `BlobImpl` struct to abstract multiple backends
2. Rewrite most interfaces of `Blob` to accommodate the change
3. Change the `read` behaviour of `FileReader`, considering the case when blob is file-backed and not cached

The design is still immature, welcome comments!

- [x] I used `DOMRefCell` to cache the bytes in `BlobImpl`, is it sound?
- [x] The interfaces (like `BlobImpl::get_bytes`) handle requests in a default-to-empty way when the inner `DataSlice` is not cached. It might be possible to handle this condition better.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11221)
<!-- Reviewable:end -->
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 1, 2019
…r=Manishearth

Fixing problems I met when trying to resolve servo/servo#10851, but most of changes here are simple fixes.

Source-Repo: https://github.com/servo/servo
Source-Revision: 14926529c9f38a014140d75ca06cc46d1a2b72ba

UltraBlame original commit: 47b37dd10751427a6b693cf5848fe8c0b56a6ad9
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 1, 2019
…r=Manishearth

Fixing problems I met when trying to resolve servo/servo#10851, but most of changes here are simple fixes.

Source-Repo: https://github.com/servo/servo
Source-Revision: 14926529c9f38a014140d75ca06cc46d1a2b72ba

UltraBlame original commit: 47b37dd10751427a6b693cf5848fe8c0b56a6ad9
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 1, 2019
…r=Manishearth

Fixing problems I met when trying to resolve servo/servo#10851, but most of changes here are simple fixes.

Source-Repo: https://github.com/servo/servo
Source-Revision: 14926529c9f38a014140d75ca06cc46d1a2b72ba

UltraBlame original commit: 47b37dd10751427a6b693cf5848fe8c0b56a6ad9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants
You can’t perform that action at this time.