Skip to content

/ servo

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 Blob URL store #11534

Merged
merged 1 commit into from Jun 3, 2016
Merged

Add Blob URL store #11534

merged 1 commit into from Jun 3, 2016

Conversation

@izgzhen
Copy link
Contributor

izgzhen commented Jun 1, 2016 •
edited by larsbergstrom

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

I finally decide to put the store under ScriptThread and interpret the "global object" as the script thread itself. The new APIs will be used during the page loading (if scheme is blob) and URL.createObjectURL/revokeObjectURL.

Related to #11131.

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix part of #10539
  • These changes do not require tests because it is new stub code which needs further integrating PRs.

This change is Reviewable
@highfive
Copy link

highfive commented Jun 1, 2016

Heads up! This PR modifies the following files:

  • @KiChjang: components/script/blob_url_store.rs, components/script/lib.rs, components/script/script_thread.rs
@highfive
Copy link

highfive commented Jun 1, 2016

warning Warning warning

  • These commits modify script code, but no tests are modified. Please consider adding a test!
@izgzhen
Copy link
Contributor Author

izgzhen commented Jun 1, 2016

cc @Manishearth
@izgzhen izgzhen mentioned this pull request Jun 1, 2016
Merged
4 of 4 tasks complete
@Ms2ger
Copy link
Contributor

Ms2ger commented Jun 1, 2016

That's an incorrect interpretation of "global object". Say you have a page with an iframe; a blob url created in the outer window should not be resolvable in the iframe.
@izgzhen
Copy link
Contributor Author

izgzhen commented Jun 1, 2016

@Ms2ger So I think the Window/Worker-wide global might work?
@Ms2ger
Copy link
Contributor

Ms2ger commented Jun 1, 2016

Yep, Window or WorkerGlobalScope
@izgzhen izgzhen force-pushed the izgzhen:add-blob-url-store branch from 6c858ad to 7ccae9e Jun 1, 2016
@highfive
Copy link

highfive commented Jun 1, 2016

New code was committed to pull request.
@SimonSapin SimonSapin assigned Ms2ger and unassigned SimonSapin Jun 1, 2016
@bors-servo
Copy link
Contributor

bors-servo commented Jun 1, 2016

☔ The latest upstream changes (presumably #11538) made this pull request unmergeable. Please resolve the merge conflicts.
@izgzhen
add Blob URL store
Loading status checks…
3d7ed42
@izgzhen izgzhen force-pushed the izgzhen:add-blob-url-store branch from 7ccae9e to 3d7ed42 Jun 3, 2016
@Ms2ger
Copy link
Contributor

Ms2ger commented Jun 3, 2016

r? @Manishearth
@highfive highfive assigned Manishearth and unassigned Ms2ger Jun 3, 2016
@Manishearth
Copy link
Member

Manishearth commented Jun 3, 2016

@bors-servo r+

Note that the resource thread will also need to know the blob uuids
@bors-servo
Copy link
Contributor

bors-servo commented Jun 3, 2016

📌 Commit 3d7ed42 has been approved by Manishearth
@bors-servo
Copy link
Contributor

bors-servo commented Jun 3, 2016

⌛ Testing commit 3d7ed42 with merge b003218...
bors-servo added a commit that referenced this pull request Jun 3, 2016
@bors-servo
Auto merge of #11534 - izgzhen:add-blob-url-store, r=Manishearth
b003218 
Add Blob URL store

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

I finally decide to put the store under `ScriptThread` and interpret the "global object" as the script thread itself. The new APIs will be used during the page loading (if scheme is `blob`) and `URL.createObjectURL/revokeObjectURL`.

Related to #11131.

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix part of #10539

<!-- Either: -->
- [x] These changes do not require tests because it is new stub code which needs further integrating PRs.

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/11534)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jun 3, 2016

💔 Test failed - linux-rel
@bors-servo
Copy link
Contributor

bors-servo commented Jun 3, 2016

💔 Test failed - linux-rel
@highfive
Copy link

highfive commented Jun 3, 2016 

  ▶ FAIL [expected PASS] /css-transforms-1_dev/html/transform-abspos-007.htm
  └   → /css-transforms-1_dev/html/transform-abspos-007.htm 71f0313eedfbfcce0b0fcc5ae55f34b1daa3b8d8
/css-transforms-1_dev/html/reference/transform-abspos-ref.htm 78d197606924062e8dd2a773c977afcecf8940f8
Testing 71f0313eedfbfcce0b0fcc5ae55f34b1daa3b8d8 == 78d197606924062e8dd2a773c977afcecf8940f8
@izgzhen izgzhen mentioned this pull request Jun 3, 2016
Closed
@izgzhen
Copy link
Contributor Author

izgzhen commented Jun 3, 2016 •
edited

@bors-servo retry
@bors-servo
Copy link
Contributor

bors-servo commented Jun 3, 2016

⌛ Testing commit 3d7ed42 with merge 410de2b...
bors-servo added a commit that referenced this pull request Jun 3, 2016
@bors-servo
Auto merge of #11534 - izgzhen:add-blob-url-store, r=Manishearth
410de2b 
Add Blob URL store

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

I finally decide to put the store under `ScriptThread` and interpret the "global object" as the script thread itself. The new APIs will be used during the page loading (if scheme is `blob`) and `URL.createObjectURL/revokeObjectURL`.

Related to #11131.

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix part of #10539

<!-- Either: -->
- [x] These changes do not require tests because it is new stub code which needs further integrating PRs.

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/11534)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jun 3, 2016

💔 Test failed - linux-rel
@highfive
Copy link

highfive commented Jun 3, 2016 

  ▶ FAIL [expected PASS] /css-transforms-1_dev/html/transform-table-007.htm
  └   → /css-transforms-1_dev/html/transform-table-007.htm a5c014b20ef1363bea6f24eda28c7efb7c45698a
/css-transforms-1_dev/html/reference/transform-blank-ref.htm fa6407b1acbbfea27e27061e7d1bdeca98e4a728
Testing a5c014b20ef1363bea6f24eda28c7efb7c45698a == fa6407b1acbbfea27e27061e7d1bdeca98e4a728
@izgzhen
Copy link
Contributor Author

izgzhen commented Jun 3, 2016

Jesus ... I will leave it cool for several hours
@jdm
Copy link
Member

jdm commented Jun 3, 2016

@bors-servo: retry
@bors-servo
Copy link
Contributor

bors-servo commented Jun 3, 2016

⌛ Testing commit 3d7ed42 with merge b389ecd...
bors-servo added a commit that referenced this pull request Jun 3, 2016
@bors-servo
Auto merge of #11534 - izgzhen:add-blob-url-store, r=Manishearth
Loading status checks…
b389ecd 
Add Blob URL store

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

I finally decide to put the store under `ScriptThread` and interpret the "global object" as the script thread itself. The new APIs will be used during the page loading (if scheme is `blob`) and `URL.createObjectURL/revokeObjectURL`.

Related to #11131.

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix part of #10539

<!-- Either: -->
- [x] These changes do not require tests because it is new stub code which needs further integrating PRs.

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/11534)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jun 3, 2016

💔 Test failed - linux-rel
@highfive
Copy link

highfive commented Jun 3, 2016 

  ▶ FAIL [expected PASS] /css-transforms-1_dev/html/transform-abspos-002.htm
  └   → /css-transforms-1_dev/html/transform-abspos-002.htm 71f0313eedfbfcce0b0fcc5ae55f34b1daa3b8d8
/css-transforms-1_dev/html/reference/transform-abspos-ref.htm 78d197606924062e8dd2a773c977afcecf8940f8
Testing 71f0313eedfbfcce0b0fcc5ae55f34b1daa3b8d8 == 78d197606924062e8dd2a773c977afcecf8940f8
@jdm
Copy link
Member

jdm commented Jun 3, 2016

@bors-servo: retry
@bors-servo
Copy link
Contributor

bors-servo commented Jun 3, 2016

⚡ Previous build results for android, arm32, arm64, linux-dev, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows are reusable. Rebuilding only linux-rel...
@bors-servo
Copy link
Contributor

bors-servo commented Jun 3, 2016

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows
@bors-servo bors-servo merged commit 3d7ed42 into servo:master Jun 3, 2016
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@izgzhen
Copy link
Contributor Author

izgzhen commented Jun 3, 2016

cool, thanks
bors-servo added a commit that referenced this pull request Jun 8, 2016
@bors-servo
Auto merge of #11536 - izgzhen:add-blob-loader, r=Manishearth
Loading status checks…
3977128 
Add blob loader

Add a blob loader to implement [Blob URL](https://w3c.github.io/FileAPI/#url). The related interfaces to script thread are also declared.

Progressing in parallel with PR #11534. Related to #11131.

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix part of #10539

<!-- Either: -->
- [x] These changes do not require tests because not integrated yet.

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/11536)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Manishearth Manishearth

Labels
S-tests-failed
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
@izgzhen @highfive @Ms2ger @bors-servo @Manishearth @jdm @SimonSapin
You can’t perform that action at this time.