Bug 140303 - [Win] Layout Test fast/canvas/canvas-path-addPath.html is failing
Summary: [Win] Layout Test fast/canvas/canvas-path-addPath.html is failing
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords: InRadar
Depends on: 130461
Blocks:
  Show dependency treegraph
 
Reported: 2015-01-09 11:56 PST by Brent Fulgham
Modified: 2015-01-09 17:31 PST (History)
3 users (show)

See Also:


Attachments
Patch (1.57 KB, patch)
2015-01-09 12:03 PST, Brent Fulgham
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2015-01-09 11:56:19 PST
The following layout test is failing on Windows.

fast/canvas/canvas-path-addPath.html

Probable cause:

This is crashing because we are attempting to add a path to itself in Path::addPath. The bug is here:

void Path::addPath(const Path& path, const AffineTransform& transform)
{
    if (!path.platformPath())
        return;

    if (!transform.isInvertible())
        return;

    CGAffineTransform transformCG = transform;
    // CG doesn't allow adding a path to itself. Optimize for the common case
    // and copy the path for the self referencing case.
    if (ensurePlatformPath() != path.platformPath()) {
        CGPathAddPath(ensurePlatformPath(), &transformCG, path.platformPath());
        return;
    }
    CGPathRef pathCopy = CGPathCreateCopy(path.platformPath());
    CGPathAddPath(ensurePlatformPath(), &transformCG, path.platformPath());
    CGPathRelease(pathCopy);
}

Note that even though we are creating the pathCopy, we are not using it. Instead, we use the original path (violating the contract of the CG API).

I'm not sure why this code seems to work on Mac, but it crashes on Windows.
Comment 1 Brent Fulgham 2015-01-09 11:59:08 PST
<rdar://problem/19428865>
Comment 2 Brent Fulgham 2015-01-09 12:03:21 PST
Created attachment 244359 [details]
Patch
Comment 3 Brent Fulgham 2015-01-09 12:17:38 PST
Committed r178186: <http://trac.webkit.org/changeset/178186>
Comment 4 Chris Dumez 2015-01-09 16:18:01 PST
May have caused 1 test failure on the bots (may be a simple rebaseline):
https://build.webkit.org/results/Apple%20MountainLion%20Debug%20WK2%20(Tests)/r178186%20(20724)/results.html
Comment 5 Brent Fulgham 2015-01-09 17:29:45 PST
(In reply to comment #4)
> May have caused 1 test failure on the bots (may be a simple rebaseline):
> https://build.webkit.org/results/
> Apple%20MountainLion%20Debug%20WK2%20(Tests)/r178186%20(20724)/results.html

Interesting. Looking at the history for this test on our platform (<http://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=css3%2Fbackground%2Fbackground-repeat-space-content.html>), it looks as if a change between r178181 and r178183 introduced the slight image difference. (See http://trac.webkit.org/log/?verbose=on&rev=178183&stop_rev=178181>)

Looking at the whole set of bots, it looks like r178183 is the change that triggered the image difference.
Comment 6 Brent Fulgham 2015-01-09 17:31:16 PST
(In reply to comment #4)
> May have caused 1 test failure on the bots (may be a simple rebaseline):
> https://build.webkit.org/results/
> Apple%20MountainLion%20Debug%20WK2%20(Tests)/r178186%20(20724)/results.html

I think it was Bug 140298 that introduced that small image difference. I've updated the bug with some notes.