Don't expose a LayerImpl reference to CompositorProxy.

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

Index: third_party/WebKit/Source/platform/graphics/CompositorMutatorClient.cpp
diff --git a/third_party/WebKit/Source/platform/graphics/CompositorMutatorClient.cpp b/third_party/WebKit/Source/platform/graphics/CompositorMutatorClient.cpp
index f0344194d387cffe88dfb9a80c86baf90bcde12d..e249f15e45ce604049b27016181cf70cf24d82a2 100644
--- a/third_party/WebKit/Source/platform/graphics/CompositorMutatorClient.cpp
+++ b/third_party/WebKit/Source/platform/graphics/CompositorMutatorClient.cpp
@@ -4,16 +4,18 @@
 
 #include "platform/graphics/CompositorMutatorClient.h"
 
+#include <memory>
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/trace_event/trace_event.h"
 #include "cc/trees/layer_tree_impl.h"
+#include "cc/trees/scroll_node.h"
+#include "platform/graphics/CompositorElementId.h"
 #include "platform/graphics/CompositorMutableStateProvider.h"
 #include "platform/graphics/CompositorMutation.h"
 #include "platform/graphics/CompositorMutationsTarget.h"
 #include "platform/graphics/CompositorMutator.h"
 #include "wtf/PtrUtil.h"
-#include <memory>
 
 namespace blink {
 
@@ -39,8 +41,35 @@ bool CompositorMutatorClient::Mutate(base::TimeTicks monotonicTime,
   double monotonicTimeNow = (monotonicTime - base::TimeTicks()).InSecondsF();
   if (!m_mutations)
     m_mutations = WTF::wrapUnique(new CompositorMutations);
-  CompositorMutableStateProvider compositorState(treeImpl, m_mutations.get());
+
+  // Populate ProxyCompositorMutablePropertiesMap.

[smcgruer, 2017/03/20 14:31:51]

Nit; this comment isn't useful, the code is clear enough

[Scott Funkenhauser, 2017/03/21 19:31:11]

Done.

+  ProxyCompositorMutablePropertiesMap inputPropertiesMap;
+  snapshotLayerTree(treeImpl, &inputPropertiesMap);
+
+  CompositorMutations newMutations;
+  CompositorMutableStateProvider compositorState(&inputPropertiesMap,
+                                                 &newMutations);
   bool shouldReinvoke = m_mutator->mutate(monotonicTimeNow, &compositorState);
+
+  for (const auto& entry : newMutations.map) {
+    updateLayerTree(treeImpl, entry.key, entry.value.get());
+  }
+
+  // TODO(sfunkenhauser): Temporary work around.
+  // Currently ScrollTree:OnScrollOffsetAnimted triggers a main frame

[smcgruer, 2017/03/20 14:31:51]

1. Typo in OnScrollOffsetAnimated

2. It may be more correct to say it triggers a main frame begin synchronously.
Only main can actually trigger a main frame afaik.

[Scott Funkenhauser, 2017/03/21 19:31:12]

Done.

+  // synchronously, which will call TakeMutations causing m_mutations to be
+  // released.  Make sure m_mutations is not null before calling

[smcgruer, 2017/03/20 14:31:51]

Nit; I think lines 61,62 would fit on one line if you got rid of the
double-space after released.

[Scott Funkenhauser, 2017/03/21 19:31:11]

Done.

+  // updateMutations.
+  if (!m_mutations)
+    m_mutations = WTF::wrapUnique(new CompositorMutations);
+
+  // After this loop newMutations will no longer hold any CompositorMutation
+  // objects.  CompositorMutation objects are moved to updateMutations to reduce
+  // the number of CompositorMutation objects that are allocated.
+  for (auto& entry : newMutations.map) {
+    updateMutations(entry.key, std::move(entry.value));
+  }
+
   return shouldReinvoke;
 }
 
@@ -55,7 +84,6 @@ base::Closure CompositorMutatorClient::TakeMutations() {
                "CompositorMutatorClient::TakeMutations");
   if (!m_mutations)
     return base::Closure();
-
   return base::Bind(&CompositorMutationsTarget::applyMutations,
                     base::Unretained(m_mutationsTarget),
                     base::Owned(m_mutations.release()));
@@ -66,9 +94,118 @@ void CompositorMutatorClient::setNeedsMutate() {
   m_client->SetNeedsMutate();
 }
 
+void CompositorMutatorClient::registerCompositorProxy(
+    uint64_t proxyId,
+    uint64_t elementId,
+    uint32_t mutableProperties) {
+  TRACE_EVENT0("compositor-worker",
+               "CompositorMutatorClient::registerCompositorProxy");
+  m_inputProperties[proxyId] = std::make_pair(elementId, mutableProperties);
+}
+
+void CompositorMutatorClient::unregisterCompositorProxy(uint64_t proxyId) {
+  TRACE_EVENT0("compositor-worker",
+               "CompositorMutatorClient::unregisterCompositorProxy");
+  DCHECK(m_inputProperties.find(proxyId) != m_inputProperties.end());
+  m_inputProperties.erase(proxyId);
+}
+
 void CompositorMutatorClient::setMutationsForTesting(
     std::unique_ptr<CompositorMutations> mutations) {
   m_mutations = std::move(mutations);
 }
 
+void CompositorMutatorClient::snapshotLayerTree(
+    const cc::LayerTreeImpl* treeImpl,
+    ProxyCompositorMutablePropertiesMap* inputMap) const {
+  for (const auto& it : m_inputProperties) {
+    uint64_t proxyId = it.first;
+    const std::pair<uint64_t, uint32_t>& value = it.second;
+    uint64_t elementId = value.first;
+    uint32_t properties = value.second;
+    cc::LayerImpl* layer = treeImpl->LayerByElementId(
+        createCompositorElementId(elementId, CompositorSubElementId::Primary));
+    cc::LayerImpl* scrollLayer = treeImpl->LayerByElementId(
+        createCompositorElementId(elementId, CompositorSubElementId::Scroll));
+    if (!layer && !scrollLayer)
+      continue;
+    (*inputMap)[proxyId].elementId = elementId;
+    (*inputMap)[proxyId].transform = SkMatrix44::I();
+    if (layer) {
+      if (properties & CompositorMutableProperty::kOpacity) {
+        (*inputMap)[proxyId].opacity = layer->Opacity();
+      }
+      if (properties & CompositorMutableProperty::kTransform) {
+        (*inputMap)[proxyId].transform = layer->Transform().matrix();
+      }
+    }
+    if (scrollLayer) {
+      if (properties & CompositorMutableProperty::kScrollLeft) {
+        (*inputMap)[proxyId].scrollLeft =
+            scrollLayer->CurrentScrollOffset().x();
+      }
+      if (properties & CompositorMutableProperty::kScrollTop) {
+        (*inputMap)[proxyId].scrollTop = scrollLayer->CurrentScrollOffset().y();
+      }
+    }
+  }
+}
+
+void CompositorMutatorClient::updateLayerTree(

[smcgruer, 2017/03/20 14:31:51]

Could this be an anon-namespace method? I don't see any member accesses.

[Scott Funkenhauser, 2017/03/21 19:31:11]

Done.

+    cc::LayerTreeImpl* treeImpl,
+    uint64_t elementId,
+    const CompositorMutation* mutation) const {
+  cc::LayerImpl* layer = treeImpl->LayerByElementId(
+      createCompositorElementId(elementId, CompositorSubElementId::Primary));
+  cc::LayerImpl* scrollLayer = treeImpl->LayerByElementId(
+      createCompositorElementId(elementId, CompositorSubElementId::Scroll));
+  if (layer) {
+    if (mutation->isOpacityMutated()) {
+      treeImpl->property_trees()->effect_tree.OnOpacityAnimated(
+          mutation->opacity(), layer->effect_tree_index(), treeImpl);
+    }
+    if (mutation->isTransformMutated()) {
+      treeImpl->property_trees()->transform_tree.OnTransformAnimated(
+          gfx::Transform(mutation->transform()), layer->transform_tree_index(),
+          treeImpl);
+    }
+  }
+  if (scrollLayer) {
+    gfx::ScrollOffset offset = scrollLayer->CurrentScrollOffset();
+    if (mutation->isScrollLeftMutated()) {
+      offset.set_x(mutation->scrollLeft());
+    }
+    if (mutation->isScrollTopMutated()) {
+      offset.set_y(mutation->scrollTop());
+    }
+    if (mutation->isScrollLeftMutated() || mutation->isScrollTopMutated()) {
+      treeImpl->property_trees()->scroll_tree.OnScrollOffsetAnimated(
+          scrollLayer->id(), scrollLayer->scroll_tree_index(), offset,
+          treeImpl);
+    }
+  }
+}
+
+void CompositorMutatorClient::updateMutations(
+    uint64_t elementId,
+    std::unique_ptr<CompositorMutation> mutation) {
+  if (m_mutations->map.contains(elementId)) {
+    CompositorMutation* existingMutation = m_mutations->map.at(elementId);
+    if (mutation->isOpacityMutated()) {
+      existingMutation->setOpacity(mutation->opacity());
+    }
+    if (mutation->isTransformMutated()) {
+      existingMutation->setTransform(mutation->transform());
+    }
+    if (mutation->isScrollTopMutated()) {
+      existingMutation->setScrollTop(mutation->scrollTop());
+    }
+    if (mutation->isScrollLeftMutated()) {
+      existingMutation->setScrollLeft(mutation->scrollLeft());
+    }
+  } else {
+    m_mutations->map.set(elementId, std::move(mutation));
+  }
+}
+
 }  // namespace blink