Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(370)

Issue 2842343002: Perform layout when viewport size is queried. (Closed)

Created:
3 years, 8 months ago by bokan
Modified:
3 years, 7 months ago
Reviewers:
skobes
CC:
chromium-reviews, blink-reviews, blink-reviews-frames_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Perform layout when viewport size is queried. If a Frame has non-overlay scrollbars, the visualViewport's dimensions should exclude them. In order to know whether the Frame has scrollbars, we need to perform any pending layouts. This was only broken for iframes as the main frame viewport size depends on page scale. Page scale depends on layout to determine the minimum/initial page scale so we already performed layout when the main frame's viewport was queried. BUG=714829 Review-Url: https://codereview.chromium.org/2842343002 Cr-Commit-Position: refs/heads/master@{#467718} Committed: https://chromium.googlesource.com/chromium/src/+/396aeae063fa2f8a1e7422bc86a568239bf8fca4

Patch Set 1 #

Patch Set 2 : Add a test for main frame too #

Patch Set 3 : Fix comment #

Total comments: 1

Patch Set 4 : Nit #

Unified diffs Side-by-side diffs Delta from patch set Stats (+81 lines, -5 lines) Patch
A third_party/WebKit/LayoutTests/fast/dom/viewport/read-viewport-size-causes-layout.html View 1 1 chunk +34 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fast/dom/viewport/read-viewport-size-in-iframe-causes-layout.html View 1 2 3 1 chunk +35 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/frame/LocalDOMWindow.cpp View 1 2 2 chunks +12 lines, -5 lines 0 comments Download

Messages

Total messages: 18 (12 generated)
bokan
3 years, 8 months ago (2017-04-26 22:33:47 UTC) #3
skobes
lgtm https://codereview.chromium.org/2842343002/diff/40001/third_party/WebKit/LayoutTests/fast/dom/viewport/read-viewport-size-in-iframe-causes-layout.html File third_party/WebKit/LayoutTests/fast/dom/viewport/read-viewport-size-in-iframe-causes-layout.html (right): https://codereview.chromium.org/2842343002/diff/40001/third_party/WebKit/LayoutTests/fast/dom/viewport/read-viewport-size-in-iframe-causes-layout.html#newcode22 third_party/WebKit/LayoutTests/fast/dom/viewport/read-viewport-size-in-iframe-causes-layout.html:22: window.frames[0].window.document.body.style.width = "2000px"; nit: no need for leading ...
3 years, 8 months ago (2017-04-27 01:12:48 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2842343002/60001
3 years, 7 months ago (2017-04-27 14:41:12 UTC) #11
commit-bot: I haz the power
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/281252)
3 years, 7 months ago (2017-04-27 16:22:10 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2842343002/60001
3 years, 7 months ago (2017-04-27 16:24:15 UTC) #15
commit-bot: I haz the power
3 years, 7 months ago (2017-04-27 17:40:35 UTC) #18
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/396aeae063fa2f8a1e7422bc86a5...

Powered by Google App Engine
This is Rietveld 408576698