Bug 96239 - Add partial load test for JPEG images
Summary: Add partial load test for JPEG images
Status: RESOLVED LATER
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-10 00:11 PDT by noel gordon
Modified: 2012-10-05 00:19 PDT (History)
1 user (show)

See Also:


Attachments
Patch (267.96 KB, patch)
2012-09-10 00:18 PDT, noel gordon
no flags Details | Formatted Diff | Diff
Patch (267.95 KB, patch)
2012-09-21 01:22 PDT, noel gordon
yutak: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description noel gordon 2012-09-10 00:11:52 PDT
Add partial load test for JPEG images: load partial number of bytes, and stall forever.  The partial image should be decoded and drawn, and any image background CSS color should be visible.
Comment 1 noel gordon 2012-09-10 00:18:16 PDT
Created attachment 163052 [details]
Patch
Comment 2 Yuta Kitamura 2012-09-20 02:11:48 PDT
Comment on attachment 163052 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=163052&action=review

Could you elaborate on the motivation of this test? Were there any regressions in the past? Do you have any reference to the specification? Are there any other tests that cover this behavior?

And I'm a little concerned about the license of the picture.

r- for a style issue mentioned in in-line comments.

> LayoutTests/http/tests/images/jpeg-partial-load.html:20
> +    if (window.testRunner) testRunner.notifyDone();

We usually don't fold "if" expression into one line.

See <http://www.webkit.org/coding/coding-style.html#linebreaking-multiple-statements>.

> LayoutTests/http/tests/images/jpeg-partial-load.html:26
> +    setTimeout(testDone, 500);

The use of setTimeout() like this can be the source of test flakiness, so it's recommended to use other reliable notification method whenever possible. I'm not sure if there's a workaround in this case, though.
Comment 3 noel gordon 2012-09-21 00:59:41 PDT
(In reply to comment #2)
> (From update of attachment 163052 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=163052&action=review
> 
> Could you elaborate on the motivation of this test? Were there any regressions in the past? 

Bug 78239 is listed as a dependent of the current bug: it leads to http://code.google.com/p/chromium/issues/detail?id=113171 for example.


> Do you have any reference to the specification? Are there any other tests that cover this behavior?

Bug 78239 added http/tests/incremental/partial-jpeg.html but the test was flaky, bug 82480.  I have recently removed that test.

Bug 93171 added http/tests/images/jpg-img-partial-load.html but the test fails on the Mountain Lion webkit port https://bugs.webkit.org/show_bug.cgi?id=93171#c13 and bug 93636.  The test also does not work for me on the Snow Leopard webkit port during local testing (FAIL: Timed out waiting for notifyDone to be called).  I intend to remove the test.


> And I'm a little concerned about the license of the picture.

I have no concerns: http://lmgtfy.com/?q=lenna+image


> r- for a style issue mentioned in in-line comments.
> 
> > LayoutTests/http/tests/images/jpeg-partial-load.html:20
> > +    if (window.testRunner) testRunner.notifyDone();
> 
> We usually don't fold "if" expression into one line.

Yes, my mistake.


> > LayoutTests/http/tests/images/jpeg-partial-load.html:26
> > +    setTimeout(testDone, 500);
> 
> The use of setTimeout() like this can be the source of test flakiness, so it's recommended to use other reliable notification method whenever possible. I'm not sure if there's a workaround in this case, though.

Any flakiness appears to come from DRT network layers not delivering payload in the time allocated to decode the partial image bytes looking at http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=http%2Ftests%2Fimages and I have had no reports of the PNG tests being flaky on the mac ports so far.  The JPEG test is based on them.
Comment 4 noel gordon 2012-09-21 01:22:48 PDT
Created attachment 165076 [details]
Patch
Comment 5 Yuta Kitamura 2012-09-21 01:38:22 PDT
(In reply to comment #3)
> Bug 93171 added http/tests/images/jpg-img-partial-load.html but the test fails on the Mountain Lion webkit port https://bugs.webkit.org/show_bug.cgi?id=93171#c13 and bug 93636.  The test also does not work for me on the Snow Leopard webkit port during local testing (FAIL: Timed out waiting for notifyDone to be called).  I intend to remove the test.

Okay.

> > And I'm a little concerned about the license of the picture.
> 
> I have no concerns: http://lmgtfy.com/?q=lenna+image

I know that image, and it's probably 99% safe to use it, but not 100%. The
image is copyrighted, and the copyright has not expired yet AFAIK. I may be
too pedantic, but we need to be extremely careful about legal issues.

Instead of this image, please use one of the following:
- an image that is already used in existing tests,
- an image that is provided under the license compatible to ours, or
- an image you own the copyright for.

If you fix the image and the style issue I mentioned before, I would be happy
to r+ this change.

> 
> Any flakiness appears to come from DRT network layers not delivering payload in the time allocated to decode the partial image bytes looking at http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=http%2Ftests%2Fimages and I have had no reports of the PNG tests being flaky on the mac ports so far.  The JPEG test is based on them.

Hm, okay.
Comment 6 Yuta Kitamura 2012-09-21 01:40:15 PDT
Comment on attachment 165076 [details]
Patch

r- for copyright concerns mentioned above.
Comment 7 noel gordon 2012-10-04 23:34:10 PDT
(In reply to comment #5)
> (In reply to comment #3)
> > Bug 93171 added http/tests/images/jpg-img-partial-load.html but the test fails on the Mountain Lion webkit port https://bugs.webkit.org/show_bug.cgi?id=93171#c13 and bug 93636.  The test also does not work for me on the Snow Leopard webkit port during local testing (FAIL: Timed out waiting for notifyDone to be called).  I intend to remove the test.
> 
> Okay.
> 
> > > And I'm a little concerned about the license of the picture.
> > 
> > I have no concerns: http://lmgtfy.com/?q=lenna+image
> 
> I know that image, and it's probably 99% safe to use it, but not 100%. The
> image is copyrighted, and the copyright has not expired yet AFAIK. I may be
> too pedantic, but we need to be extremely careful about legal issues.

the image is already used, and if you do know this image then you alos know that the owner does not care about its copyright [1]

[1] http://www.wired.com/culture/lifestyle/news/1997/05/4000
Comment 8 Yuta Kitamura 2012-10-05 00:04:31 PDT
(In reply to comment #7)
> the image is already used, and if you do know this image then you alos know that the owner does not care about its copyright [1]
> 
> [1] http://www.wired.com/culture/lifestyle/news/1997/05/4000

Are you really sure that the declaration has a legal effect in countries all
over the world? For example, in Japan, a person is not allowed to throw away
some rights derived from his/her own copyright.

As you know, WebKit code is used by many international companies, and we set
a high bar on legal standards. We don't want to take a slightest legal risk.

Please replace the image. That should be easy.