Commit 66c9faff authored by Yoav Weiss's avatar Yoav Weiss Committed by Commit Bot

[client-hints] Viewport-Width correct value for navigation requests

Currently, Viewport-Width hints send the display width for navigation
requests, rather than the actual viewport width.
This CL fixes that for iframes and for new windows that have the same
viewport as their opener.

Bug: 825892
Change-Id: Ib01f394325ed3753b9a89dbdbcada1db0d3b029c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2494876
Commit-Queue: Yoav Weiss <yoavweiss@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: default avatarArthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: default avatarTarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826750}
parent 0ccf6420
...@@ -249,27 +249,22 @@ void AddDPRHeader(net::HttpRequestHeaders* headers, ...@@ -249,27 +249,22 @@ void AddDPRHeader(net::HttpRequestHeaders* headers,
void AddViewportWidthHeader(net::HttpRequestHeaders* headers, void AddViewportWidthHeader(net::HttpRequestHeaders* headers,
BrowserContext* context, BrowserContext* context,
const GURL& url) { const GURL& url,
FrameTreeNode* frame_tree_node) {
DCHECK(headers); DCHECK(headers);
DCHECK(context); DCHECK(context);
// The default value on Android. See DCHECK(frame_tree_node);
// https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/css/viewportAndroid.css. int viewport_width = frame_tree_node->current_frame_host()
double viewport_width = 980; ->GetRenderViewHost()
->GetWidget()
#if !defined(OS_ANDROID) ->GetView()
->GetVisibleViewportSize()
.width();
double device_scale_factor = GetDeviceScaleFactor(); double device_scale_factor = GetDeviceScaleFactor();
viewport_width = (display::Screen::GetScreen() viewport_width /= GetZoomFactor(context, url) * device_scale_factor;
->GetPrimaryDisplay()
.GetSizeInPixel()
.width()) /
GetZoomFactor(context, url) / device_scale_factor;
#endif // !OS_ANDROID
DCHECK_LT(0, viewport_width); DCHECK_LT(0, viewport_width);
// TODO(yoav): Find out why this 0 check is needed... SetHeaderToInt(headers, network::mojom::WebClientHintsType::kViewportWidth,
if (viewport_width > 0) { viewport_width);
SetHeaderToInt(headers, network::mojom::WebClientHintsType::kViewportWidth,
viewport_width);
}
} }
void AddRttHeader(net::HttpRequestHeaders* headers, void AddRttHeader(net::HttpRequestHeaders* headers,
...@@ -623,7 +618,7 @@ void AddNavigationRequestClientHintsHeaders( ...@@ -623,7 +618,7 @@ void AddNavigationRequestClientHintsHeaders(
} }
if (ShouldAddClientHint(data, if (ShouldAddClientHint(data,
network::mojom::WebClientHintsType::kViewportWidth)) { network::mojom::WebClientHintsType::kViewportWidth)) {
AddViewportWidthHeader(headers, context, url); AddViewportWidthHeader(headers, context, url, frame_tree_node);
} }
network::NetworkQualityTracker* network_quality_tracker = network::NetworkQualityTracker* network_quality_tracker =
delegate->GetNetworkQualityTracker(); delegate->GetNetworkQualityTracker();
......
...@@ -6201,6 +6201,8 @@ crbug.com/1139052 virtual/webrtc-wpt-plan-b/external/wpt/webrtc/protocol/rtp-dem ...@@ -6201,6 +6201,8 @@ crbug.com/1139052 virtual/webrtc-wpt-plan-b/external/wpt/webrtc/protocol/rtp-dem
crbug.com/1137228 [ Mac ] external/wpt/infrastructure/testdriver/click_iframe_crossorigin.sub.html [ Pass Failure ] crbug.com/1137228 [ Mac ] external/wpt/infrastructure/testdriver/click_iframe_crossorigin.sub.html [ Pass Failure ]
crbug.com/825892 external/wpt/client-hints/viewport-width-window-different-dimensions.https.html [ Failure ]
# Rename document.featurePolicy to document.permissionsPolicy # Rename document.featurePolicy to document.permissionsPolicy
crbug.com/1123116 external/wpt/permissions-policy/payment-supported-by-permissions-policy.tentative.html [ Failure ] crbug.com/1123116 external/wpt/permissions-policy/payment-supported-by-permissions-policy.tentative.html [ Failure ]
crbug.com/1123116 external/wpt/permissions-policy/permissions-policy-frame-policy-allowed-for-all.https.sub.html [ Failure ] crbug.com/1123116 external/wpt/permissions-policy/permissions-policy-frame-policy-allowed-for-all.https.sub.html [ Failure ]
......
def main(request, response):
"""
postMessage Viewport-Width headers
"""
if b"viewport-width" in request.headers:
result = request.headers["viewport-width"]
else:
result = u"FAIL"
headers = [(b"Content-Type", b"text/html"),
(b"Access-Control-Allow-Origin", b"*")]
content = b'''
<script>
let parentOrOpener = window.opener || window.parent;
parentOrOpener.postMessage({ viewport: '%s' }, "*");
</script>
''' % (result)
return 200, headers, content
<script>
(async () => {
const response = await fetch("viewport.py");
const body = await response.text();
parent.postMessage(body, "*");
})();
</script>
def main(request, response):
"""
Reflect Viewport-Width headers
"""
if b"viewport-width" in request.headers:
result = request.headers["viewport-width"]
else:
result = u"FAIL"
headers = [(b"Content-Type", b"text/html"),
(b"Access-Control-Allow-Origin", b"*")]
return 200, headers, result
<html>
<body>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script>
promise_test(t => {
return new Promise(resolve => {
window.addEventListener("message", t.step_func(e => {
assert_equals(e.data.viewport, window.innerWidth.toString());
resolve();
}));
});
});
</script>
<iframe src="resources/viewport-frame.py" width=503></iframe>
<html>
<body>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script>
promise_test(t => {
return new Promise(resolve => {
window.addEventListener("message", t.step_func(e => {
assert_equals(e.data, "503");
resolve();
}));
});
});
</script>
<iframe src="resources/viewport-measurement.html" width=503>
<html>
<body>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script>
promise_test(t => {
return new Promise(resolve => {
window.addEventListener("message", t.step_func(e => {
assert_equals(e.data.viewport, "503");
resolve();
}));
});
});
window.open("resources/viewport-frame.py", "", "width=503");
</script>
<html>
<body>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script>
promise_test(t => {
return new Promise(resolve => {
window.addEventListener("message", t.step_func(e => {
assert_equals(e.data.viewport, window.innerWidth.toString());
resolve();
}));
});
});
window.open("resources/viewport-frame.py", "");
</script>
This is a testharness.js-based test.
FAIL viewport-width-frame assert_equals: expected "800" but got "FAIL"
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL viewport-width-iframe assert_equals: expected "800" but got "FAIL"
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL viewport-width-subresource assert_equals: expected "503" but got "FAIL"
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL viewport-width-window assert_equals: expected "800" but got "FAIL"
Harness: the test ran to completion.
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment