Ensure all iframes are sized correctly at creation #24677

jdm · 2019-11-06T21:31:31Z

Following in the path of #22395, these commits serve several purposes:

prevent layout instability early during iframe loads caused by a succession of resize events
reduce the complexity of determining what actual DPI and initial window size values are being used at startup
ensure that all documents have a correct initial viewport size at creation

These changes fix problems that were exposed by the changes in #24462 but are independent of that PR.

./mach build -d does not report any errors
./mach test-tidy does not report any errors
There are tests for these changes


        layout: Record sizes for iframes that have no pipeline.


        dom: Calculate the viewport size of iframes when they are first added…

… to the tree.


        Centralize definitions of window sizes and DPI in compositor/constell…

…ation during startup.


        script: Ignore redundant resize notifications.


        Ensure layout/script always have a correct viewport size when a new p…

…ipeline is created.

highfive · 2019-11-06T21:31:37Z

Heads up! This PR modifies the following files:

@asajeffrey: components/script/dom/htmliframeelement.rs, components/constellation/pipeline.rs, components/constellation/constellation.rs, components/script/script_thread.rs, components/script/dom/window.rs and 1 more
@cbrewster: components/constellation/pipeline.rs, components/constellation/constellation.rs, components/constellation/session_history.rs
@KiChjang: components/script/dom/htmliframeelement.rs, components/script/script_thread.rs, components/script/dom/window.rs, components/script_traits/lib.rs
@emilio: components/layout/display_list/builder.rs, components/layout/query.rs

jdm · 2019-11-06T21:31:39Z

@bors-servo try=wpt

highfive · 2019-11-06T21:31:39Z

Warning

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


        Auto merge of #24677 - jdm:iframe-sizing-cleanup, r=<try>

Iframe sizing cleanup Following in the path of #22395, these commits serve several purposes: * prevent layout instability early during iframe loads caused by a succession of resize events * reduce the complexity of determining what actual DPI and initial window size values are being used at startup * ensure that all documents have a correct initial viewport size at creation These changes fix problems that were exposed by the changes in #24462 but are independent of that PR. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] There are tests for these changes

bors-servo · 2019-11-06T21:31:44Z

⌛ Trying commit 91dfa35 with merge b09fd92...

jdm · 2019-11-06T21:33:06Z

r? @asajeffrey

jdm · 2019-11-06T21:35:03Z

components/script/dom/htmliframeelement.rs

@@ -173,6 +172,13 @@ impl HTMLIFrameElement {
            replace: replace,
        };

+        let window_size = WindowSizeData {
+            initial_viewport: window
+                .inner_window_dimensions_query(browsing_context_id)


Note: this new query is subtly more correct than the previous use of (effectively) getClientBoundingRect. That query includes the margin/padding/borders that exist on the iframe, so by default that's an extra 2px on every side. The actual viewport for the window should not include these, hence the new layout query.

bors-servo · 2019-11-06T21:58:53Z

💔 Test failed - linux-rel-css


        Update a test that expected a resize event to trigger for an iframe t…

…hat isn't resized.

asajeffrey

LGTM

asajeffrey · 2019-11-06T23:29:57Z

components/script/dom/htmliframeelement.rs

@@ -173,6 +172,13 @@ impl HTMLIFrameElement {
            replace: replace,
        };

+        let window_size = WindowSizeData {
+            initial_viewport: window
+                .inner_window_dimensions_query(browsing_context_id)


jdm · 2019-11-06T23:36:58Z

@bors-servo r=asajeffrey

bors-servo · 2019-11-06T23:36:59Z

📌 Commit 4578ae8 has been approved by asajeffrey

bors-servo · 2019-11-07T08:29:01Z

⌛ Testing commit 4578ae8 with merge 2a43d5e...


        Auto merge of #24677 - jdm:iframe-sizing-cleanup, r=asajeffrey

Ensure all iframes are sized correctly at creation Following in the path of #22395, these commits serve several purposes: * prevent layout instability early during iframe loads caused by a succession of resize events * reduce the complexity of determining what actual DPI and initial window size values are being used at startup * ensure that all documents have a correct initial viewport size at creation These changes fix problems that were exposed by the changes in #24462 but are independent of that PR. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] There are tests for these changes

bors-servo · 2019-11-07T10:09:15Z

💔 Test failed - status-taskcluster

jdm · 2019-11-07T12:40:26Z

@bors-servo retry

bors-servo · 2019-11-07T12:40:31Z

⌛ Testing commit 4578ae8 with merge 9a518cf...


        Auto merge of #24677 - jdm:iframe-sizing-cleanup, r=asajeffrey

Ensure all iframes are sized correctly at creation Following in the path of #22395, these commits serve several purposes: * prevent layout instability early during iframe loads caused by a succession of resize events * reduce the complexity of determining what actual DPI and initial window size values are being used at startup * ensure that all documents have a correct initial viewport size at creation These changes fix problems that were exposed by the changes in #24462 but are independent of that PR. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] There are tests for these changes

bors-servo · 2019-11-07T13:41:12Z

☀️ Test successful - linux-rel-css, linux-rel-wpt, status-community-tc, status-taskcluster
Approved by: asajeffrey
Pushing 9a518cf to master...


        bisect #24677


        Revert "Auto merge of servo#24677 - jdm:iframe-sizing-cleanup, r=asaj…

…effrey" This reverts commit 9a518cf, reversing changes made to a33d493.


        Revert "Auto merge of servo#24677 - jdm:iframe-sizing-cleanup, r=asaj…

…effrey" This reverts commit 9a518cf, reversing changes made to a33d493.


        Revert "Auto merge of #24677 - jdm:iframe-sizing-cleanup, r=asajeffrey"

This reverts commit 9a518cf, reversing changes made to a33d493.


        Revert servo#24677 due to frequent intermittent reftest failures.


        Auto merge of #25566 - jdm:revert-iframes, r=<try>

Revert #24677 due to frequent intermittent reftest failures. This should fix #24726.


        Revert servo#24677 due to frequent intermittent reftest failures.


        Revert servo#24677 due to frequent intermittent reftest failures.

jdm added 5 commits Nov 6, 2019

layout: Record sizes for iframes that have no pipeline.

38e4ae0

dom: Calculate the viewport size of iframes when they are first added…

fd260f7

… to the tree.

Centralize definitions of window sizes and DPI in compositor/constell…

7c365b0

…ation during startup.

script: Ignore redundant resize notifications.

2aa5ddf

Ensure layout/script always have a correct viewport size when a new p…

Loading status checks…

91dfa35

…ipeline is created.

highfive assigned ferjm Nov 6, 2019

highfive added the S-awaiting-review label Nov 6, 2019

jdm mentioned this pull request Nov 6, 2019

Account for pending request state in <img>.complete #24462

Open

4 of 4 tasks complete

jdm changed the title ~~Iframe sizing cleanup~~ Ensure all iframes are sized correctly at creation Nov 6, 2019

highfive assigned asajeffrey and unassigned ferjm Nov 6, 2019

jdm reviewed Nov 6, 2019

View changes

highfive added S-tests-failed and removed S-tests-failed labels Nov 6, 2019

Update a test that expected a resize event to trigger for an iframe t…

Loading status checks…

4578ae8

…hat isn't resized.

jdm force-pushed the jdm:iframe-sizing-cleanup branch from 46f149e to 4578ae8 Nov 6, 2019

asajeffrey approved these changes Nov 6, 2019

View changes

highfive added S-awaiting-merge and removed S-awaiting-review labels Nov 6, 2019

highfive added S-tests-failed and removed S-awaiting-merge labels Nov 7, 2019

highfive added S-awaiting-merge and removed S-tests-failed labels Nov 7, 2019

bors-servo merged commit 4578ae8 into servo:master Nov 7, 2019
2 of 3 checks passed

2 of 3 checks passed

Community-TC (pull_request) TaskGroup: failure
Details

Taskcluster (pull_request) TaskGroup: success
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Nov 7, 2019

This was referenced Nov 13, 2019

Intermittent reftest failures on linux (blank rendering) #24726

Closed

Intermittent failure in /css/css-values/vh-interpolate-vh.html, /css/css-values/vh-interpolate-px.html #24748

Open

jdm added a commit that referenced this pull request Dec 28, 2019

bisect #24677

Loading status checks…

c3fd30a

jdm added a commit to jdm/servo that referenced this pull request Jan 9, 2020

Revert "Auto merge of servo#24677 - jdm:iframe-sizing-cleanup, r=asaj…

574016e

…effrey" This reverts commit 9a518cf, reversing changes made to a33d493.

jdm added a commit to jdm/servo that referenced this pull request Jan 9, 2020

Revert "Auto merge of servo#24677 - jdm:iframe-sizing-cleanup, r=asaj…

cf8896e

…effrey" This reverts commit 9a518cf, reversing changes made to a33d493.

jdm added a commit that referenced this pull request Jan 15, 2020

Revert "Auto merge of #24677 - jdm:iframe-sizing-cleanup, r=asajeffrey"

c385953

This reverts commit 9a518cf, reversing changes made to a33d493.

jdm added a commit to jdm/servo that referenced this pull request Jan 21, 2020

Revert servo#24677 due to frequent intermittent reftest failures.

2d88e47

asajeffrey mentioned this pull request Jan 21, 2020

Revert PR that introduced frequent linux reftest failures #25566

Closed

jdm added a commit to jdm/servo that referenced this pull request Feb 15, 2020

Revert servo#24677 due to frequent intermittent reftest failures.

31074a4

jdm added a commit to jdm/servo that referenced this pull request Feb 15, 2020

Revert servo#24677 due to frequent intermittent reftest failures.

cecdec9

servo / servo

Ensure all iframes are sized correctly at creation #24677

Ensure all iframes are sized correctly at creation #24677

jdm commented Nov 6, 2019

highfive commented Nov 6, 2019

jdm commented Nov 6, 2019

highfive commented Nov 6, 2019

bors-servo commented Nov 6, 2019

jdm commented Nov 6, 2019

This comment has been minimized.

This comment has been minimized.

bors-servo commented Nov 6, 2019

asajeffrey left a comment

This comment has been minimized.

jdm commented Nov 6, 2019

bors-servo commented Nov 6, 2019

bors-servo commented Nov 7, 2019

bors-servo commented Nov 7, 2019

jdm commented Nov 7, 2019

bors-servo commented Nov 7, 2019

bors-servo commented Nov 7, 2019

servo / servo

Join GitHub today

Ensure all iframes are sized correctly at creation #24677

Ensure all iframes are sized correctly at creation #24677

Conversation

jdm commented Nov 6, 2019

highfive commented Nov 6, 2019

jdm commented Nov 6, 2019

highfive commented Nov 6, 2019

bors-servo commented Nov 6, 2019

jdm commented Nov 6, 2019

This comment has been minimized.

jdm Nov 6, 2019

This comment has been minimized.

asajeffrey Nov 6, 2019

bors-servo commented Nov 6, 2019

asajeffrey left a comment

This comment has been minimized.

asajeffrey Nov 6, 2019

jdm commented Nov 6, 2019

bors-servo commented Nov 6, 2019

bors-servo commented Nov 7, 2019

bors-servo commented Nov 7, 2019

jdm commented Nov 7, 2019

bors-servo commented Nov 7, 2019

bors-servo commented Nov 7, 2019