Bug 140303

Summary: [Win] Layout Test fast/canvas/canvas-path-addPath.html is failing
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: Layout and RenderingAssignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, cdumez, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 130461    
Bug Blocks:    
Attachments:
Description Flags
Patch simon.fraser: review+

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.