Avoid exposing cc::Layer tree to CompositorProxy

third_party/WebKit/Source/platform/graphics/CompositorMutableStateProvider.cpp

Index: third_party/WebKit/Source/platform/graphics/CompositorMutableStateProvider.cpp
diff --git a/third_party/WebKit/Source/platform/graphics/CompositorMutableStateProvider.cpp b/third_party/WebKit/Source/platform/graphics/CompositorMutableStateProvider.cpp
index d80dd61f5d2d94cb5f8e3a4c91e88c0358440e2a..ed7dc55b3ad4518062d9a6e07924da87b34028e0 100644
--- a/third_party/WebKit/Source/platform/graphics/CompositorMutableStateProvider.cpp
+++ b/third_party/WebKit/Source/platform/graphics/CompositorMutableStateProvider.cpp
@@ -5,10 +5,7 @@
 #include "platform/graphics/CompositorMutableStateProvider.h"
 
 #include <memory>
-#include "cc/layers/layer_impl.h"
-#include "cc/trees/layer_tree_impl.h"
 #include "platform/graphics/CompositorElementId.h"
-#include "platform/graphics/CompositorMutableProperties.h"
 #include "platform/graphics/CompositorMutableState.h"
 #include "platform/graphics/CompositorMutation.h"
 #include "platform/wtf/PtrUtil.h"
@@ -16,33 +13,31 @@
 namespace blink {
 
 CompositorMutableStateProvider::CompositorMutableStateProvider(
-    cc::LayerTreeImpl* tree_impl,
+    ProxyCompositorMutablePropertiesMap* input_properties,
     CompositorMutations* mutations)
-    : tree_(tree_impl), mutations_(mutations) {}
+    : input_properties_(input_properties), mutations_(mutations) {}
 
 CompositorMutableStateProvider::~CompositorMutableStateProvider() {}
 
 std::unique_ptr<CompositorMutableState>
-CompositorMutableStateProvider::GetMutableStateFor(uint64_t element_id) {
-  cc::LayerImpl* main_layer = tree_->LayerByElementId(
-      CreateCompositorElementId(element_id, CompositorSubElementId::kPrimary));
-  cc::LayerImpl* scroll_layer = tree_->LayerByElementId(
-      CreateCompositorElementId(element_id, CompositorSubElementId::kScroll));
-
-  if (!main_layer && !scroll_layer)
+CompositorMutableStateProvider::GetMutableStateFor(uint64_t proxy_id) {

[flackr, 2017/04/11 17:51:36]

I'm not sure I understand why the change to getting mutable state for a certain
proxy id rather than the element. Ultimately we just need to copy the current
values for the element which could be shared between all of the proxies
referencing it, couldn't it?

[smcgruer, 2017/04/19 15:38:38]

I might not be following you correctly, but the CL is designed so that each
animator retains read/write access to the mutatable properties, whilst not
affecting animators. I.e. if A1 and A2 both proxy element P via proxies P1 and
P2, then you get the following behavior:

A1: read P1.opacity /* returns current value, say 0.5 */
A1: write 1.0 to P1.opacity
A1: read P1.opacity /* returns 1.0 */
A2: read P2.opacity /* returns 0.5 */

==> After all animators run, mutations[P].opacity is 1.0


To provide this behavior we have CompositorMutableProperties which is per-proxy,
and CompositorMutation which is per-element.

[flackr, 2017/04/20 06:54:14]

Ah I see now, can we intead store the "current" values on the CompositorProxy
objects, copying the values into the CompositorProxy object at the same time as
we give them a CompositorMutableState? I feel like this would be less complex.

[smcgruer, 2017/04/21 14:42:25]

I'm looking at this, but I don't really see how it simplifies things? We have to
store per-proxy state all the way back to CompositorMutatorClient because each
proxy can specify different mutable properties for the element, and we use that
list of mutable properties to decide what to snapshot in
CompositorMutatorClient::SnapshotLayerTree.

We could remove the concept of proxy id, but we would then have to snapshot
everything in CompositorMutatorClient::SnapshotLayerTree because there would be
no way to tell which properties are allowed for an element. That said each
individual CompositorProxy already filters for mutable properties in it's
getters/setters, so perhaps this is fine?

+  const auto it = input_properties_->find(proxy_id);
+  if (it == input_properties_->end())
     return nullptr;
 
+  CompositorMutableProperties* properties = &it->second;
+
   // Ensure that we have an entry in the map for |elementId| but do as few
   // allocations and queries as possible. This will update the map only if we
   // have not added a value for |elementId|.
-  auto result = mutations_->map.insert(element_id, nullptr);
+  auto result = mutations_->map.insert(properties->element_id, nullptr);
 
   // Only if this is a new entry do we want to allocate a new mutation.
   if (result.is_new_entry)
     result.stored_value->value = WTF::WrapUnique(new CompositorMutation);
 
-  return WTF::WrapUnique(new CompositorMutableState(
-      result.stored_value->value.get(), main_layer, scroll_layer));
+  return WTF::WrapUnique(
+      new CompositorMutableState(result.stored_value->value.get(), properties));
 }
 
 }  // namespace blink