[Webkit-unassigned] [Bug 142694] Update some canvas tests and fix a related bug with unavailable images

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=142694

Chris Dumez <cdumez at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #248686|review?                     |review-
              Flags|                            |

--- Comment #13 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 248686
  --> https://bugs.webkit.org/attachment.cgi?id=248686
Patch

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

> Source/WebCore/ChangeLog:10
> +        these tests and fixes a canvas bug the new tests exposed.

Please explain in detail which bug you fixed and how you changed Web-Exposed behavior. Pointing to a spec and comparing the new behavior to other browsers would also be helpful.

> Source/WebCore/ChangeLog:11
> +

Please indicate in the ChangeLog where you got the new tests from. You can also include the W3C bug URL that explains which tests were replaced by which.

> Source/WebCore/html/HTMLImageElement.h:81
> +    bool unavailable() const;

Ideally, as per coding style, this would be called isUnavaiable(). I would also rename complete() to isComplete() for consistency.

> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:1754
> +    if (image->unavailable())

I don't see any mention of the unavailable state at https://html.spec.whatwg.org/#check-the-usability-of-the-image-argument
Could you clarify why this is the right thing to do here?

> Source/WebCore/loader/ImageLoader.cpp:155
> +        m_imageUnavailable = false;

The spec says we are supposed to move out of "unavailable" state as soon as we have enough data to determine the image's dimensions (https://html.spec.whatwg.org/#img-none). Aren't we moving out of unavailable state too late here?

> Source/WebCore/loader/ImageLoader.cpp:284
> +    m_imageUnavailable = false;

We should have moved out of "unavailable" state as soon as we got enough data to know the image dimensions.

> LayoutTests/ChangeLog:25
> +        This test currently fails and is fixed in https://bugs.webkit.org/show_bug.cgi?id=142677

"and will be fixed in..."

> LayoutTests/TestExpectations:501
> +webkit.org/b/142677 canvas/philip/tests/2d.pattern.image.incomplete.removedsrc.html [ Pass Failure ]

Why [ Pass, Failure] ? This means flaky. Either commit the incorrect expectations (I think this would be OK since you are fixing the test in a follow-up patch) or commit the correct expectations and mark the test as [ Failure ] (ideal).

> LayoutTests/canvas/philip/tests/2d.pattern.image.incomplete.removedsrc-expected.txt:8
> +file:///Users/yweiss/OS/WebKit/LayoutTests/canvas/philip/tests/2d.pattern.image.incomplete.removedsrc.html:24:30

These absolute paths are going to be an issue. Either commit the "correct" expected result or tweak the test so that it does not print absolute paths.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150315/1534c857/attachment-0002.html>