New issue
Advanced search Search tips

Issue 631004 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner: ----
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocked on:
issue 630983



Sign in to add a comment

[Resource Timing] PerformanceResourceTiming size fields are not set correctly for multi-part images

Project Member Reported by ricea@chromium.org, Jul 25 2016

Issue description

What steps will reproduce the problem?
(1) Load a page containing a multipart image (ie. one with content-type multipart/x-mixed-replace) which is not cached.
(2) Examine transferSize for the image URL.

What is the expected output?

transferSize is set to a non-zero value.

What do you see instead?

transferSize is 0.


 

Comment 1 by ricea@chromium.org, Jul 25 2016

Layout tests for this at https://codereview.chromium.org/2176153002

At the moment it crashes due to  issue 630983 .

Comment 2 by ricea@chromium.org, Jul 27 2016

Blockedon: 630983

Comment 3 by ricea@chromium.org, Jul 29 2016

Summary: PerformanceResourceTiming size fields are not set correctly for multi-part images (was: PerformanceResourceTiming transferSize field is not set for multi-part images)
The layout test actually shows transferSize being correct but decodedBodySize and encodedBodySize both being 8469 for a 12242 byte image.

I have modified the title of the bug to match.

Firefox manages to return reasonable results for this test.
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 1 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d0441c41caadbe2029de72434ae7712dccf03662

commit d0441c41caadbe2029de72434ae7712dccf03662
Author: ricea <ricea@chromium.org>
Date: Mon Aug 01 11:15:14 2016

Layout test of PerformanceResourceTiming for multipart

PerformanceResourceTiming fields should be correctly initialised for
multipart images.

They are not, so this CL includes an expectation that
the test will fail.

BUG= 631004 

Review-Url: https://codereview.chromium.org/2176153002
Cr-Commit-Position: refs/heads/master@{#408938}

[add] https://crrev.com/d0441c41caadbe2029de72434ae7712dccf03662/third_party/WebKit/LayoutTests/http/tests/misc/resource-timing-sizes-multipart.html

Comment 5 by ricea@chromium.org, Oct 3 2016

Background: onload is fired at the end of the first frame for multipart images, for backwards compatibility. The PerformanceResourceTiming object is created at the same time. However, the image in the test only has one frame, so it's still mysterious that the sizes come out wrong.

This bug is essentially to track the issue. It's unlikely anyone will fix this, except by deprecating multipart/x-mixed-replace images altogether.
Summary: [Resource Timing] PerformanceResourceTiming size fields are not set correctly for multi-part images (was: PerformanceResourceTiming size fields are not set correctly for multi-part images)
Components: -Blink>PerformanceAPIs Blink>PerformanceAPIs>ResourceTiming

Comment 8 by maxlg@chromium.org, Apr 23 2018

What's the status of this bug? Do we have plan to deprecate multipart/x-mixed-replace images?

Comment 9 by ricea@chromium.org, Apr 24 2018

Status: Ava (was: Assigned)

Comment 10 by ricea@chromium.org, Apr 24 2018

Status: available (was: ava)

Comment 11 by ricea@chromium.org, Apr 24 2018

Owner: ----
#8 entry.encodedBodySize and entry.decodedBodySize no longer return nonsense values, so I will update the layout test.

As far as I know, we are no closer to being able to deprecate multipart/x-mixed-replace images.

Comment 12 by ricea@chromium.org, Apr 24 2018

Actually, now that all the fields have reasonable values, I think I can mark this issue as fixed.
Project Member

Comment 13 by bugdroid1@chromium.org, May 7 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4461b89aec5ffc3d25493e76bfdf0267a9c6181f

commit 4461b89aec5ffc3d25493e76bfdf0267a9c6181f
Author: Adam Rice <ricea@chromium.org>
Date: Mon May 07 05:23:44 2018

Re-enable disabled checks in resource-timing-sizes-multipart.html

The checks of entry.encodedBodySize and entry.decodedBodySize in the
http/tests/misc/resource-timing-sizes-multipart.html layout test were
disabled because the values of those fields were nonsense.

Those fields now have meaningful values, so re-enable the checks.

BUG= 631004 

Change-Id: Id7902c194d03b4f26b9bdab5c891aa21cbddd01c
Reviewed-on: https://chromium-review.googlesource.com/1036965
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556379}
[modify] https://crrev.com/4461b89aec5ffc3d25493e76bfdf0267a9c6181f/third_party/WebKit/LayoutTests/http/tests/misc/resource-timing-sizes-multipart.html

Status: Fixed (was: Available)
Project Member

Comment 15 by bugdroid1@chromium.org, May 9 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b450ddb8349f62bea05765ab5e7fb1de960664c7

commit b450ddb8349f62bea05765ab5e7fb1de960664c7
Author: Adam Rice <ricea@chromium.org>
Date: Wed May 09 07:07:06 2018

Make bufferedAmount unsigned long long

WebSocket bufferedAmount is defined in the HTML standard as being
unsigned long long
(https://html.spec.whatwg.org/#the-websocket-interface), but Blink has
been using "unsigned long" for this value.

Change to "unsigned long long". This also permits us to remove the check
for overflow.

BUG= 631004 

Change-Id: I6c6cc356d9b3c425306376e210b9df7a3be13998
Reviewed-on: https://chromium-review.googlesource.com/1037105
Commit-Queue: Adam Rice <ricea@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557114}
[modify] https://crrev.com/b450ddb8349f62bea05765ab5e7fb1de960664c7/third_party/blink/renderer/modules/websockets/dom_websocket.cc
[modify] https://crrev.com/b450ddb8349f62bea05765ab5e7fb1de960664c7/third_party/blink/renderer/modules/websockets/dom_websocket.h
[modify] https://crrev.com/b450ddb8349f62bea05765ab5e7fb1de960664c7/third_party/blink/renderer/modules/websockets/websocket.idl

Sign in to add a comment