Remove unnecessary explicit reflows by jdm · Pull Request #23118 · servo/servo

jdm · 2019-03-27T19:04:11Z

These should be unnecessary since the introduction of the catch-all reflow in #9421. Any explicit reflow that is not performing a layout query is unnecessary, and we should be postponing reflow for as long as possible. This should allow pages to perform better when processing mouse events, for example, and allow profiles to better reflect the actual work being done by a page.

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix Document why calls to methods like force_relayout are required #5329
There are tests for these changes

This change is

highfive · 2019-03-27T19:04:16Z

Heads up! This PR modifies the following files:

@asajeffrey: components/script/dom/htmliframeelement.rs, components/script/dom/document.rs, components/script/dom/activation.rs, components/script/dom/element.rs, components/script/dom/htmlimageelement.rs and 2 more
@KiChjang: components/script/dom/htmliframeelement.rs, components/script/dom/document.rs, components/script/dom/activation.rs, components/script/dom/element.rs, components/script/dom/htmlimageelement.rs and 2 more

highfive · 2019-03-27T19:04:18Z

Warning

These commits modify script code, but no tests are modified. Please consider adding a test!

jdm · 2019-03-27T19:09:00Z

r? @nox

nox · 2019-03-28T18:50:59Z

Nice.

@bors-servo r+

bors-servo · 2019-03-28T18:51:00Z

📌 Commit d21a163 has been approved by nox

bors-servo · 2019-03-28T19:46:27Z

⌛ Testing commit d21a163 with merge 080338d...

Remove unnecessary explicit reflows These should be unnecessary since the introduction of the catch-all reflow in #9421. Any explicit reflow that is not performing a layout query is unnecessary, and we should be postponing reflow for as long as possible. This should allow pages to perform better when processing mouse events, for example, and allow profiles to better reflect the actual work being done by a page. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #5329 - [x] There are tests for these changes  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23118)

bors-servo · 2019-03-28T20:11:02Z

💔 Test failed - linux-rel-css

jdm · 2019-03-28T21:06:55Z

{
    "status": "TIMEOUT", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": null, 
    "test": "/_mozilla/css/iframe/size_attributes.html", 
    "line": 36205, 
    "action": "test_result", 
    "expected": "PASS"
}
{
    "status": "TIMEOUT", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": null, 
    "test": "/_mozilla/css/iframe/size_attributes_vertical_writing_mode.html", 
    "line": 38582, 
    "action": "test_result", 
    "expected": "PASS"
}
{
    "status": "TIMEOUT", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": null, 
    "test": "/css/compositing/mix-blend-mode/mix-blend-mode-iframe-parent.html", 
    "line": 42843, 
    "action": "test_result", 
    "expected": "PASS"
}
{
    "status": "TIMEOUT", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": null, 
    "test": "/css/compositing/mix-blend-mode/mix-blend-mode-iframe-sibling.html", 
    "line": 42909, 
    "action": "test_result", 
    "expected": "PASS"
}
{
    "status": "TIMEOUT", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": null, 
    "test": "/css/mediaqueries/min-width-tables-001.html", 
    "line": 43866, 
    "action": "test_result", 
    "expected": "PASS"
}
{
    "status": "TIMEOUT", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": null, 
    "test": "/_mozilla/css/iframe/bg_color.html", 
    "line": 45894, 
    "action": "test_result", 
    "expected": "PASS"
}
{
    "status": "TIMEOUT", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": null, 
    "test": "/_mozilla/mozilla/stylesheet-adopt-panic.html", 
    "line": 45977, 
    "action": "test_result", 
    "expected": "PASS"
}
{
    "status": "NOTRUN", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "Using matchMedia inside of a MediaQueryList callback does not panic", 
    "test": "/_mozilla/mozilla/mql_borrow.html", 
    "line": 246158, 
    "action": "test_result", 
    "expected": "PASS"
}
{
    "status": "TIMEOUT", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": null, 
    "test": "/_mozilla/mozilla/mql_borrow.html", 
    "line": 246159, 
    "action": "test_result", 
    "expected": "OK"
}

jdm · 2019-04-09T14:00:07Z

@bors-servo try=linux

jdm · 2019-04-09T14:53:41Z

@bors-servo retry

bors-servo · 2019-04-09T14:53:49Z

⌛ Trying commit d21a163 with merge 349d2f6...

Remove unnecessary explicit reflows These should be unnecessary since the introduction of the catch-all reflow in #9421. Any explicit reflow that is not performing a layout query is unnecessary, and we should be postponing reflow for as long as possible. This should allow pages to perform better when processing mouse events, for example, and allow profiles to better reflect the actual work being done by a page. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #5329 - [x] There are tests for these changes  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23118)

jdm · 2019-04-09T14:54:23Z

@bors-servo r-

jdm · 2019-04-09T14:55:00Z

@bors-servo retry

jdm · 2019-04-09T14:55:24Z

@bors-servo try- r-

jdm · 2019-04-09T14:55:44Z

@bors-servo try=linux

Remove unnecessary explicit reflows These should be unnecessary since the introduction of the catch-all reflow in #9421. Any explicit reflow that is not performing a layout query is unnecessary, and we should be postponing reflow for as long as possible. This should allow pages to perform better when processing mouse events, for example, and allow profiles to better reflect the actual work being done by a page. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #5329 - [x] There are tests for these changes  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23118)

bors-servo · 2019-04-09T14:56:18Z

⌛ Trying commit d21a163 with merge 1562967...

bors-servo · 2019-04-09T15:01:23Z

💔 Test failed - status-taskcluster

…triggers.

jdm · 2019-04-09T15:26:48Z

@bors-servo try=linux

bors-servo · 2019-04-09T15:37:11Z

⌛ Trying commit 4916f39 with merge b674a2e...

Remove unnecessary explicit reflows These should be unnecessary since the introduction of the catch-all reflow in #9421. Any explicit reflow that is not performing a layout query is unnecessary, and we should be postponing reflow for as long as possible. This should allow pages to perform better when processing mouse events, for example, and allow profiles to better reflect the actual work being done by a page. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #5329 - [x] There are tests for these changes  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23118)

bors-servo · 2019-04-22T16:41:21Z

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

jdm · 2019-05-02T21:06:47Z

@bors-servo r- try-

jdm · 2019-05-24T20:21:37Z

@bors-servo r-

highfive assigned SimonSapin Mar 27, 2019

highfive added the S-awaiting-review There is new code that needs to be reviewed. label Mar 27, 2019

jdm force-pushed the rm-explicit-reflow branch from 49720a3 to d21a163 Compare March 27, 2019 19:08

highfive assigned nox and unassigned SimonSapin Mar 27, 2019

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Mar 28, 2019

highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Mar 28, 2019

jdm force-pushed the rm-explicit-reflow branch from d21a163 to 419f41b Compare April 9, 2019 15:12

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Apr 9, 2019

Remove explicit reflow invocations. Annotate known sources of reflow …

4916f39

…triggers.

jdm force-pushed the rm-explicit-reflow branch from 419f41b to 4916f39 Compare April 9, 2019 15:13

highfive added the S-needs-rebase There are merge conflict errors. label Apr 22, 2019

jdm closed this May 2, 2019

jdm reopened this May 2, 2019

jdm closed this May 24, 2019

jdm reopened this May 24, 2019

mrobinson mentioned this pull request Aug 15, 2024

Remove many explicit reflow calls #33067

Merged

4 tasks

jdm closed this Aug 15, 2024

Uh oh!

Comments

Conversation

jdm commented Mar 27, 2019 • edited by SimonSapin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

highfive commented Mar 27, 2019

Uh oh!

highfive commented Mar 27, 2019

Uh oh!

jdm commented Mar 27, 2019

Uh oh!

nox commented Mar 28, 2019

Uh oh!

bors-servo commented Mar 28, 2019

Uh oh!

bors-servo commented Mar 28, 2019

Uh oh!

bors-servo commented Mar 28, 2019

Uh oh!

jdm commented Mar 28, 2019

Uh oh!

jdm commented Apr 9, 2019

Uh oh!

jdm commented Apr 9, 2019

Uh oh!

bors-servo commented Apr 9, 2019

Uh oh!

jdm commented Apr 9, 2019

Uh oh!

jdm commented Apr 9, 2019

Uh oh!

jdm commented Apr 9, 2019

Uh oh!

jdm commented Apr 9, 2019

Uh oh!

bors-servo commented Apr 9, 2019

Uh oh!

bors-servo commented Apr 9, 2019

Uh oh!

jdm commented Apr 9, 2019

Uh oh!

bors-servo commented Apr 9, 2019

Uh oh!

bors-servo commented Apr 22, 2019

Uh oh!

jdm commented May 2, 2019

Uh oh!

jdm commented May 24, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jdm commented Mar 27, 2019 •

edited by SimonSapin

Loading