Bug 183327 - [Cairo] Path copy constructor and operator must also copy over CTM
Summary: [Cairo] Path copy constructor and operator must also copy over CTM
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Zan Dobersek
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-03-05 03:56 PST by Zan Dobersek
Modified: 2020-03-16 12:27 PDT (History)
7 users (show)

See Also:

Attachments
Patch (2.88 KB, patch)
2018-03-05 04:03 PST, Zan Dobersek 		cgarcia: review+
Details | Formatted Diff | Diff
Patch for landing (2.63 KB, patch)
2020-03-16 05:43 PDT, Carlos Garcia Campos 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Zan Dobersek 2018-03-05 03:56:03 PST 
[Cairo] Path copy constructor and operator must also copy over CTM
Comment 1 Zan Dobersek 2018-03-05 04:03:49 PST 
Created attachment 334995 [details]
Patch
Comment 2 Carlos Garcia Campos 2018-03-06 02:08:34 PST 
Comment on attachment 334995 [details]
Patch

Wow, I'm surprised this doesn't fix any test, how did you notice? does this fix a particular website, can we add a test?
Comment 3 Zan Dobersek 2018-03-06 03:39:59 PST 
(In reply to Carlos Garcia Campos from comment #2)
> Comment on attachment 334995 [details]
> Patch
> 
> Wow, I'm surprised this doesn't fix any test, how did you notice? does this
> fix a particular website, can we add a test?

Copying of Path objects isn't that common, or at least isn't that well tested. This was spotted while running HTML5 Canvas tests with DisplayList recording enabled. That's one case of a Path objects being copied, since they are stored for later execution. It was still only affecting one single test.

I'll try to come up with a test.
Comment 4 Carlos Garcia Campos 2020-03-16 05:43:04 PDT 
Created attachment 393650 [details]
Patch for landing
Comment 5 WebKit Commit Bot 2020-03-16 08:27:10 PDT 
Comment on attachment 393650 [details]
Patch for landing

Clearing flags on attachment: 393650

Committed r258497: <https://trac.webkit.org/changeset/258497>
Comment 6 WebKit Commit Bot 2020-03-16 08:27:12 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 7 Radar WebKit Bug Importer 2020-03-16 08:28:13 PDT 
<rdar://problem/60495533>
Comment 8 Darin Adler 2020-03-16 12:27:23 PDT 
Comment on attachment 393650 [details]
Patch for landing

I strongly suggest we follow this patch up with the following changes:

- Move the private function swap(Path&) out of #if USE(CG) and implement it for Cairo and Direct2D.

- Move the implementations of the two Path::operator= overloads from PathCG.cpp to Path.cpp.

- Delete the implementations of Path::operator= from Cairo and Direct2D.