Avoid redundant initialization of GraphicsContextState.

Avoid redundant initialization of GraphicsContextState.

When creating a new GraphicsContextState the constructor initializes the state
with various default values. In GraphicsContext::realizePaintState we immediately
overwrite these values after creation. This patch adds a copy constructor to
GraphicsContextState to avoid that.

Even though this seems like a clear win, the benefits are not visible when
profiling. Looks like the overhead of the redundant initialization is
not very large. Still seems worth fixing though.

BUG=377687
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=174985

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=175894

[Dominik Grewe, 2014-05-27 17:14:03]

ptal.

[f(malita), 2014-05-27 18:16:43]

This is likely not showing up in profiles because we're re-using
GraphicsContextState stack entries (so the initialization churn only happens
when the stack high water mark moves up). Thus the benefit is minimal - but
maybe still worth landing.

https://codereview.chromium.org/303613002/diff/40001/Source/platform/graphics...
File Source/platform/graphics/GraphicsContextState.h (right):

https://codereview.chromium.org/303613002/diff/40001/Source/platform/graphics...
Source/platform/graphics/GraphicsContextState.h:50: static
PassOwnPtr<GraphicsContextState> create()
Let's update this instead: create(const GraphicsContextState& other).

https://codereview.chromium.org/303613002/diff/40001/Source/platform/graphics...
Source/platform/graphics/GraphicsContextState.h:55: static
PassOwnPtr<GraphicsContextState> createAndCopy(const GraphicsContextState*
other)
realizePaintSave() is the only user of create(), no need to add a new variant.

https://codereview.chromium.org/303613002/diff/40001/Source/platform/graphics...
Source/platform/graphics/GraphicsContextState.h:60: void copy(const
GraphicsContextState*);
Let's also update this to a const reference argument.

[Dominik Grewe, 2014-05-27 19:59:21]

https://codereview.chromium.org/303613002/diff/40001/Source/platform/graphics...
File Source/platform/graphics/GraphicsContextState.h (right):

https://codereview.chromium.org/303613002/diff/40001/Source/platform/graphics...
Source/platform/graphics/GraphicsContextState.h:55: static
PassOwnPtr<GraphicsContextState> createAndCopy(const GraphicsContextState*
other)
On 2014/05/27 18:16:44, Florin Malita wrote:
> realizePaintSave() is the only user of create(), no need to add a new variant.

The GraphicsContext constructor also calls create, so we want to keep it.

https://codereview.chromium.org/303613002/diff/40001/Source/platform/graphics...
Source/platform/graphics/GraphicsContextState.h:60: void copy(const
GraphicsContextState*);
On 2014/05/27 18:16:44, Florin Malita wrote:
> Let's also update this to a const reference argument.

Will do.

[Dominik Grewe, 2014-05-28 09:25:35]

On 2014/05/27 19:59:21, Dominik Grewe wrote:
>
https://codereview.chromium.org/303613002/diff/40001/Source/platform/graphics...
> File Source/platform/graphics/GraphicsContextState.h (right):
> 
>
https://codereview.chromium.org/303613002/diff/40001/Source/platform/graphics...
> Source/platform/graphics/GraphicsContextState.h:55: static
> PassOwnPtr<GraphicsContextState> createAndCopy(const GraphicsContextState*
> other)
> On 2014/05/27 18:16:44, Florin Malita wrote:
> > realizePaintSave() is the only user of create(), no need to add a new
variant.
> 
> The GraphicsContext constructor also calls create, so we want to keep it.
> 
>
https://codereview.chromium.org/303613002/diff/40001/Source/platform/graphics...
> Source/platform/graphics/GraphicsContextState.h:60: void copy(const
> GraphicsContextState*);
> On 2014/05/27 18:16:44, Florin Malita wrote:
> > Let's also update this to a const reference argument.
> 
> Will do.

Updated. Ptal.

[f(malita), 2014-05-28 12:59:18]

lgtm

https://codereview.chromium.org/303613002/diff/50001/Source/platform/graphics...
File Source/platform/graphics/GraphicsContextState.h (right):

https://codereview.chromium.org/303613002/diff/50001/Source/platform/graphics...
Source/platform/graphics/GraphicsContextState.h:149: explicit
GraphicsContextState(const GraphicsContextState&);
Since we're dropping WTF_MAKE_NONCOPYABLE but only defining a copy constructor,
let's keep a private GraphicsContextState& operator=(const
GraphicsContextState&) decl here.

[Dominik Grewe, 2014-05-28 14:52:09]

https://codereview.chromium.org/303613002/diff/50001/Source/platform/graphics...
File Source/platform/graphics/GraphicsContextState.h (right):

https://codereview.chromium.org/303613002/diff/50001/Source/platform/graphics...
Source/platform/graphics/GraphicsContextState.h:149: explicit
GraphicsContextState(const GraphicsContextState&);
On 2014/05/28 12:59:19, Florin Malita wrote:
> Since we're dropping WTF_MAKE_NONCOPYABLE but only defining a copy
constructor,
> let's keep a private GraphicsContextState& operator=(const
> GraphicsContextState&) decl here.

Done.

[Dominik Grewe, 2014-05-28 17:25:09]

The CQ bit was checked by dominikg@chromium.org

[commit-bot: I haz the power, 2014-05-28 17:25:39]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dominikg@chromium.org/303613002/60001

[commit-bot: I haz the power, 2014-05-28 18:42:31]

Change committed as 174985

[Julien - ping for review, 2014-05-30 22:15:24]

A revert of this CL has been created in
https://codereview.chromium.org/303403006/ by jchaffraix@chromium.org.

The reason for reverting is: This change is the cause of a
leak in PhishingDOMFeatureExtractorTest.ScriptAndImageFeatures
under LSAN. See
http://build.chromium.org/p/chromium.memory/builders/Linux%20ASan%20LSan%20Te....

[Dominik Grewe, 2014-06-10 12:57:32]

I couldn't find any evidence that this CL was responsible for the memory leaks.
None of the functions touched here showed up in the logs.
I've run the failing test with this patch on a LSan build and couldn't repro the
leaks.

I'll re-land.

[Dominik Grewe, 2014-06-10 12:58:57]

The CQ bit was checked by dominikg@chromium.org

[commit-bot: I haz the power, 2014-06-10 12:59:58]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dominikg@chromium.org/303613002/80001

[commit-bot: I haz the power, 2014-06-10 14:11:34]

Change committed as 175894