compositor-worker: Add mutable attributes to CompositorProxy.

compositor-worker: Add mutable attributes to CompositorProxy.

Add some mutable attributes to CompositorProxy. These attributes are mutable
only in the compositor-worker thread, and only during requestCompositorFrame
callbacks (this is currently not-implemented in this CL, and will be added
later). Trying to mutate these attributes from the main-thread raises
exceptions.

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

[sadrul, 2015-03-28 22:29:22]

sadrul@chromium.org changed reviewers:
+ esprehn@chromium.org

[esprehn, 2015-03-29 04:44:57]

lgtm w/ nits

https://codereview.chromium.org/1025893002/diff/1/Source/core/dom/CompositorP...
File Source/core/dom/CompositorProxy.h (right):

https://codereview.chromium.org/1025893002/diff/1/Source/core/dom/CompositorP...
Source/core/dom/CompositorProxy.h:60: uint32_t m_mutatedAttributes;
Maybe use a bit field instead of the enum?

https://codereview.chromium.org/1025893002/diff/80001/LayoutTests/fast/compos...
File LayoutTests/fast/compositorworker/compositor-attribute-change-worker.html
(right):

https://codereview.chromium.org/1025893002/diff/80001/LayoutTests/fast/compos...
LayoutTests/fast/compositorworker/compositor-attribute-change-worker.html:2:
<div id='container'></div>
double quotes? same with the other test, it's nice to be consistent.

https://codereview.chromium.org/1025893002/diff/80001/Source/core/dom/Composi...
File Source/core/dom/CompositorProxy.cpp (right):

https://codereview.chromium.org/1025893002/diff/80001/Source/core/dom/Composi...
Source/core/dom/CompositorProxy.cpp:57: // TODO(sad):
TODO what?

https://codereview.chromium.org/1025893002/diff/80001/Source/core/dom/Composi...
Source/core/dom/CompositorProxy.cpp:180: m_opacity = opacity;
opacity must be positive, you probably want to just ignore negative values here
(or throw?)

https://codereview.chromium.org/1025893002/diff/80001/Source/core/dom/Composi...
File Source/core/dom/CompositorProxy.h (right):

https://codereview.chromium.org/1025893002/diff/80001/Source/core/dom/Composi...
Source/core/dom/CompositorProxy.h:67: uint32_t m_mutatedAttributes;
Why not set all of these to = 0 also?

https://codereview.chromium.org/1025893002/diff/80001/Source/core/dom/Composi...
Source/core/dom/CompositorProxy.h:69: double m_opacity = 0;
Is this allowed in blink C++11 style? :)

[sadrul, 2015-03-30 16:07:45]

https://codereview.chromium.org/1025893002/diff/80001/LayoutTests/fast/compos...
File LayoutTests/fast/compositorworker/compositor-attribute-change-worker.html
(right):

https://codereview.chromium.org/1025893002/diff/80001/LayoutTests/fast/compos...
LayoutTests/fast/compositorworker/compositor-attribute-change-worker.html:2:
<div id='container'></div>
On 2015/03/29 04:44:56, esprehn wrote:
> double quotes? same with the other test, it's nice to be consistent.

Done.

https://codereview.chromium.org/1025893002/diff/80001/Source/core/dom/Composi...
File Source/core/dom/CompositorProxy.cpp (right):

https://codereview.chromium.org/1025893002/diff/80001/Source/core/dom/Composi...
Source/core/dom/CompositorProxy.cpp:57: // TODO(sad):
On 2015/03/29 04:44:56, esprehn wrote:
> TODO what?

Oops. My bad. Done.

https://codereview.chromium.org/1025893002/diff/80001/Source/core/dom/Composi...
Source/core/dom/CompositorProxy.cpp:180: m_opacity = opacity;
On 2015/03/29 04:44:56, esprehn wrote:
> opacity must be positive, you probably want to just ignore negative values
here
> (or throw?)

Looks like setting style.opacity on an element in the main-thread clamps the
value between [0, 1]. So I have done the same here.

https://codereview.chromium.org/1025893002/diff/80001/Source/core/dom/Composi...
File Source/core/dom/CompositorProxy.h (right):

https://codereview.chromium.org/1025893002/diff/80001/Source/core/dom/Composi...
Source/core/dom/CompositorProxy.h:67: uint32_t m_mutatedAttributes;
On 2015/03/29 04:44:56, esprehn wrote:
> Why not set all of these to = 0 also?

Good point. Done.

https://codereview.chromium.org/1025893002/diff/80001/Source/core/dom/Composi...
Source/core/dom/CompositorProxy.h:69: double m_opacity = 0;
On 2015/03/29 04:44:56, esprehn wrote:
> Is this allowed in blink C++11 style? :)

Oh, I hadn't considered the blink style guide. I took a look, and looks like
c++11 use in blink is expected to be the same as in chromium
(https://www.chromium.org/blink/coding-style#TOC-C--11-Features), and
"Non-Static Class Member Initializers" is allowed there
(https://chromium-cpp.appspot.com/).

[sadrul, 2015-03-30 16:07:56]

The CQ bit was checked by sadrul@chromium.org

[sadrul, 2015-03-30 16:07:57]

The patchset sent to the CQ was uploaded after l-g-t-m from esprehn@chromium.org
Link to the patchset: https://codereview.chromium.org/1025893002/#ps100001
(title: ".")

[commit-bot: I haz the power, 2015-03-30 16:08:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1025893002/100001

[commit-bot: I haz the power, 2015-03-30 17:12:32]

Committed patchset #6 (id:100001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=192768