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

net: Don't load the placeholder image for background images, only for image fragments. #5586

Merged
merged 1 commit into from May 20, 2015
Merged

net: Don't load the placeholder image for background images, only for image fragments. #5586

merged 1 commit into from May 20, 2015

Conversation

@pcwalton
Copy link
Contributor

pcwalton commented Apr 7, 2015

r? @jdm

Review on Reviewable
@hoppipolla-critic-bot
Copy link

hoppipolla-critic-bot commented Apr 7, 2015

Critic review: https://critic.hoppipolla.co.uk/r/4578

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.
@bors-servo
Copy link
Contributor

bors-servo commented Apr 8, 2015

☔ The latest upstream changes (presumably #5574) made this pull request unmergeable. Please resolve the merge conflicts.
@jdm jdm added S-needs-rebase and removed S-awaiting-review labels Apr 8, 2015
@Ms2ger Ms2ger mentioned this pull request Apr 8, 2015
Closed
@jdm jdm added the S-needs-squash label Apr 10, 2015
@pcwalton pcwalton force-pushed the pcwalton:no-broken-background-image-redux branch from ddce50b to b76ae2a Apr 10, 2015
@pcwalton
Copy link
Contributor Author

pcwalton commented Apr 10, 2015

@bors-servo: r=jdm
@bors-servo
Copy link
Contributor

bors-servo commented Apr 10, 2015

📌 Commit b76ae2a has been approved by jdm
@bors-servo
Copy link
Contributor

bors-servo commented Apr 10, 2015

⌛ Testing commit b76ae2a with merge fb8051a...
bors-servo pushed a commit that referenced this pull request Apr 10, 2015
bors-servo
Auto merge of #5586 - pcwalton:no-broken-background-image-redux, r=jdm
fb8051a 
r? @jdm
@bors-servo
Copy link
Contributor

bors-servo commented Apr 10, 2015

💔 Test failed - linux1
@jdm
Copy link
Member

jdm commented Apr 10, 2015 

failures:
    no-image.html == no-image-ref.html
@pcwalton
Copy link
Contributor Author

pcwalton commented Apr 22, 2015

I think I'll wait on #5767 to land first, because the test failure is a Linux-only race that's tightly coupled to the image cache task.
@bors-servo
Copy link
Contributor

bors-servo commented Apr 23, 2015

☔ The latest upstream changes (presumably #5767) made this pull request unmergeable. Please resolve the merge conflicts.
@pcwalton pcwalton force-pushed the pcwalton:no-broken-background-image-redux branch from b76ae2a to e3703c9 Apr 27, 2015
@pcwalton pcwalton force-pushed the pcwalton:no-broken-background-image-redux branch from e3703c9 to 72c3866 Apr 27, 2015
components/net_traits/image/holder.rs Outdated
@@ -0,0 +1,104 @@
/* This Source Code Form is subject to the terms of the Mozilla Public

This comment has been minimized.

Sign in to view
@glennw

glennw Apr 27, 2015

Member

This whole file can be removed.
components/net_traits/local_image_cache.rs Outdated
@@ -0,0 +1,174 @@
/* This Source Code Form is subject to the terms of the Mozilla Public

This comment has been minimized.

Sign in to view
@glennw

glennw Apr 27, 2015

Member

This whole file can be removed.
@pcwalton pcwalton force-pushed the pcwalton:no-broken-background-image-redux branch from 72c3866 to 7fb7b49 Apr 27, 2015
@pcwalton
Copy link
Contributor Author

pcwalton commented Apr 27, 2015

Review comments addressed.

@bors-servo: r=glennw
@jdm jdm removed the S-awaiting-merge label Apr 28, 2015
@bors-servo
Copy link
Contributor

bors-servo commented May 1, 2015

☔ The latest upstream changes (presumably #5913) made this pull request unmergeable. Please resolve the merge conflicts.
@pcwalton pcwalton force-pushed the pcwalton:no-broken-background-image-redux branch from 7fb7b49 to 4be567e May 1, 2015
@pcwalton
Copy link
Contributor Author

pcwalton commented May 1, 2015

@bors-servo: r=glennw
@bors-servo
Copy link
Contributor

bors-servo commented May 1, 2015

📌 Commit 4be567e has been approved by glennw
@bors-servo
Copy link
Contributor

bors-servo commented May 1, 2015

⌛ Testing commit 4be567e with merge dfabbe6...
bors-servo pushed a commit that referenced this pull request May 1, 2015
bors-servo
Auto merge of #5586 - pcwalton:no-broken-background-image-redux, r=gl…
dfabbe6 
…ennw

r? @jdm

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/5586)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented May 1, 2015

💔 Test failed - mac2
@jdm
Copy link
Member

jdm commented May 1, 2015 

failures:
    no_image_background_a.html == no_image_background_ref.html
@jdm jdm added S-tests-failed and removed S-awaiting-merge labels May 1, 2015
@bors-servo
Copy link
Contributor

bors-servo commented May 5, 2015

☔ The latest upstream changes (presumably #5935) made this pull request unmergeable. Please resolve the merge conflicts.
@pcwalton
net: Don't load the placeholder image for background images, only for
Loading status checks…
7e7675c 
image fragments.

This also changes the way the placeholder is handled in the image cache
task to decode it up front instead of each time an image fails to load,
both because it was more convenient to implement that way and because
it saves CPU cycles to do so.

This matches the behavior of Gecko and WebKit. It improves the look of
our cached copy of Wikipedia.
@pcwalton pcwalton force-pushed the pcwalton:no-broken-background-image-redux branch from 4be567e to 7e7675c May 20, 2015
@pcwalton
Copy link
Contributor Author

pcwalton commented May 20, 2015

@bors-servo: r=glennw

Apologies for trying again, but I've run the tests multiple times on both Mac and Linux and have been unable to reproduce. I want to rule out the possibility that this was a manifestation of the flakiness that @glennw fixed. If this fails again I will SSH into the bots to try to reproduce.
@bors-servo
Copy link
Contributor

bors-servo commented May 20, 2015

📌 Commit 7e7675c has been approved by glennw
@bors-servo
Copy link
Contributor

bors-servo commented May 20, 2015

⌛ Testing commit 7e7675c with merge 77099b2...
bors-servo pushed a commit that referenced this pull request May 20, 2015
bors-servo
Auto merge of #5586 - pcwalton:no-broken-background-image-redux, r=gl…
Loading status checks…
77099b2 
…ennw

r? @jdm

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/5586)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented May 20, 2015

☀️ Test successful - android, gonk, linux1, linux2, mac1, mac2
@bors-servo bors-servo mentioned this pull request May 20, 2015
Closed
@bors-servo bors-servo merged commit 7e7675c into servo:master May 20, 2015
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@pcwalton pcwalton deleted the pcwalton:no-broken-background-image-redux branch May 20, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@jdm jdm

Labels
S-awaiting-merge
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
@pcwalton @hoppipolla-critic-bot @bors-servo @jdm @glennw @highfive
You can’t perform that action at this time.