Remove unnecessary style calculations in HTMLCanvasElement

Remove unnecessary style calculations in HTMLCanvasElement

When creating a WebGL canvas, and when allocating a 2D canvas backing,
the implementation was triggering an immediate style calculation
to evaluate the image-redering CSS property.  This CL gets rid of of
these calculations by relying on the fact that if the style is dirty,
it will be updated before the next paint, which will allow for the state
to be corrected after the fact, if necessary.

BUG=585135
Committed: https://crrev.com/4f3d842e26a4e568985fcbf98f24be49c324898e
Cr-Commit-Position: refs/heads/master@{#374753}

[Justin Novosad, 2016-02-08 21:58:18]

junov@chromium.org changed reviewers:
+ chrishtr@chromium.org

[Justin Novosad, 2016-02-08 21:58:19]

PTAL

[Justin Novosad, 2016-02-08 22:00:14]

Description was changed from

==========
Revove unnecessary style calculations in HTMLCanvasElement

When creating a WebGL canvas, and when allocating a 2D canvas backing,
the implementation was triggering an immediate style calculation
to evaluate the image-redering CSS property.  This CL gets rid of of
these calculations by relying on the fact that if the style is dirty,
it will be update before the next paint, which will allow for the state
to be corrected after the fact, if necessary.

BUG=585135
==========

to

==========
Revove unnecessary style calculations in HTMLCanvasElement

When creating a WebGL canvas, and when allocating a 2D canvas backing,
the implementation was triggering an immediate style calculation
to evaluate the image-redering CSS property.  This CL gets rid of of
these calculations by relying on the fact that if the style is dirty,
it will be updated before the next paint, which will allow for the state
to be corrected after the fact, if necessary.

BUG=585135
==========

[chrishtr, 2016-02-09 18:11:20]

https://codereview.chromium.org/1674123003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp (right):

https://codereview.chromium.org/1674123003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:270:
updateContextFilterQuality();
Why do you still need to update the filter quality? Shouldn't that also be set
naturally as part of the lifecycle's style update?

[Justin Novosad, 2016-02-09 19:55:23]

On 2016/02/09 18:11:20, chrishtr wrote:
>
https://codereview.chromium.org/1674123003/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp (right):
> 
>
https://codereview.chromium.org/1674123003/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:270:
> updateContextFilterQuality();
> Why do you still need to update the filter quality? Shouldn't that also be set
> naturally as part of the lifecycle's style update?

The use of external texture layers for WebGL and accelerated 2d canvas
complicates things... 
Anyways, this fix doe not even work (tests fail) because didRecalcStyle does not
get called when I expected.
It seems that didRecalcStyle is not invoked the first time style is computed on
the canvas. That's weird because we call setHasCustomStyleCallbacks() right in
the constructor.

[Justin Novosad, 2016-02-09 20:20:26]

New patch. 
This works: I just handle the style during paint.  Even though
HTMLCanvasElement::paint is a no-op for GPU accelerated canvases, this is still
an appropriate place to handle the style since paint is guaranteed to come
before committing the content layers to the compositor.

[chrishtr, 2016-02-10 17:53:41]

https://codereview.chromium.org/1674123003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp (right):

https://codereview.chromium.org/1674123003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:439: const
ComputedStyle* style = ensureComputedStyle();
The change looks good, but one final step: HTMLCanvasElement::paint should be
in HTMLCanvasPainter, this is I think the only paint method that calls back into
a DOM class.

[Justin Novosad, 2016-02-10 18:05:19]

On 2016/02/10 17:53:41, chrishtr wrote:
>
https://codereview.chromium.org/1674123003/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp (right):
> 
>
https://codereview.chromium.org/1674123003/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:439: const
> ComputedStyle* style = ensureComputedStyle();
> The change looks good, but one final step: HTMLCanvasElement::paint should be
> in HTMLCanvasPainter, this is I think the only paint method that calls back
into
> a DOM class.

That would require making a lot of the canvas element's internals public.  Why
is that a better design?
I was actually planning to refactor in the opposite direction and to push most
of the implementation of paint() down into the RenderingContext classes in order
to have simple individual implementations (no more mixed logic of the form
if(2d) do this, if (3d) do that...)

[chrishtr, 2016-02-10 18:36:32]

On 2016/02/10 at 18:05:19, junov wrote:
> On 2016/02/10 17:53:41, chrishtr wrote:
> >
https://codereview.chromium.org/1674123003/diff/40001/third_party/WebKit/Sour...
> > File third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp (right):
> > 
> >
https://codereview.chromium.org/1674123003/diff/40001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:439: const
> > ComputedStyle* style = ensureComputedStyle();
> > The change looks good, but one final step: HTMLCanvasElement::paint should
be
> > in HTMLCanvasPainter, this is I think the only paint method that calls back
into
> > a DOM class.
> 
> That would require making a lot of the canvas element's internals public.  Why
is that a better design?
> I was actually planning to refactor in the opposite direction and to push most
of the implementation of paint() down into the RenderingContext classes in order
to have simple individual implementations (no more mixed logic of the form
if(2d) do this, if (3d) do that...)

The argument for was to be in line with other painter. But looking more at the
code, I recall again that canvas is different than other things, since it is an
immediate mode API vs retained.
I withdraw the suggestion. ;)

[chrishtr, 2016-02-10 18:36:35]

lgtm

[Justin Novosad, 2016-02-10 19:10:37]

Description was changed from

==========
Revove unnecessary style calculations in HTMLCanvasElement

When creating a WebGL canvas, and when allocating a 2D canvas backing,
the implementation was triggering an immediate style calculation
to evaluate the image-redering CSS property.  This CL gets rid of of
these calculations by relying on the fact that if the style is dirty,
it will be updated before the next paint, which will allow for the state
to be corrected after the fact, if necessary.

BUG=585135
==========

to

==========
Remove unnecessary style calculations in HTMLCanvasElement

When creating a WebGL canvas, and when allocating a 2D canvas backing,
the implementation was triggering an immediate style calculation
to evaluate the image-redering CSS property.  This CL gets rid of of
these calculations by relying on the fact that if the style is dirty,
it will be updated before the next paint, which will allow for the state
to be corrected after the fact, if necessary.

BUG=585135
==========

[Justin Novosad, 2016-02-10 19:11:00]

The CQ bit was checked by junov@chromium.org

[commit-bot: I haz the power, 2016-02-10 19:11:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1674123003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1674123003/40001

[commit-bot: I haz the power, 2016-02-10 22:39:13]

Description was changed from

==========
Remove unnecessary style calculations in HTMLCanvasElement

When creating a WebGL canvas, and when allocating a 2D canvas backing,
the implementation was triggering an immediate style calculation
to evaluate the image-redering CSS property.  This CL gets rid of of
these calculations by relying on the fact that if the style is dirty,
it will be updated before the next paint, which will allow for the state
to be corrected after the fact, if necessary.

BUG=585135
==========

to

==========
Remove unnecessary style calculations in HTMLCanvasElement

When creating a WebGL canvas, and when allocating a 2D canvas backing,
the implementation was triggering an immediate style calculation
to evaluate the image-redering CSS property.  This CL gets rid of of
these calculations by relying on the fact that if the style is dirty,
it will be updated before the next paint, which will allow for the state
to be corrected after the fact, if necessary.

BUG=585135
==========

[commit-bot: I haz the power, 2016-02-10 22:39:14]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-02-16 22:31:49]

Description was changed from

==========
Remove unnecessary style calculations in HTMLCanvasElement

When creating a WebGL canvas, and when allocating a 2D canvas backing,
the implementation was triggering an immediate style calculation
to evaluate the image-redering CSS property.  This CL gets rid of of
these calculations by relying on the fact that if the style is dirty,
it will be updated before the next paint, which will allow for the state
to be corrected after the fact, if necessary.

BUG=585135
==========

to

==========
Remove unnecessary style calculations in HTMLCanvasElement

When creating a WebGL canvas, and when allocating a 2D canvas backing,
the implementation was triggering an immediate style calculation
to evaluate the image-redering CSS property.  This CL gets rid of of
these calculations by relying on the fact that if the style is dirty,
it will be updated before the next paint, which will allow for the state
to be corrected after the fact, if necessary.

BUG=585135

Committed: https://crrev.com/4f3d842e26a4e568985fcbf98f24be49c324898e
Cr-Commit-Position: refs/heads/master@{#374753}
==========

[commit-bot: I haz the power, 2016-02-16 22:31:50]

Patchset 3 (id:??) landed as
https://crrev.com/4f3d842e26a4e568985fcbf98f24be49c324898e
Cr-Commit-Position: refs/heads/master@{#374753}