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

Track event state changes on Document and do the dirtying from layout #8098

Merged
merged 4 commits into from Oct 28, 2015
Merged

Track event state changes on Document and do the dirtying from layout #8098

merged 4 commits into from Oct 28, 2015

Conversation

@bholley
Copy link
Contributor

bholley commented Oct 20, 2015

This is a first step in fixing #6942.

Review on Reviewable
@highfive
Copy link

highfive commented Oct 20, 2015

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
  • These commits modify layout code, but no reftests are modified. Please consider adding a reftest!
@bholley
Copy link
Contributor Author

bholley commented Oct 20, 2015

r? @pcwalton
@bholley
Copy link
Contributor Author

bholley commented Oct 20, 2015

@frewsxcv can you kick off a try push for me?
@bholley
Copy link
Contributor Author

bholley commented Oct 20, 2015

oh wait, need to fix some tidness. Moment.
@bholley bholley force-pushed the bholley:dirty_from_layout branch from cfd0441 to 3e5d83d Oct 20, 2015
@jdm
Copy link
Member

jdm commented Oct 20, 2015

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

bors-servo commented Oct 20, 2015

⌛ Trying commit 3e5d83d with merge 4fd0ef4...
bors-servo pushed a commit that referenced this pull request Oct 20, 2015
bors-servo
Auto merge of #8098 - bholley:dirty_from_layout, r=<try>
4fd0ef4 
Track event state changes on Document and do the dirtying from layout

This is a first step in fixing #6942.

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

bors-servo commented Oct 20, 2015

💔 Test failed - linux-dev
@jdm
Copy link
Member

jdm commented Oct 20, 2015 

failures:
    iframe/bg_color.html == iframe/bg_color_ref.html
    iframe/hide_after_load.html == iframe/hide_after_load_ref.html
    iframe/hide_layers1.html == iframe/hide_layers_ref.html
    iframe/simple_inline_height.html == iframe/simple_inline_height_ref.html
@bholley bholley force-pushed the bholley:dirty_from_layout branch from 3e5d83d to 0b3e0e4 Oct 20, 2015
@bholley
Copy link
Contributor Author

bholley commented Oct 20, 2015

Another try push plz. :-)
@bholley
Copy link
Contributor Author

bholley commented Oct 20, 2015

/me facepalms about tidiness again.
@bholley bholley force-pushed the bholley:dirty_from_layout branch from 0b3e0e4 to 17c3be0 Oct 20, 2015
@jdm
Copy link
Member

jdm commented Oct 20, 2015

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

bors-servo commented Oct 20, 2015

⌛ Trying commit 17c3be0 with merge 48c3c16...
bors-servo pushed a commit that referenced this pull request Oct 20, 2015
bors-servo
Auto merge of #8098 - bholley:dirty_from_layout, r=<try>
48c3c16 
Track event state changes on Document and do the dirtying from layout

This is a first step in fixing #6942.

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

bors-servo commented Oct 20, 2015

💔 Test failed - mac-rel-wpt
@frewsxcv
Copy link
Member

frewsxcv commented Oct 20, 2015

@bors-servo: try

That last fail was intermittent
@jdm
Copy link
Member

jdm commented Oct 20, 2015

I don't think there's value in re-trying try pushes that have known intermittent failures.
@bors-servo
Copy link
Contributor

bors-servo commented Oct 21, 2015

☔ The latest upstream changes (presumably #8041) made this pull request unmergeable. Please resolve the merge conflicts.
@bholley bholley mentioned this pull request Oct 24, 2015
Merged
@bholley bholley force-pushed the bholley:dirty_from_layout branch from 7d8febb to 21bad89 Oct 24, 2015
@bholley bholley removed the S-needs-rebase label Oct 24, 2015
@bors-servo
Copy link
Contributor

bors-servo commented Oct 25, 2015

☔ The latest upstream changes (presumably #8181) made this pull request unmergeable. Please resolve the merge conflicts.
bholley added 4 commits Oct 10, 2015
@bholley
Implement Hash and Eq for JS<T> and LayoutJS<T>.
630b9f8
@bholley
Use an RAII guard to join the script task.
85596b5
@bholley
Pass the document instead of the documentElement to reflow.
441c84d
@bholley
Check modified event state from layout and dirty it there.
Loading status checks…
069c40f 
This adds some overhead, but also provides the small performance benefit of
avoiding dirtying in the case where an event state is toggled an even
number of times between reflows.

The main benefit here though is that it sets us up to be smarter about
what we mark as dirty using restyle hints.
@bholley bholley force-pushed the bholley:dirty_from_layout branch from 21bad89 to 069c40f Oct 27, 2015
@pcwalton
Copy link
Contributor

pcwalton commented Oct 27, 2015

Looks good to me.
@pcwalton
Copy link
Contributor

pcwalton commented Oct 27, 2015

(at least layout. r? @jdm for script)
@highfive highfive assigned jdm and unassigned pcwalton Oct 27, 2015
@Manishearth
Copy link
Member

Manishearth commented Oct 28, 2015

Reviewed 1 of 1 files at r1, 1 of 1 files at r2.
Review status: 1 of 8 files reviewed at latest revision, all discussions resolved, all commit checks successful.

Comments from the review on Reviewable.io
@jdm
Copy link
Member

jdm commented Oct 28, 2015

@bors-servo: r+

Reviewed 1 of 1 files at r1, 1 of 1 files at r2, 5 of 5 files at r3, 6 of 6 files at r4.
Review status: :shipit: all files reviewed at latest revision, all discussions resolved, all commit checks successful.

Comments from the review on Reviewable.io
@bors-servo
Copy link
Contributor

bors-servo commented Oct 28, 2015

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

bors-servo commented Oct 28, 2015

⌛ Testing commit 069c40f with merge 285e29c...
bors-servo added a commit that referenced this pull request Oct 28, 2015
@bors-servo
Auto merge of #8098 - bholley:dirty_from_layout, r=jdm
Loading status checks…
285e29c 
Track event state changes on Document and do the dirtying from layout

This is a first step in fixing #6942.

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

bors-servo commented Oct 28, 2015

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css, mac-rel-wpt
@bors-servo bors-servo merged commit 069c40f into servo:master Oct 28, 2015
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
This was referenced Oct 28, 2015
Merged
Merged
@bholley bholley deleted the bholley:dirty_from_layout branch Oct 30, 2016
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

8 participants
@bholley @highfive @jdm @bors-servo @frewsxcv @pcwalton @Manishearth @nox
You can’t perform that action at this time.