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

Move HTTP requests out of image cache logic #7708

Closed
jdm opened this issue Sep 22, 2015 · 6 comments
Closed

Move HTTP requests out of image cache logic #7708

jdm opened this issue Sep 22, 2015 · 6 comments

Comments

@jdm
Copy link
Member

@jdm jdm commented Sep 22, 2015

The cache should be a simple store for decoded pixels with a key that can distinguish between two image requests for the same URL that included different sets of cookies, for example. The (eventual) network cache should be in charge of caching the original response bytes to facilitate re-decoding if necessary.

This work will also enable much easier implementation of image elements blocking the document's load event.

@jdm jdm changed the title Move HTTP requests out of image cache Move HTTP requests out of image cache logic Sep 22, 2015
@jdm
Copy link
Member Author

@jdm jdm commented May 30, 2016

Current effort exists in #11453.

@KiChjang
Copy link
Member

@KiChjang KiChjang commented Oct 1, 2016

I'm looking for something to kill some time, and this looks like a good thing to do so

@KiChjang KiChjang self-assigned this Oct 1, 2016
@jdm
Copy link
Member Author

@jdm jdm commented Oct 1, 2016

We should chat about the problems I ran up against.

@KiChjang
Copy link
Member

@KiChjang KiChjang commented Oct 1, 2016

There's at least a rebased version of #11453 located here.

@jdm
Copy link
Member Author

@jdm jdm commented Oct 2, 2016

So as not to leave you hanging, these were the Hard Problems that I realized:

  • https://groups.google.com/d/msg/mozilla.dev.servo/kxgpmsuDw_s/_BtBVW7bPAAJ
  • for images that are loaded based on CSS (eg. background-image), it is much more difficult (and more code-duplication-y) to respect things like the document's referrer policy since we're in the layout thread. It would be so nice if we could somehow make the image load originate from the script thread instead, but I don't have a clear idea of how to make this happen (handwave: somehow return a list of image URLs and originating loads from layout to script, make script deal with them)

I think those are the only two that I got stuck on.

@KiChjang
Copy link
Member

@KiChjang KiChjang commented Oct 13, 2016

Both problems sound like they can be solved with futures-rs, though that crate only provides a trait for Futures, and we'll need to work on the implementation on how exactly futures get resolved by the script thread.

@KiChjang KiChjang assigned jdm and unassigned KiChjang Dec 11, 2016
@jdm jdm mentioned this issue Jan 11, 2017
4 of 4 tasks complete
bors-servo added a commit that referenced this issue Jan 11, 2017
Remove network requests from image cache thread

The design of initiating network requests from the image cache thread was simple, but it makes it difficult to implement image loading that conforms to the HTML specification. These changes make the implementation of HTMLImageElement responsible for network requests for `<img>` elements, and CSS-based images (background-image, bullets, etc.) are requested by the script thread to ensure that the layout thread does not attempt to retain unsafe pointers to DOM nodes during asynchronous operations.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #7708
- [X] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14962)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Feb 16, 2017
Remove network requests from image cache thread

The design of initiating network requests from the image cache thread was simple, but it makes it difficult to implement image loading that conforms to the HTML specification. These changes make the implementation of HTMLImageElement responsible for network requests for `<img>` elements, and CSS-based images (background-image, bullets, etc.) are requested by the script thread to ensure that the layout thread does not attempt to retain unsafe pointers to DOM nodes during asynchronous operations.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #7708
- [X] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14962)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Feb 17, 2017
Remove network requests from image cache thread

The design of initiating network requests from the image cache thread was simple, but it makes it difficult to implement image loading that conforms to the HTML specification. These changes make the implementation of HTMLImageElement responsible for network requests for `<img>` elements, and CSS-based images (background-image, bullets, etc.) are requested by the script thread to ensure that the layout thread does not attempt to retain unsafe pointers to DOM nodes during asynchronous operations.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #7708
- [X] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14962)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Feb 18, 2017
Remove network requests from image cache thread

The design of initiating network requests from the image cache thread was simple, but it makes it difficult to implement image loading that conforms to the HTML specification. These changes make the implementation of HTMLImageElement responsible for network requests for `<img>` elements, and CSS-based images (background-image, bullets, etc.) are requested by the script thread to ensure that the layout thread does not attempt to retain unsafe pointers to DOM nodes during asynchronous operations.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #7708
- [X] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14962)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Feb 18, 2017
Remove network requests from image cache thread

The design of initiating network requests from the image cache thread was simple, but it makes it difficult to implement image loading that conforms to the HTML specification. These changes make the implementation of HTMLImageElement responsible for network requests for `<img>` elements, and CSS-based images (background-image, bullets, etc.) are requested by the script thread to ensure that the layout thread does not attempt to retain unsafe pointers to DOM nodes during asynchronous operations.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #7708
- [X] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14962)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Feb 21, 2017
Remove network requests from image cache thread

The design of initiating network requests from the image cache thread was simple, but it makes it difficult to implement image loading that conforms to the HTML specification. These changes make the implementation of HTMLImageElement responsible for network requests for `<img>` elements, and CSS-based images (background-image, bullets, etc.) are requested by the script thread to ensure that the layout thread does not attempt to retain unsafe pointers to DOM nodes during asynchronous operations.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #7708
- [X] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14962)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Feb 22, 2017
Remove network requests from image cache thread

The design of initiating network requests from the image cache thread was simple, but it makes it difficult to implement image loading that conforms to the HTML specification. These changes make the implementation of HTMLImageElement responsible for network requests for `<img>` elements, and CSS-based images (background-image, bullets, etc.) are requested by the script thread to ensure that the layout thread does not attempt to retain unsafe pointers to DOM nodes during asynchronous operations.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #7708
- [X] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14962)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Feb 23, 2017
Remove network requests from image cache thread

The design of initiating network requests from the image cache thread was simple, but it makes it difficult to implement image loading that conforms to the HTML specification. These changes make the implementation of HTMLImageElement responsible for network requests for `<img>` elements, and CSS-based images (background-image, bullets, etc.) are requested by the script thread to ensure that the layout thread does not attempt to retain unsafe pointers to DOM nodes during asynchronous operations.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #7708
- [X] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14962)
<!-- Reviewable:end -->
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.

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