compositor-worker: Introduce WebCompositorMutableState

compositor-worker: Introduce WebCompositorMutableState

This change introduces a wrapper around compositer-owned state that may
be mutated by a compositor worker.

This depends on crrev.com/1405993008

BUG=430155
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/9c7f6d0e46a46605801d143c04b86f5e4445cd37
Cr-Commit-Position: refs/heads/master@{#368273}

[Ian Vollick, 2015-11-15 03:33:19]

Description was changed from

==========
compositor-worker: Introduce WebMutableState

This change introduces a wrapper around compositer-owned state that may
be mutated by a compositor worker.

This depends on crrev.com/1405993008

BUG=430155
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
compositor-worker: Introduce WebMutableState

This change introduces a wrapper around compositer-owned state that may
be mutated by a compositor worker.

This depends on crrev.com/1405993008

BUG=430155
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[Ian Vollick, 2015-11-15 03:33:19]

vollick@chromium.org changed reviewers:
+ enne@chromium.org, esprehn@chromium.org

[Ian Vollick, 2015-11-15 03:34:27]

This is the follow-on patch to the initial element id plumbing:
https://codereview.chromium.org/1405993008/

[enne (OOO), 2015-11-16 19:12:33]

This looks mostly like wrapping/plumbing without anything real being used? Do
you think any unit tests would be useful at this point? Or better to wait until
there's something more substantial?

https://codereview.chromium.org/1447893002/diff/1/cc/blink/web_mutable_state_...
File cc/blink/web_mutable_state_impl.h (right):

https://codereview.chromium.org/1447893002/diff/1/cc/blink/web_mutable_state_...
cc/blink/web_mutable_state_impl.h:48: class CC_BLINK_EXPORT
WebMutableStateProviderImpl
Different classes, different files?

https://codereview.chromium.org/1447893002/diff/1/cc/blink/web_mutable_state_...
cc/blink/web_mutable_state_impl.h:52: // property trees, no the layer tree impl.
*not

https://codereview.chromium.org/1447893002/diff/1/cc/blink/web_mutable_state_...
cc/blink/web_mutable_state_impl.h:53:
WebMutableStateProviderImpl(cc::LayerTreeImpl* state,
Who owns these pointers?

https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.cc
File cc/trees/layer_tree_impl.cc (right):

https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.cc...
cc/trees/layer_tree_impl.cc:48: const uint32_t kMainLayerFlags =
What does "main" here mean?

https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.cc...
cc/trees/layer_tree_impl.cc:52: const uint32_t kMainLayerIndex = 0;
Do you expect to have more layer types? It looks a bit odd to have constant
indices when n=2.

https://codereview.chromium.org/1447893002/diff/1/third_party/WebKit/public/p...
File third_party/WebKit/public/platform/WebMutableState.h (right):

https://codereview.chromium.org/1447893002/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/WebMutableState.h:21: virtual
WebMutableState* getMutableStateFor(uint64_t elementId) = 0;
Raw pointers in APIs.  T_T

unique_ptr support should be here any day now, so this can get cleaned up after
that.

[danakj, 2015-11-16 19:31:18]

https://codereview.chromium.org/1447893002/diff/1/third_party/WebKit/public/p...
File third_party/WebKit/public/platform/WebMutableState.h (right):

https://codereview.chromium.org/1447893002/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/WebMutableState.h:21: virtual
WebMutableState* getMutableStateFor(uint64_t elementId) = 0;
On 2015/11/16 19:12:33, enne wrote:
> Raw pointers in APIs.  T_T
> 
> unique_ptr support should be here any day now, so this can get cleaned up
after
> that.

This is platform, can it just use scoped_ptr?

[esprehn, 2015-11-16 20:01:04]

https://codereview.chromium.org/1447893002/diff/1/cc/animation/layer_tree_mut...
File cc/animation/layer_tree_mutation.h (right):

https://codereview.chromium.org/1447893002/diff/1/cc/animation/layer_tree_mut...
cc/animation/layer_tree_mutation.h:60: typedef std::map<uint64_t,
LayerTreeMutation> LayerTreeMutationMap;
why not hash map? Is the ordering actually important?

https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.cc
File cc/trees/layer_tree_impl.cc (right):

https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.cc...
cc/trees/layer_tree_impl.cc:67: LayerImpl* layers[2];
just add two members, why use an array? Also this thing seems like it just wants
public fields, no reason to have getters at all

https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.cc...
cc/trees/layer_tree_impl.cc:388: layers.main() = layer;
this is very weird, why not just make the fields public or have a setter?

https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.cc...
cc/trees/layer_tree_impl.cc:421: *scroll_layer = iter->second.scroll();
just return the ElementLayers object by value, it's just two pointers. No need
for the out params. Callers can null check the ElementLayers object

https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.h
File cc/trees/layer_tree_impl.h (right):

https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.h#...
cc/trees/layer_tree_impl.h:460: std::map<uint64_t, ElementLayers>
element_layers_map_;
does the order matter here? Why map instead of hash map?

https://codereview.chromium.org/1447893002/diff/1/third_party/WebKit/public/p...
File third_party/WebKit/public/platform/WebMutableState.h (right):

https://codereview.chromium.org/1447893002/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/WebMutableState.h:16: class
WebMutableStateProvider {
this needs a less generic name, "mutable state" is not something specific enough
to compositor, you need WebElementMutableState I guess?

I have to admit the "mutable" part here doesn't really make sense either, why
use this term?

Also please add a comment explaining what this class does.

https://codereview.chromium.org/1447893002/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/WebMutableState.h:24: class WebMutableState {
ditto, needs a better name, also a comment.

https://codereview.chromium.org/1447893002/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/WebMutableState.h:28: virtual double
getOpacity() const = 0;
opacity(), no get* for a getter.

https://codereview.chromium.org/1447893002/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/WebMutableState.h:31: virtual const
SkMatrix44& getTransform() const = 0;
ditto for all of these

[Ian Vollick, 2015-11-18 17:20:34]

Thanks for the reviews! I've hopefully addressed your comments.

(btw, if you're tempted to do another round of reviews on this cl, could you
please take a look at the CL this change depends on first? rrev.com/1405993008)

https://codereview.chromium.org/1447893002/diff/1/cc/animation/layer_tree_mut...
File cc/animation/layer_tree_mutation.h (right):

https://codereview.chromium.org/1447893002/diff/1/cc/animation/layer_tree_mut...
cc/animation/layer_tree_mutation.h:60: typedef std::map<uint64_t,
LayerTreeMutation> LayerTreeMutationMap;
On 2015/11/16 20:01:03, esprehn wrote:
> why not hash map? Is the ordering actually important?

Done.

https://codereview.chromium.org/1447893002/diff/1/cc/blink/web_mutable_state_...
File cc/blink/web_mutable_state_impl.h (right):

https://codereview.chromium.org/1447893002/diff/1/cc/blink/web_mutable_state_...
cc/blink/web_mutable_state_impl.h:48: class CC_BLINK_EXPORT
WebMutableStateProviderImpl
On 2015/11/16 19:12:33, enne wrote:
> Different classes, different files?

Done.

https://codereview.chromium.org/1447893002/diff/1/cc/blink/web_mutable_state_...
cc/blink/web_mutable_state_impl.h:52: // property trees, no the layer tree impl.
On 2015/11/16 19:12:33, enne wrote:
> *not

Done.

https://codereview.chromium.org/1447893002/diff/1/cc/blink/web_mutable_state_...
cc/blink/web_mutable_state_impl.h:53:
WebMutableStateProviderImpl(cc::LayerTreeImpl* state,
On 2015/11/16 19:12:33, enne wrote:
> Who owns these pointers?

Caller owns them. Added a comment.

https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.cc
File cc/trees/layer_tree_impl.cc (right):

https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.cc...
cc/trees/layer_tree_impl.cc:48: const uint32_t kMainLayerFlags =
On 2015/11/16 19:12:33, enne wrote:
> What does "main" here mean?

Added comments in the header. It's analogous to m_graphicsLayer in CLM. I
couldn't think of a better name -- if you have a suggestion I would gladly take
it :)

https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.cc...
cc/trees/layer_tree_impl.cc:52: const uint32_t kMainLayerIndex = 0;
On 2015/11/16 19:12:33, enne wrote:
> Do you expect to have more layer types? It looks a bit odd to have constant
> indices when n=2.

It is odd. The whole ElementLayers class was wonky and I don't know what
possessed me to write it the way I did. I've switched it to a POD struct with
two named pointers.

https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.cc...
cc/trees/layer_tree_impl.cc:67: LayerImpl* layers[2];
On 2015/11/16 20:01:04, esprehn wrote:
> just add two members, why use an array? Also this thing seems like it just
wants
> public fields, no reason to have getters at all

Agreed. Updated.

https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.cc...
cc/trees/layer_tree_impl.cc:388: layers.main() = layer;
On 2015/11/16 20:01:04, esprehn wrote:
> this is very weird, why not just make the fields public or have a setter?

Done.

https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.cc...
cc/trees/layer_tree_impl.cc:421: *scroll_layer = iter->second.scroll();
On 2015/11/16 20:01:04, esprehn wrote:
> just return the ElementLayers object by value, it's just two pointers. No need
> for the out params. Callers can null check the ElementLayers object

Done.

https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.h
File cc/trees/layer_tree_impl.h (right):

https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.h#...
cc/trees/layer_tree_impl.h:460: std::map<uint64_t, ElementLayers>
element_layers_map_;
On 2015/11/16 20:01:04, esprehn wrote:
> does the order matter here? Why map instead of hash map?

Done.

https://codereview.chromium.org/1447893002/diff/1/third_party/WebKit/public/p...
File third_party/WebKit/public/platform/WebMutableState.h (right):

https://codereview.chromium.org/1447893002/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/WebMutableState.h:16: class
WebMutableStateProvider {
On 2015/11/16 20:01:04, esprehn wrote:
> this needs a less generic name, "mutable state" is not something specific
enough
> to compositor, you need WebElementMutableState I guess?
> 
> I have to admit the "mutable" part here doesn't really make sense either, why
> use this term?
> 
> Also please add a comment explaining what this class does.

Done.

https://codereview.chromium.org/1447893002/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/WebMutableState.h:21: virtual
WebMutableState* getMutableStateFor(uint64_t elementId) = 0;
On 2015/11/16 19:31:18, danakj (behind on reviews) wrote:
> On 2015/11/16 19:12:33, enne wrote:
> > Raw pointers in APIs.  T_T
> > 
> > unique_ptr support should be here any day now, so this can get cleaned up
> after
> > that.
> 
> This is platform, can it just use scoped_ptr?

This pointer ultimately ends up being owned by core/ (the CompositorProxy
instances hang onto them).

https://codereview.chromium.org/1447893002/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/WebMutableState.h:24: class WebMutableState {
On 2015/11/16 20:01:04, esprehn wrote:
> ditto, needs a better name, also a comment.

Done.

https://codereview.chromium.org/1447893002/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/WebMutableState.h:28: virtual double
getOpacity() const = 0;
On 2015/11/16 20:01:04, esprehn wrote:
> opacity(), no get* for a getter.

Done.

https://codereview.chromium.org/1447893002/diff/1/third_party/WebKit/public/p...
third_party/WebKit/public/platform/WebMutableState.h:31: virtual const
SkMatrix44& getTransform() const = 0;
On 2015/11/16 20:01:04, esprehn wrote:
> ditto for all of these

Done.

[Ian Vollick, 2015-12-09 18:19:26]

On 2015/11/18 17:20:34, vollick wrote:
> Thanks for the reviews! I've hopefully addressed your comments.
> 
> (btw, if you're tempted to do another round of reviews on this cl, could you
> please take a look at the CL this change depends on first?
rrev.com/1405993008)
> 
>
https://codereview.chromium.org/1447893002/diff/1/cc/animation/layer_tree_mut...
> File cc/animation/layer_tree_mutation.h (right):
> 
>
https://codereview.chromium.org/1447893002/diff/1/cc/animation/layer_tree_mut...
> cc/animation/layer_tree_mutation.h:60: typedef std::map<uint64_t,
> LayerTreeMutation> LayerTreeMutationMap;
> On 2015/11/16 20:01:03, esprehn wrote:
> > why not hash map? Is the ordering actually important?
> 
> Done.
> 
>
https://codereview.chromium.org/1447893002/diff/1/cc/blink/web_mutable_state_...
> File cc/blink/web_mutable_state_impl.h (right):
> 
>
https://codereview.chromium.org/1447893002/diff/1/cc/blink/web_mutable_state_...
> cc/blink/web_mutable_state_impl.h:48: class CC_BLINK_EXPORT
> WebMutableStateProviderImpl
> On 2015/11/16 19:12:33, enne wrote:
> > Different classes, different files?
> 
> Done.
> 
>
https://codereview.chromium.org/1447893002/diff/1/cc/blink/web_mutable_state_...
> cc/blink/web_mutable_state_impl.h:52: // property trees, no the layer tree
impl.
> On 2015/11/16 19:12:33, enne wrote:
> > *not
> 
> Done.
> 
>
https://codereview.chromium.org/1447893002/diff/1/cc/blink/web_mutable_state_...
> cc/blink/web_mutable_state_impl.h:53:
> WebMutableStateProviderImpl(cc::LayerTreeImpl* state,
> On 2015/11/16 19:12:33, enne wrote:
> > Who owns these pointers?
> 
> Caller owns them. Added a comment.
> 
> https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.cc
> File cc/trees/layer_tree_impl.cc (right):
> 
>
https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.cc...
> cc/trees/layer_tree_impl.cc:48: const uint32_t kMainLayerFlags =
> On 2015/11/16 19:12:33, enne wrote:
> > What does "main" here mean?
> 
> Added comments in the header. It's analogous to m_graphicsLayer in CLM. I
> couldn't think of a better name -- if you have a suggestion I would gladly
take
> it :)
> 
>
https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.cc...
> cc/trees/layer_tree_impl.cc:52: const uint32_t kMainLayerIndex = 0;
> On 2015/11/16 19:12:33, enne wrote:
> > Do you expect to have more layer types? It looks a bit odd to have constant
> > indices when n=2.
> 
> It is odd. The whole ElementLayers class was wonky and I don't know what
> possessed me to write it the way I did. I've switched it to a POD struct with
> two named pointers.
> 
>
https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.cc...
> cc/trees/layer_tree_impl.cc:67: LayerImpl* layers[2];
> On 2015/11/16 20:01:04, esprehn wrote:
> > just add two members, why use an array? Also this thing seems like it just
> wants
> > public fields, no reason to have getters at all
> 
> Agreed. Updated.
> 
>
https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.cc...
> cc/trees/layer_tree_impl.cc:388: layers.main() = layer;
> On 2015/11/16 20:01:04, esprehn wrote:
> > this is very weird, why not just make the fields public or have a setter?
> 
> Done.
> 
>
https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.cc...
> cc/trees/layer_tree_impl.cc:421: *scroll_layer = iter->second.scroll();
> On 2015/11/16 20:01:04, esprehn wrote:
> > just return the ElementLayers object by value, it's just two pointers. No
need
> > for the out params. Callers can null check the ElementLayers object
> 
> Done.
> 
> https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.h
> File cc/trees/layer_tree_impl.h (right):
> 
>
https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.h#...
> cc/trees/layer_tree_impl.h:460: std::map<uint64_t, ElementLayers>
> element_layers_map_;
> On 2015/11/16 20:01:04, esprehn wrote:
> > does the order matter here? Why map instead of hash map?
> 
> Done.
> 
>
https://codereview.chromium.org/1447893002/diff/1/third_party/WebKit/public/p...
> File third_party/WebKit/public/platform/WebMutableState.h (right):
> 
>
https://codereview.chromium.org/1447893002/diff/1/third_party/WebKit/public/p...
> third_party/WebKit/public/platform/WebMutableState.h:16: class
> WebMutableStateProvider {
> On 2015/11/16 20:01:04, esprehn wrote:
> > this needs a less generic name, "mutable state" is not something specific
> enough
> > to compositor, you need WebElementMutableState I guess?
> > 
> > I have to admit the "mutable" part here doesn't really make sense either,
why
> > use this term?
> > 
> > Also please add a comment explaining what this class does.
> 
> Done.
> 
>
https://codereview.chromium.org/1447893002/diff/1/third_party/WebKit/public/p...
> third_party/WebKit/public/platform/WebMutableState.h:21: virtual
> WebMutableState* getMutableStateFor(uint64_t elementId) = 0;
> On 2015/11/16 19:31:18, danakj (behind on reviews) wrote:
> > On 2015/11/16 19:12:33, enne wrote:
> > > Raw pointers in APIs.  T_T
> > > 
> > > unique_ptr support should be here any day now, so this can get cleaned up
> > after
> > > that.
> > 
> > This is platform, can it just use scoped_ptr?
> 
> This pointer ultimately ends up being owned by core/ (the CompositorProxy
> instances hang onto them).
> 
>
https://codereview.chromium.org/1447893002/diff/1/third_party/WebKit/public/p...
> third_party/WebKit/public/platform/WebMutableState.h:24: class WebMutableState
{
> On 2015/11/16 20:01:04, esprehn wrote:
> > ditto, needs a better name, also a comment.
> 
> Done.
> 
>
https://codereview.chromium.org/1447893002/diff/1/third_party/WebKit/public/p...
> third_party/WebKit/public/platform/WebMutableState.h:28: virtual double
> getOpacity() const = 0;
> On 2015/11/16 20:01:04, esprehn wrote:
> > opacity(), no get* for a getter.
> 
> Done.
> 
>
https://codereview.chromium.org/1447893002/diff/1/third_party/WebKit/public/p...
> third_party/WebKit/public/platform/WebMutableState.h:31: virtual const
> SkMatrix44& getTransform() const = 0;
> On 2015/11/16 20:01:04, esprehn wrote:
> > ditto for all of these
> 
> Done.

Now that has landed, I've updated this patch and it should be ready for review.
PTAL!

Enne, it occurred to me that I missed one of your previous review comments:

> This looks mostly like wrapping/plumbing without anything real being used? Do
> you think any unit tests would be useful at this point? Or better to wait
until
> there's something more substantial?

I'd forgotten to git add the unittest. :)  The code should be exercised now.

[Ian Vollick, 2015-12-12 04:28:10]

On 2015/12/09 18:19:26, vollick wrote:
> On 2015/11/18 17:20:34, vollick wrote:
> > Thanks for the reviews! I've hopefully addressed your comments.
> > 
> > (btw, if you're tempted to do another round of reviews on this cl, could you
> > please take a look at the CL this change depends on first?
> rrev.com/1405993008)
> > 
> >
>
https://codereview.chromium.org/1447893002/diff/1/cc/animation/layer_tree_mut...
> > File cc/animation/layer_tree_mutation.h (right):
> > 
> >
>
https://codereview.chromium.org/1447893002/diff/1/cc/animation/layer_tree_mut...
> > cc/animation/layer_tree_mutation.h:60: typedef std::map<uint64_t,
> > LayerTreeMutation> LayerTreeMutationMap;
> > On 2015/11/16 20:01:03, esprehn wrote:
> > > why not hash map? Is the ordering actually important?
> > 
> > Done.
> > 
> >
>
https://codereview.chromium.org/1447893002/diff/1/cc/blink/web_mutable_state_...
> > File cc/blink/web_mutable_state_impl.h (right):
> > 
> >
>
https://codereview.chromium.org/1447893002/diff/1/cc/blink/web_mutable_state_...
> > cc/blink/web_mutable_state_impl.h:48: class CC_BLINK_EXPORT
> > WebMutableStateProviderImpl
> > On 2015/11/16 19:12:33, enne wrote:
> > > Different classes, different files?
> > 
> > Done.
> > 
> >
>
https://codereview.chromium.org/1447893002/diff/1/cc/blink/web_mutable_state_...
> > cc/blink/web_mutable_state_impl.h:52: // property trees, no the layer tree
> impl.
> > On 2015/11/16 19:12:33, enne wrote:
> > > *not
> > 
> > Done.
> > 
> >
>
https://codereview.chromium.org/1447893002/diff/1/cc/blink/web_mutable_state_...
> > cc/blink/web_mutable_state_impl.h:53:
> > WebMutableStateProviderImpl(cc::LayerTreeImpl* state,
> > On 2015/11/16 19:12:33, enne wrote:
> > > Who owns these pointers?
> > 
> > Caller owns them. Added a comment.
> > 
> >
https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.cc
> > File cc/trees/layer_tree_impl.cc (right):
> > 
> >
>
https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.cc...
> > cc/trees/layer_tree_impl.cc:48: const uint32_t kMainLayerFlags =
> > On 2015/11/16 19:12:33, enne wrote:
> > > What does "main" here mean?
> > 
> > Added comments in the header. It's analogous to m_graphicsLayer in CLM. I
> > couldn't think of a better name -- if you have a suggestion I would gladly
> take
> > it :)
> > 
> >
>
https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.cc...
> > cc/trees/layer_tree_impl.cc:52: const uint32_t kMainLayerIndex = 0;
> > On 2015/11/16 19:12:33, enne wrote:
> > > Do you expect to have more layer types? It looks a bit odd to have
constant
> > > indices when n=2.
> > 
> > It is odd. The whole ElementLayers class was wonky and I don't know what
> > possessed me to write it the way I did. I've switched it to a POD struct
with
> > two named pointers.
> > 
> >
>
https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.cc...
> > cc/trees/layer_tree_impl.cc:67: LayerImpl* layers[2];
> > On 2015/11/16 20:01:04, esprehn wrote:
> > > just add two members, why use an array? Also this thing seems like it just
> > wants
> > > public fields, no reason to have getters at all
> > 
> > Agreed. Updated.
> > 
> >
>
https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.cc...
> > cc/trees/layer_tree_impl.cc:388: layers.main() = layer;
> > On 2015/11/16 20:01:04, esprehn wrote:
> > > this is very weird, why not just make the fields public or have a setter?
> > 
> > Done.
> > 
> >
>
https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.cc...
> > cc/trees/layer_tree_impl.cc:421: *scroll_layer = iter->second.scroll();
> > On 2015/11/16 20:01:04, esprehn wrote:
> > > just return the ElementLayers object by value, it's just two pointers. No
> need
> > > for the out params. Callers can null check the ElementLayers object
> > 
> > Done.
> > 
> > https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.h
> > File cc/trees/layer_tree_impl.h (right):
> > 
> >
>
https://codereview.chromium.org/1447893002/diff/1/cc/trees/layer_tree_impl.h#...
> > cc/trees/layer_tree_impl.h:460: std::map<uint64_t, ElementLayers>
> > element_layers_map_;
> > On 2015/11/16 20:01:04, esprehn wrote:
> > > does the order matter here? Why map instead of hash map?
> > 
> > Done.
> > 
> >
>
https://codereview.chromium.org/1447893002/diff/1/third_party/WebKit/public/p...
> > File third_party/WebKit/public/platform/WebMutableState.h (right):
> > 
> >
>
https://codereview.chromium.org/1447893002/diff/1/third_party/WebKit/public/p...
> > third_party/WebKit/public/platform/WebMutableState.h:16: class
> > WebMutableStateProvider {
> > On 2015/11/16 20:01:04, esprehn wrote:
> > > this needs a less generic name, "mutable state" is not something specific
> > enough
> > > to compositor, you need WebElementMutableState I guess?
> > > 
> > > I have to admit the "mutable" part here doesn't really make sense either,
> why
> > > use this term?
> > > 
> > > Also please add a comment explaining what this class does.
> > 
> > Done.
> > 
> >
>
https://codereview.chromium.org/1447893002/diff/1/third_party/WebKit/public/p...
> > third_party/WebKit/public/platform/WebMutableState.h:21: virtual
> > WebMutableState* getMutableStateFor(uint64_t elementId) = 0;
> > On 2015/11/16 19:31:18, danakj (behind on reviews) wrote:
> > > On 2015/11/16 19:12:33, enne wrote:
> > > > Raw pointers in APIs.  T_T
> > > > 
> > > > unique_ptr support should be here any day now, so this can get cleaned
up
> > > after
> > > > that.
> > > 
> > > This is platform, can it just use scoped_ptr?
> > 
> > This pointer ultimately ends up being owned by core/ (the CompositorProxy
> > instances hang onto them).
> > 
> >
>
https://codereview.chromium.org/1447893002/diff/1/third_party/WebKit/public/p...
> > third_party/WebKit/public/platform/WebMutableState.h:24: class
WebMutableState
> {
> > On 2015/11/16 20:01:04, esprehn wrote:
> > > ditto, needs a better name, also a comment.
> > 
> > Done.
> > 
> >
>
https://codereview.chromium.org/1447893002/diff/1/third_party/WebKit/public/p...
> > third_party/WebKit/public/platform/WebMutableState.h:28: virtual double
> > getOpacity() const = 0;
> > On 2015/11/16 20:01:04, esprehn wrote:
> > > opacity(), no get* for a getter.
> > 
> > Done.
> > 
> >
>
https://codereview.chromium.org/1447893002/diff/1/third_party/WebKit/public/p...
> > third_party/WebKit/public/platform/WebMutableState.h:31: virtual const
> > SkMatrix44& getTransform() const = 0;
> > On 2015/11/16 20:01:04, esprehn wrote:
> > > ditto for all of these
> > 
> > Done.
> 
> Now that has landed, I've updated this patch and it should be ready for
review.
> PTAL!
> 
> Enne, it occurred to me that I missed one of your previous review comments:
> 
> > This looks mostly like wrapping/plumbing without anything real being used?
Do
> > you think any unit tests would be useful at this point? Or better to wait
> until
> > there's something more substantial?
> 
> I'd forgotten to git add the unittest. :)  The code should be exercised now.

Ping :)

[Ian Vollick, 2015-12-14 18:13:29]

vollick@chromium.org changed reviewers:
+ ajuma@chromium.org

[ajuma, 2015-12-14 19:41:47]

https://codereview.chromium.org/1447893002/diff/220001/cc/blink/web_composito...
File cc/blink/web_compositor_mutable_state_impl.cc (right):

https://codereview.chromium.org/1447893002/diff/220001/cc/blink/web_composito...
cc/blink/web_compositor_mutable_state_impl.cc:42:
main_layer_->OnTransformAnimated(gfx::Transform(matrix));
Are property tree nodes currently created for mutable layers, or is that in a
future CL?

https://codereview.chromium.org/1447893002/diff/220001/cc/trees/layer_tree_im...
File cc/trees/layer_tree_impl.cc (right):

https://codereview.chromium.org/1447893002/diff/220001/cc/trees/layer_tree_im...
cc/trees/layer_tree_impl.cc:55: struct ElementLayers {
There's also an ElementLayers defined inside LayerTreeImpl in layer_tree_impl.h.
Is the one defined here actually needed?

[Ian Vollick, 2015-12-14 20:22:07]

https://codereview.chromium.org/1447893002/diff/220001/cc/blink/web_composito...
File cc/blink/web_compositor_mutable_state_impl.cc (right):

https://codereview.chromium.org/1447893002/diff/220001/cc/blink/web_composito...
cc/blink/web_compositor_mutable_state_impl.cc:42:
main_layer_->OnTransformAnimated(gfx::Transform(matrix));
On 2015/12/14 19:41:47, ajuma wrote:
> Are property tree nodes currently created for mutable layers, or is that in a
> future CL?

This is certainly necessary, but it is in a future CL.

https://codereview.chromium.org/1447893002/diff/220001/cc/trees/layer_tree_im...
File cc/trees/layer_tree_impl.cc (right):

https://codereview.chromium.org/1447893002/diff/220001/cc/trees/layer_tree_im...
cc/trees/layer_tree_impl.cc:55: struct ElementLayers {
On 2015/12/14 19:41:47, ajuma wrote:
> There's also an ElementLayers defined inside LayerTreeImpl in
layer_tree_impl.h.
> Is the one defined here actually needed?

No, not needed - forgot to remove this code when I moved the defn to the header.

[ajuma, 2015-12-14 20:24:08]

On 2015/12/14 20:22:07, vollick wrote:
>
https://codereview.chromium.org/1447893002/diff/220001/cc/blink/web_composito...
> File cc/blink/web_compositor_mutable_state_impl.cc (right):
> 
>
https://codereview.chromium.org/1447893002/diff/220001/cc/blink/web_composito...
> cc/blink/web_compositor_mutable_state_impl.cc:42:
> main_layer_->OnTransformAnimated(gfx::Transform(matrix));
> On 2015/12/14 19:41:47, ajuma wrote:
> > Are property tree nodes currently created for mutable layers, or is that in
a
> > future CL?
> 
> This is certainly necessary, but it is in a future CL.
> 
>
https://codereview.chromium.org/1447893002/diff/220001/cc/trees/layer_tree_im...
> File cc/trees/layer_tree_impl.cc (right):
> 
>
https://codereview.chromium.org/1447893002/diff/220001/cc/trees/layer_tree_im...
> cc/trees/layer_tree_impl.cc:55: struct ElementLayers {
> On 2015/12/14 19:41:47, ajuma wrote:
> > There's also an ElementLayers defined inside LayerTreeImpl in
> layer_tree_impl.h.
> > Is the one defined here actually needed?
> 
> No, not needed - forgot to remove this code when I moved the defn to the
header.

Thanks! lgtm.

[esprehn, 2015-12-16 04:01:59]

There's no tests inside blink, and no usage of the API, this patch should
contain some usage of the new API. There's no reason to do half sided patches
with the repo merge. It makes it hard to gauge if this API is designed
correctly.

https://codereview.chromium.org/1447893002/diff/240001/cc/trees/layer_tree_im...
File cc/trees/layer_tree_impl.cc (right):

https://codereview.chromium.org/1447893002/diff/240001/cc/trees/layer_tree_im...
cc/trees/layer_tree_impl.cc:370: ElementLayers& layers =
element_layers_map_[layer->element_id()];
this puts a new hash lookup for every composited layer, and now every layer is
replicated into this map, so this map is the size of the layer tree, is that
really necessary?

https://codereview.chromium.org/1447893002/diff/240001/cc/trees/layer_tree_im...
cc/trees/layer_tree_impl.cc:388: ElementLayers& layers =
element_layers_map_[layer->element_id()];
and another map lookup for every layer removal

https://codereview.chromium.org/1447893002/diff/240001/third_party/WebKit/pub...
File third_party/WebKit/public/platform/WebCompositorMutableStateProvider.h
(right):

https://codereview.chromium.org/1447893002/diff/240001/third_party/WebKit/pub...
third_party/WebKit/public/platform/WebCompositorMutableStateProvider.h:16: class
WebCompositorMutableStateProvider {
This is dead code right now, where is it going to get used?

https://codereview.chromium.org/1447893002/diff/240001/third_party/WebKit/pub...
third_party/WebKit/public/platform/WebCompositorMutableStateProvider.h:21:
virtual WebCompositorMutableState* getMutableStateFor(uint64_t elementId) = 0;
return a WebPassOwnPtr, how does the caller know when to call this?

[Ian Vollick, 2015-12-23 17:34:40]

> There's no tests inside blink, and no usage of the API, this patch should
> contain some usage of the new API. There's no reason to do half sided patches
> with the repo merge. It makes it hard to gauge if this API is designed
> correctly.

I've updated the mega-patch to show this work in context and so you can judge
the API (crrev.com/1547893003).

Roughly, my upstreaming plan is as follows
 1. Set up an association between blink and cc data [landed].
 2. Refactor the worker code to actually use the compositor thread [landed].
 3. Wrap up the compositor state that blink will talk to (this patch).
 4. Connect blink to this state (see CompositorProxyClient*, WebMutator*).
 5. Teach the compositor to issue callbacks.
 6. Plumb back changes to blink.

Although the unit tests in this patch do test the new APIs, to properly
demonstrate their usage from within blink I think we'd need both 4 and 5 since
the compositor state is summoned as part of the callback execution flow, and
that seemed like a very big change to review all at once.

https://codereview.chromium.org/1447893002/diff/240001/cc/trees/layer_tree_im...
File cc/trees/layer_tree_impl.cc (right):

https://codereview.chromium.org/1447893002/diff/240001/cc/trees/layer_tree_im...
cc/trees/layer_tree_impl.cc:370: ElementLayers& layers =
element_layers_map_[layer->element_id()];
On 2015/12/16 04:01:58, esprehn wrote:
> this puts a new hash lookup for every composited layer, and now every layer is
> replicated into this map, so this map is the size of the layer tree, is that
> really necessary?

A non-zero element id is actually pretty rare. Before we called this function,
we check that we have a non-zero id, but that wasn't obvious. I've moved these
checks into this function and RemoveFromElementMap, both to ensure that we don't
forget and also for documentation.

https://codereview.chromium.org/1447893002/diff/240001/cc/trees/layer_tree_im...
cc/trees/layer_tree_impl.cc:388: ElementLayers& layers =
element_layers_map_[layer->element_id()];
On 2015/12/16 04:01:58, esprehn wrote:
> and another map lookup for every layer removal

Ditto.

https://codereview.chromium.org/1447893002/diff/240001/third_party/WebKit/pub...
File third_party/WebKit/public/platform/WebCompositorMutableStateProvider.h
(right):

https://codereview.chromium.org/1447893002/diff/240001/third_party/WebKit/pub...
third_party/WebKit/public/platform/WebCompositorMutableStateProvider.h:16: class
WebCompositorMutableStateProvider {
On 2015/12/16 04:01:59, esprehn wrote:
> This is dead code right now, where is it going to get used?

At a high level, when we execute the compositor-worker callbacks we send out to
blink a fresh wrapper around the compositor state. The CompositorProxies are
connected to the appropriate state before the callbacks are executed
(https://codereview.chromium.org/1547893003/diff/1/third_party/WebKit/Source/w...).

https://codereview.chromium.org/1447893002/diff/240001/third_party/WebKit/pub...
third_party/WebKit/public/platform/WebCompositorMutableStateProvider.h:21:
virtual WebCompositorMutableState* getMutableStateFor(uint64_t elementId) = 0;
On 2015/12/16 04:01:58, esprehn wrote:
> return a WebPassOwnPtr, how does the caller know when to call this?

Done.

In the current mega-patch (linked above), we grab a mutable state wrapper for
the proxies before executing the compositor worker callbacks. We have a lot of
flexibility about how we connect the proxies to the state, though. Some other
alternatives I'd considered were
 * not wrapping the per-element mutable state.
   * this has a big downside: if we don't cache the per-element state, every
property read and write is a hash map lookup.
 * lazily creating the per-element wrappers for the compositor proxies
   * This may actually be a good way to go, but it's orthogonal to this patch,
so I'd prefer to leave that discussion to the subsequent patch that connects the
CompositorProxies to the compositor's mutable state.

[Ian Vollick, 2016-01-05 15:38:16]

On 2015/12/23 17:34:40, vollick wrote:
> > There's no tests inside blink, and no usage of the API, this patch should
> > contain some usage of the new API. There's no reason to do half sided
patches
> > with the repo merge. It makes it hard to gauge if this API is designed
> > correctly.
> 
> I've updated the mega-patch to show this work in context and so you can judge
> the API (crrev.com/1547893003).
> 
> Roughly, my upstreaming plan is as follows
>  1. Set up an association between blink and cc data [landed].
>  2. Refactor the worker code to actually use the compositor thread [landed].
>  3. Wrap up the compositor state that blink will talk to (this patch).
>  4. Connect blink to this state (see CompositorProxyClient*, WebMutator*).
>  5. Teach the compositor to issue callbacks.
>  6. Plumb back changes to blink.
> 
> Although the unit tests in this patch do test the new APIs, to properly
> demonstrate their usage from within blink I think we'd need both 4 and 5 since
> the compositor state is summoned as part of the callback execution flow, and
> that seemed like a very big change to review all at once.
> 
>
https://codereview.chromium.org/1447893002/diff/240001/cc/trees/layer_tree_im...
> File cc/trees/layer_tree_impl.cc (right):
> 
>
https://codereview.chromium.org/1447893002/diff/240001/cc/trees/layer_tree_im...
> cc/trees/layer_tree_impl.cc:370: ElementLayers& layers =
> element_layers_map_[layer->element_id()];
> On 2015/12/16 04:01:58, esprehn wrote:
> > this puts a new hash lookup for every composited layer, and now every layer
is
> > replicated into this map, so this map is the size of the layer tree, is that
> > really necessary?
> 
> A non-zero element id is actually pretty rare. Before we called this function,
> we check that we have a non-zero id, but that wasn't obvious. I've moved these
> checks into this function and RemoveFromElementMap, both to ensure that we
don't
> forget and also for documentation.
> 
>
https://codereview.chromium.org/1447893002/diff/240001/cc/trees/layer_tree_im...
> cc/trees/layer_tree_impl.cc:388: ElementLayers& layers =
> element_layers_map_[layer->element_id()];
> On 2015/12/16 04:01:58, esprehn wrote:
> > and another map lookup for every layer removal
> 
> Ditto.
> 
>
https://codereview.chromium.org/1447893002/diff/240001/third_party/WebKit/pub...
> File third_party/WebKit/public/platform/WebCompositorMutableStateProvider.h
> (right):
> 
>
https://codereview.chromium.org/1447893002/diff/240001/third_party/WebKit/pub...
> third_party/WebKit/public/platform/WebCompositorMutableStateProvider.h:16:
class
> WebCompositorMutableStateProvider {
> On 2015/12/16 04:01:59, esprehn wrote:
> > This is dead code right now, where is it going to get used?
> 
> At a high level, when we execute the compositor-worker callbacks we send out
to
> blink a fresh wrapper around the compositor state. The CompositorProxies are
> connected to the appropriate state before the callbacks are executed
>
(https://codereview.chromium.org/1547893003/diff/1/third_party/WebKit/Source/w...).
> 
>
https://codereview.chromium.org/1447893002/diff/240001/third_party/WebKit/pub...
> third_party/WebKit/public/platform/WebCompositorMutableStateProvider.h:21:
> virtual WebCompositorMutableState* getMutableStateFor(uint64_t elementId) = 0;
> On 2015/12/16 04:01:58, esprehn wrote:
> > return a WebPassOwnPtr, how does the caller know when to call this?
> 
> Done.
> 
> In the current mega-patch (linked above), we grab a mutable state wrapper for
> the proxies before executing the compositor worker callbacks. We have a lot of
> flexibility about how we connect the proxies to the state, though. Some other
> alternatives I'd considered were
>  * not wrapping the per-element mutable state.
>    * this has a big downside: if we don't cache the per-element state, every
> property read and write is a hash map lookup.
>  * lazily creating the per-element wrappers for the compositor proxies
>    * This may actually be a good way to go, but it's orthogonal to this patch,
> so I'd prefer to leave that discussion to the subsequent patch that connects
the
> CompositorProxies to the compositor's mutable state.

esprehn@, ping :)

[esprehn, 2016-01-06 19:48:38]

that release() method doesn't seem needed just for tests, why not just use a
WebOwnPtr? Also it should be called leakPtr() not release().

With that fixed lgtm.

https://codereview.chromium.org/1447893002/diff/280001/cc/animation/layer_tre...
File cc/animation/layer_tree_mutation.h (right):

https://codereview.chromium.org/1447893002/diff/280001/cc/animation/layer_tre...
cc/animation/layer_tree_mutation.h:14: struct LayerTreeMutation {
class ? You have public: and all the state is private

https://codereview.chromium.org/1447893002/diff/280001/cc/animation/layer_tre...
cc/animation/layer_tree_mutation.h:59: typedef base::hash_map<uint64_t,
LayerTreeMutation> LayerTreeMutationMap;
note that LayerTreeMutation objects are pretty big, so the map is wasting a
bunch of space. We usually do put OwnPtr<T> into the map instead. I guess this
map isn't usually very large?

https://codereview.chromium.org/1447893002/diff/280001/cc/blink/web_composito...
File cc/blink/web_compositor_mutable_state_impl_unittest.cc (right):

https://codereview.chromium.org/1447893002/diff/280001/cc/blink/web_composito...
cc/blink/web_compositor_mutable_state_impl_unittest.cc:140:
provider.getMutableStateFor(42).release());
just use a WebOwnPtr, why use a scoped_ptr and add a release() method?

https://codereview.chromium.org/1447893002/diff/280001/cc/blink/web_composito...
File cc/blink/web_compositor_mutable_state_provider_impl.h (right):

https://codereview.chromium.org/1447893002/diff/280001/cc/blink/web_composito...
cc/blink/web_compositor_mutable_state_provider_impl.h:17: typedef
base::hash_map<uint64_t, LayerTreeMutation> LayerTreeMutationMap;
why not just include the header? typedef of a complex type like this seems a bit
error prone.

https://codereview.chromium.org/1447893002/diff/280001/third_party/WebKit/pub...
File third_party/WebKit/public/platform/WebPassOwnPtr.h (right):

https://codereview.chromium.org/1447893002/diff/280001/third_party/WebKit/pub...
third_party/WebKit/public/platform/WebPassOwnPtr.h:54: T* release()
leakPtr()

[Ian Vollick, 2016-01-08 01:12:31]

Description was changed from

==========
compositor-worker: Introduce WebMutableState

This change introduces a wrapper around compositer-owned state that may
be mutated by a compositor worker.

This depends on crrev.com/1405993008

BUG=430155
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
compositor-worker: Introduce WebCompositorMutableState

This change introduces a wrapper around compositer-owned state that may
be mutated by a compositor worker.

This depends on crrev.com/1405993008

BUG=430155
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[Ian Vollick, 2016-01-08 03:01:54]

https://codereview.chromium.org/1447893002/diff/280001/cc/animation/layer_tre...
File cc/animation/layer_tree_mutation.h (right):

https://codereview.chromium.org/1447893002/diff/280001/cc/animation/layer_tre...
cc/animation/layer_tree_mutation.h:14: struct LayerTreeMutation {
On 2016/01/06 19:48:37, esprehn wrote:
> class ? You have public: and all the state is private

Done.

https://codereview.chromium.org/1447893002/diff/280001/cc/animation/layer_tre...
cc/animation/layer_tree_mutation.h:59: typedef base::hash_map<uint64_t,
LayerTreeMutation> LayerTreeMutationMap;
On 2016/01/06 19:48:37, esprehn wrote:
> note that LayerTreeMutation objects are pretty big, so the map is wasting a
> bunch of space. We usually do put OwnPtr<T> into the map instead. I guess this
> map isn't usually very large?

That's true. You could have one of these for every compositor proxy you muck
with in a frame, so it's probably not usually very large, but it could be. That
said, the plan is to copy this as little as possible -- we'll make a copy to
send values back to the main thread, eg.

https://codereview.chromium.org/1447893002/diff/280001/cc/blink/web_composito...
File cc/blink/web_compositor_mutable_state_impl_unittest.cc (right):

https://codereview.chromium.org/1447893002/diff/280001/cc/blink/web_composito...
cc/blink/web_compositor_mutable_state_impl_unittest.cc:140:
provider.getMutableStateFor(42).release());
On 2016/01/06 19:48:37, esprehn wrote:
> just use a WebOwnPtr, why use a scoped_ptr and add a release() method?

I took your offline suggestion and made a conversion from WebPassOwnPtr to
scoped_ptr.

https://codereview.chromium.org/1447893002/diff/280001/cc/blink/web_composito...
File cc/blink/web_compositor_mutable_state_provider_impl.h (right):

https://codereview.chromium.org/1447893002/diff/280001/cc/blink/web_composito...
cc/blink/web_compositor_mutable_state_provider_impl.h:17: typedef
base::hash_map<uint64_t, LayerTreeMutation> LayerTreeMutationMap;
On 2016/01/06 19:48:37, esprehn wrote:
> why not just include the header? typedef of a complex type like this seems a
bit
> error prone.

Done.

https://codereview.chromium.org/1447893002/diff/280001/third_party/WebKit/pub...
File third_party/WebKit/public/platform/WebPassOwnPtr.h (right):

https://codereview.chromium.org/1447893002/diff/280001/third_party/WebKit/pub...
third_party/WebKit/public/platform/WebPassOwnPtr.h:54: T* release()
On 2016/01/06 19:48:37, esprehn wrote:
> leakPtr()

This has been replaced with the scoped_ptr conversion.

[esprehn, 2016-01-08 03:36:26]

Lgtm, if people end up creating a lot of proxies we should consider storing
scoped_ptr in the map.

[Ian Vollick, 2016-01-08 03:44:59]

On 2016/01/08 03:36:26, esprehn wrote:
> Lgtm, if people end up creating a lot of proxies we should consider storing
> scoped_ptr in the map.

Sg. Thanks also for the nice WebPassOwnPtr suggestion.

[Ian Vollick, 2016-01-08 03:45:02]

On 2016/01/08 03:36:26, esprehn wrote:
> Lgtm, if people end up creating a lot of proxies we should consider storing
> scoped_ptr in the map.

Sg. Thanks also for the nice WebPassOwnPtr suggestion.

[Ian Vollick, 2016-01-08 03:45:22]

The CQ bit was checked by vollick@chromium.org

[Ian Vollick, 2016-01-08 03:45:23]

The patchset sent to the CQ was uploaded after l-g-t-m from ajuma@chromium.org
Link to the patchset: https://codereview.chromium.org/1447893002/#ps300001
(title: "Address review feedback.")

[commit-bot: I haz the power, 2016-01-08 03:45:38]

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

[commit-bot: I haz the power, 2016-01-08 04:36:44]

Description was changed from

==========
compositor-worker: Introduce WebCompositorMutableState

This change introduces a wrapper around compositer-owned state that may
be mutated by a compositor worker.

This depends on crrev.com/1405993008

BUG=430155
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
compositor-worker: Introduce WebCompositorMutableState

This change introduces a wrapper around compositer-owned state that may
be mutated by a compositor worker.

This depends on crrev.com/1405993008

BUG=430155
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[commit-bot: I haz the power, 2016-01-08 04:36:45]

Committed patchset #16 (id:300001)

[commit-bot: I haz the power, 2016-01-08 04:37:51]

Description was changed from

==========
compositor-worker: Introduce WebCompositorMutableState

This change introduces a wrapper around compositer-owned state that may
be mutated by a compositor worker.

This depends on crrev.com/1405993008

BUG=430155
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
compositor-worker: Introduce WebCompositorMutableState

This change introduces a wrapper around compositer-owned state that may
be mutated by a compositor worker.

This depends on crrev.com/1405993008

BUG=430155
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/9c7f6d0e46a46605801d143c04b86f5e4445cd37
Cr-Commit-Position: refs/heads/master@{#368273}
==========

[commit-bot: I haz the power, 2016-01-08 04:37:52]

Patchset 16 (id:??) landed as
https://crrev.com/9c7f6d0e46a46605801d143c04b86f5e4445cd37
Cr-Commit-Position: refs/heads/master@{#368273}