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

Implement cancellable runnables, and make image load events cancellab… #8138

Merged
merged 1 commit into from Nov 6, 2015

Conversation

@jdm
Copy link
Member

jdm commented Oct 21, 2015

…le. Resolves #7731.

Review on Reviewable

@jdm jdm force-pushed the jdm:createelementintermittent branch from e4db639 to ea15377 Oct 21, 2015
@jdm
Copy link
Member Author

jdm commented Oct 22, 2015

r? @nox

@nox
Copy link
Member

nox commented Oct 22, 2015

Seems good; just one nit and question. If you could rewrite the commit message to a short title and a body it would be perfect.

-S-awaiting-review +S-needs-code-changes


Reviewed 4 of 4 files at r1.
Review status: all files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.


components/script/dom/window.rs, line 69 [r1] (raw file):
Nit: that space.


components/script/script_task.rs, line 170 [r1] (raw file):
Couldn't we avoid the nested boxes, somehow?


Comments from the review on Reviewable.io

@jdm
Copy link
Member Author

jdm commented Oct 22, 2015

Review status: 1 of 4 files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.


components/script/script_task.rs, line 170 [r1] (raw file):
I can't figure out how, since Runnable::handler requires self: Box<Self> and I can't call it without inner being boxed.


Comments from the review on Reviewable.io

@bors-servo
Copy link
Contributor

bors-servo commented Nov 3, 2015

The latest upstream changes (presumably #8056) made this pull request unmergeable. Please resolve the merge conflicts.

Additionally, make image load events cancellable. Resolves #7731.
@jdm jdm force-pushed the jdm:createelementintermittent branch from ea15377 to 989e800 Nov 6, 2015
@jdm jdm removed the S-needs-rebase label Nov 6, 2015
@jdm
Copy link
Member Author

jdm commented Nov 6, 2015

Nit fixed. Let's kill off an annoying intermittent test failure!

@Ms2ger
Copy link
Contributor

Ms2ger commented Nov 6, 2015

@bors-servo r=nox

@bors-servo
Copy link
Contributor

bors-servo commented Nov 6, 2015

📌 Commit 989e800 has been approved by nox

@bors-servo
Copy link
Contributor

bors-servo commented Nov 6, 2015

Testing commit 989e800 with merge 4113302...

bors-servo added a commit that referenced this pull request Nov 6, 2015
Implement cancellable runnables, and make image load events cancellab…

…le. Resolves #7731.

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

bors-servo commented Nov 6, 2015

@bors-servo bors-servo merged commit 989e800 into servo:master Nov 6, 2015
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

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