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

Ignore the HTML parser's borrow flag in GC tracing #3716

Closed
wants to merge 1 commit into from

Conversation

@kmcallister
Copy link
Contributor

kmcallister commented Oct 17, 2014

r? @mbrubeck (who hit the bug), @pcwalton (who advised me on the safety of possible solutions)

@hoppipolla-critic-bot
Copy link

hoppipolla-critic-bot commented Oct 17, 2014

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

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.

@kmcallister

This comment has been minimized.

Copy link
Owner Author

kmcallister commented on 0debc87 Oct 21, 2014

r=mbrubeck

This comment has been minimized.

Copy link

mbrubeck replied Oct 21, 2014

@bors: retry

This comment has been minimized.

Copy link

pcwalton replied Oct 21, 2014

@bors: retry

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented on 0debc87 Oct 21, 2014

saw approval from mbrubeck
at kmcallister@0debc87

This comment has been minimized.

Copy link
Contributor

bors-servo replied Oct 21, 2014

merging kmcallister/servo/tokenizercell = 0debc87 into auto

This comment has been minimized.

Copy link
Contributor

bors-servo replied Oct 21, 2014

kmcallister/servo/tokenizercell = 0debc87 merged ok, testing candidate = 3eea7ed

This comment has been minimized.

Copy link
Contributor

bors-servo replied Oct 21, 2014

This comment has been minimized.

Copy link
Contributor

bors-servo replied Oct 21, 2014

saw approval from mbrubeck
at kmcallister@0debc87

This comment has been minimized.

Copy link
Contributor

bors-servo replied Oct 21, 2014

This comment has been minimized.

Copy link
Contributor

bors-servo replied Oct 21, 2014

saw approval from mbrubeck
at kmcallister@0debc87

This comment has been minimized.

Copy link
Contributor

bors-servo replied Oct 21, 2014

merging kmcallister/servo/tokenizercell = 0debc87 into auto

This comment has been minimized.

Copy link
Contributor

bors-servo replied Oct 21, 2014

kmcallister/servo/tokenizercell = 0debc87 merged ok, testing candidate = 2d1996d

This comment has been minimized.

Copy link
Contributor

bors-servo replied Oct 21, 2014

bors-servo pushed a commit that referenced this pull request Oct 21, 2014
r? @mbrubeck (who hit the bug), @pcwalton (who advised me on the safety of possible solutions)
bors-servo pushed a commit that referenced this pull request Oct 21, 2014
r? @mbrubeck (who hit the bug), @pcwalton (who advised me on the safety of possible solutions)
bors-servo pushed a commit that referenced this pull request Oct 21, 2014
r? @mbrubeck (who hit the bug), @pcwalton (who advised me on the safety of possible solutions)
@mbrubeck
Copy link
Contributor

mbrubeck commented Oct 21, 2014

/old-tests/submission/Opera/script_scheduling/039.html
------------------------------------------------------
FAIL  scheduler: IFRAMEs added with DOM
OK expected CRASH [Parent]

This looks like a genuine fix caused by this patch. You'll need to git rm tests/wpt/metadata/old-tests/submission/Opera/script_scheduling/039.html.ini.

@jdm
Copy link
Member

jdm commented Oct 21, 2014

It involves iframes, which makes me suspect an intermittent pass. Hopefully that will be corrected by #3759.

@mbrubeck
Copy link
Contributor

mbrubeck commented Oct 23, 2014

Needs a rebase now that #3759 has landed. If old-tests/submission/Opera/script_scheduling/039.html is still intermittent with this patch, we might need to skip the test for now. #3721 also seems to affect that test case, so maybe we'll be able to re-enable it once both PRs are in.

@kmcallister
Copy link
Contributor Author

kmcallister commented Oct 23, 2014

Hm, maybe I should just add the ability to break the rules to DOMRefCell, since it will be in upstream RefCell eventually (rust-lang/rust#18212)

bors-servo pushed a commit that referenced this pull request Oct 25, 2014
r? @mbrubeck, @jdm

Alternative to #3770 and #3716.
bors-servo pushed a commit that referenced this pull request Oct 29, 2014
Fixes #3356.  This makes script elements fetch their source synchronously and execute immediately by default.  It also lays some groundwork for future deferred or async script loading.

(Depends on #3716 to fix a content test failure caused by running script during parsing.)
bors-servo pushed a commit that referenced this pull request Oct 29, 2014
Fixes #3356.  This makes script elements fetch their source synchronously and execute immediately by default.  It also lays some groundwork for future deferred or async script loading.

(Depends on #3716 to fix a content test failure caused by running script during parsing.)
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

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