compositor-worker: plumb element id and mutable properties

third_party/WebKit/Source/core/dom/CompositorProxy.cpp

Index: third_party/WebKit/Source/core/dom/CompositorProxy.cpp
diff --git a/third_party/WebKit/Source/core/dom/CompositorProxy.cpp b/third_party/WebKit/Source/core/dom/CompositorProxy.cpp
index 371f3fb2e980bdaf081e92a47536b349c0d3d073..568aaacc583350f852f6ef0feaa21f2e6c45d3bf 100644
--- a/third_party/WebKit/Source/core/dom/CompositorProxy.cpp
+++ b/third_party/WebKit/Source/core/dom/CompositorProxy.cpp
@@ -12,42 +12,34 @@
 #include "core/dom/ExecutionContext.h"
 #include "platform/ThreadSafeFunctional.h"
 #include "public/platform/Platform.h"
+#include "public/platform/WebCompositorMutableProperties.h"
 #include "public/platform/WebTraceLocation.h"
 
 namespace blink {
 
-struct AttributeFlagMapping {
+struct MutablePropertyMapping {
     const char* name;
     unsigned length;
-    CompositorProxy::Attributes attribute;
+    WebCompositorMutableProperty compositorMutableProperty;

[esprehn, 2015/12/02 03:28:48]

I would just call this "property"

 };
 
-static AttributeFlagMapping allowedAttributes[] = {
-    { "opacity", 7, CompositorProxy::Attributes::OPACITY },
-    { "scrollleft", 10, CompositorProxy::Attributes::SCROLL_LEFT },
-    { "scrolltop", 9, CompositorProxy::Attributes::SCROLL_TOP },
-    { "touch", 5, CompositorProxy::Attributes::TOUCH },
-    { "transform", 8, CompositorProxy::Attributes::TRANSFORM },
+static MutablePropertyMapping allowedProperties[] = {
+    { "opacity", 7, WebCompositorMutablePropertyOpacity },
+    { "scrollleft", 10, WebCompositorMutablePropertyScrollLeft },
+    { "scrolltop", 9, WebCompositorMutablePropertyScrollTop },
+    { "transform", 9, WebCompositorMutablePropertyTransform },
 };
 
-static bool CompareAttributeName(const AttributeFlagMapping& attribute, StringImpl* attributeLower)
+static WebCompositorMutableProperty compositorMutablePropertyForName(const String& attributeName)
 {
-    ASSERT(attributeLower->is8Bit());
-    return memcmp(attribute.name, attributeLower->characters8(), std::min(attribute.length, attributeLower->length())) < 0;
-}
-
-static CompositorProxy::Attributes attributeFlagForName(const String& attributeName)
-{
-    CompositorProxy::Attributes attributeFlag = CompositorProxy::Attributes::NONE;
-    const String attributeLower = attributeName.lower();
-    const AttributeFlagMapping* start = allowedAttributes;
-    const AttributeFlagMapping* end = allowedAttributes + WTF_ARRAY_LENGTH(allowedAttributes);
-    if (attributeLower.impl()->is8Bit()) {
-        const AttributeFlagMapping* match = std::lower_bound(start, end, attributeLower.impl(), CompareAttributeName);
-        if (match != end)
-            attributeFlag = match->attribute;
+    String attributeLower = attributeName.lower();
+    for (size_t i = 0; i < WTF_ARRAY_LENGTH(allowedProperties); ++i) {
+        const MutablePropertyMapping& mapping = allowedProperties[i];
+        String propertyName(mapping.name, mapping.length);

[esprehn, 2015/12/02 03:28:48]

this mallocs a string repeatedly as you loop, don't do that. Do std::lower_bound
on the bytes of the attributeLower itself.

[Ian Vollick, 2015/12/03 17:54:43]

Done. There was a bug here (crbug.com/531577). I've added a fix to the
lower_bound version and added a test to cover it.

+        if (propertyName == attributeLower)
+            return mapping.compositorMutableProperty;
     }
-    return attributeFlag;
+    return WebCompositorMutablePropertyNone;
 }
 
 static bool isControlThread()
@@ -61,24 +53,24 @@ static bool isCallingCompositorFrameCallback()
     return true;
 }
 
-static void decrementCountForElement(uint64_t elementId)
+static void decrementProxiedPropertyCountsForElement(uint64_t elementId, uint32_t compositorMutableProperties)
 {
     ASSERT(isMainThread());
     Node* node = DOMNodeIds::nodeForId(elementId);
     if (!node)
         return;
     Element* element = toElement(node);
-    element->decrementProxyCount();
+    element->decrementProxiedPropertyCounts(compositorMutableProperties);
 }
 
-static void incrementProxyCountForElement(uint64_t elementId)
+static void incrementProxiedPropertyCountsForElement(uint64_t elementId, uint32_t compositorMutableProperties)
 {
     ASSERT(isMainThread());
     Node* node = DOMNodeIds::nodeForId(elementId);
     if (!node)
         return;
     Element* element = toElement(node);
-    element->incrementProxyCount();
+    element->incrementProxiedPropertyCounts(compositorMutableProperties);
 }
 
 static bool raiseExceptionIfMutationNotAllowed(ExceptionState& exceptionState)
@@ -94,23 +86,24 @@ static bool raiseExceptionIfMutationNotAllowed(ExceptionState& exceptionState)
     return false;
 }
 
-static uint32_t attributesBitfieldFromNames(const Vector<String>& attributeArray)
+static uint32_t compositorMutablePropertiesFromNames(const Vector<String>& attributeArray)
 {
-    uint32_t attributesBitfield = 0;
+    uint32_t compositorMutableProperties = 0;

[esprehn, 2015/12/02 03:28:48]

you can name your local vars a lot shorter if you want, like this can just be
|properties|

[Ian Vollick, 2015/12/03 17:54:43]

Done.

     for (const auto& attribute : attributeArray) {
-        attributesBitfield |= static_cast<uint32_t>(attributeFlagForName(attribute));
+        compositorMutableProperties |= static_cast<uint32_t>(compositorMutablePropertyForName(attribute));
     }
-    return attributesBitfield;
+    return compositorMutableProperties;
 }
 
 #if ENABLE(ASSERT)
-static bool sanityCheckAttributeFlags(uint32_t attributeFlags)
+static bool sanityCheckMutableProperties(uint32_t compositorMutableProperties)
 {
-    uint32_t sanityCheckAttributes = attributeFlags;
-    for (unsigned i = 0; i < arraysize(allowedAttributes); ++i) {
-        sanityCheckAttributes &= ~static_cast<uint32_t>(allowedAttributes[i].attribute);
+    // Ensures that we only have bits set for valid mutable properties.
+    uint32_t sanityCheckProperties = compositorMutableProperties;
+    for (unsigned i = 0; i < arraysize(allowedProperties); ++i) {
+        sanityCheckProperties &= ~static_cast<uint32_t>(allowedProperties[i].compositorMutableProperty);
     }
-    return !sanityCheckAttributes;
+    return !sanityCheckProperties;
 }
 #endif
 
@@ -125,29 +118,29 @@ CompositorProxy* CompositorProxy::create(ExecutionContext* context, Element* ele
     return new CompositorProxy(*element, attributeArray);
 }
 
-CompositorProxy* CompositorProxy::create(uint64_t elementId, uint32_t attributeFlags)
+CompositorProxy* CompositorProxy::create(uint64_t elementId, uint32_t compositorMutableProperties)
 {
-    return new CompositorProxy(elementId, attributeFlags);
+    return new CompositorProxy(elementId, compositorMutableProperties);
 }
 
 CompositorProxy::CompositorProxy(Element& element, const Vector<String>& attributeArray)
     : m_elementId(DOMNodeIds::idForNode(&element))
-    , m_bitfieldsSupported(attributesBitfieldFromNames(attributeArray))
+    , m_compositorMutableProperties(compositorMutablePropertiesFromNames(attributeArray))
 {
     ASSERT(isMainThread());
-    ASSERT(m_bitfieldsSupported);
-    ASSERT(sanityCheckAttributeFlags(m_bitfieldsSupported));
+    ASSERT(m_compositorMutableProperties);
+    ASSERT(sanityCheckMutableProperties(m_compositorMutableProperties));
 
-    incrementProxyCountForElement(m_elementId);
+    incrementProxiedPropertyCountsForElement(m_elementId, m_compositorMutableProperties);
 }
 
-CompositorProxy::CompositorProxy(uint64_t elementId, uint32_t attributeFlags)
+CompositorProxy::CompositorProxy(uint64_t elementId, uint32_t compositorMutableProperties)
     : m_elementId(elementId)
-    , m_bitfieldsSupported(attributeFlags)
+    , m_compositorMutableProperties(compositorMutableProperties)
 {
     ASSERT(isControlThread());
-    ASSERT(sanityCheckAttributeFlags(m_bitfieldsSupported));
-    Platform::current()->mainThread()->taskRunner()->postTask(BLINK_FROM_HERE, threadSafeBind(&incrementProxyCountForElement, m_elementId));
+    ASSERT(sanityCheckMutableProperties(m_compositorMutableProperties));
+    Platform::current()->mainThread()->taskRunner()->postTask(BLINK_FROM_HERE, threadSafeBind(&incrementProxiedPropertyCountsForElement, m_elementId, m_compositorMutableProperties));
 }
 
 CompositorProxy::~CompositorProxy()
@@ -158,14 +151,14 @@ CompositorProxy::~CompositorProxy()
 
 bool CompositorProxy::supports(const String& attributeName) const
 {
-    return !!(m_bitfieldsSupported & static_cast<uint32_t>(attributeFlagForName(attributeName)));
+    return !!(m_compositorMutableProperties & static_cast<uint32_t>(compositorMutablePropertyForName(attributeName)));
 }
 
 double CompositorProxy::opacity(ExceptionState& exceptionState) const
 {
     if (raiseExceptionIfMutationNotAllowed(exceptionState))
         return 0.0;
-    if (raiseExceptionIfNotMutable(Attributes::OPACITY, exceptionState))
+    if (raiseExceptionIfNotMutable(static_cast<uint32_t>(WebCompositorMutablePropertyOpacity), exceptionState))
         return 0.0;
     return m_opacity;
 }
@@ -174,7 +167,7 @@ double CompositorProxy::scrollLeft(ExceptionState& exceptionState) const
 {
     if (raiseExceptionIfMutationNotAllowed(exceptionState))
         return 0.0;
-    if (raiseExceptionIfNotMutable(Attributes::SCROLL_LEFT, exceptionState))
+    if (raiseExceptionIfNotMutable(static_cast<uint32_t>(WebCompositorMutablePropertyScrollLeft), exceptionState))
         return 0.0;
     return m_scrollLeft;
 }
@@ -183,7 +176,7 @@ double CompositorProxy::scrollTop(ExceptionState& exceptionState) const
 {
     if (raiseExceptionIfMutationNotAllowed(exceptionState))
         return 0.0;
-    if (raiseExceptionIfNotMutable(Attributes::SCROLL_TOP, exceptionState))
+    if (raiseExceptionIfNotMutable(static_cast<uint32_t>(WebCompositorMutablePropertyScrollTop), exceptionState))
         return 0.0;
     return m_scrollTop;
 }
@@ -192,7 +185,7 @@ DOMMatrix* CompositorProxy::transform(ExceptionState& exceptionState) const
 {
     if (raiseExceptionIfMutationNotAllowed(exceptionState))
         return nullptr;
-    if (raiseExceptionIfNotMutable(Attributes::TRANSFORM, exceptionState))
+    if (raiseExceptionIfNotMutable(static_cast<uint32_t>(WebCompositorMutablePropertyTransform), exceptionState))
         return nullptr;
     return m_transform;
 }
@@ -201,45 +194,45 @@ void CompositorProxy::setOpacity(double opacity, ExceptionState& exceptionState)
 {
     if (raiseExceptionIfMutationNotAllowed(exceptionState))
         return;
-    if (raiseExceptionIfNotMutable(Attributes::OPACITY, exceptionState))
+    if (raiseExceptionIfNotMutable(static_cast<uint32_t>(WebCompositorMutablePropertyOpacity), exceptionState))
         return;
     m_opacity = std::min(1., std::max(0., opacity));
-    m_mutatedAttributes |= static_cast<uint32_t>(Attributes::OPACITY);
+    m_mutatedProperties |= static_cast<uint32_t>(WebCompositorMutablePropertyTransform);
 }
 
 void CompositorProxy::setScrollLeft(double scrollLeft, ExceptionState& exceptionState)
 {
     if (raiseExceptionIfMutationNotAllowed(exceptionState))
         return;
-    if (raiseExceptionIfNotMutable(Attributes::SCROLL_LEFT, exceptionState))
+    if (raiseExceptionIfNotMutable(static_cast<uint32_t>(WebCompositorMutablePropertyScrollLeft), exceptionState))
         return;
     m_scrollLeft = scrollLeft;
-    m_mutatedAttributes |= static_cast<uint32_t>(Attributes::SCROLL_LEFT);
+    m_mutatedProperties |= static_cast<uint32_t>(WebCompositorMutablePropertyScrollLeft);
 }
 
 void CompositorProxy::setScrollTop(double scrollTop, ExceptionState& exceptionState)
 {
     if (raiseExceptionIfMutationNotAllowed(exceptionState))
         return;
-    if (raiseExceptionIfNotMutable(Attributes::SCROLL_TOP, exceptionState))
+    if (raiseExceptionIfNotMutable(static_cast<uint32_t>(WebCompositorMutablePropertyScrollTop), exceptionState))
         return;
     m_scrollTop = scrollTop;
-    m_mutatedAttributes |= static_cast<uint32_t>(Attributes::SCROLL_TOP);
+    m_mutatedProperties |= static_cast<uint32_t>(WebCompositorMutablePropertyScrollTop);
 }
 
 void CompositorProxy::setTransform(DOMMatrix* transform, ExceptionState& exceptionState)
 {
     if (raiseExceptionIfMutationNotAllowed(exceptionState))
         return;
-    if (raiseExceptionIfNotMutable(Attributes::TRANSFORM, exceptionState))
+    if (raiseExceptionIfNotMutable(static_cast<uint32_t>(WebCompositorMutablePropertyTransform), exceptionState))
         return;
     m_transform = transform;
-    m_mutatedAttributes |= static_cast<uint32_t>(Attributes::TRANSFORM);
+    m_mutatedProperties |= static_cast<uint32_t>(WebCompositorMutablePropertyTransform);
 }
 
-bool CompositorProxy::raiseExceptionIfNotMutable(Attributes attribute, ExceptionState& exceptionState) const
+bool CompositorProxy::raiseExceptionIfNotMutable(uint32_t property, ExceptionState& exceptionState) const
 {
-    if (m_connected && (m_bitfieldsSupported & static_cast<uint32_t>(attribute)))
+    if (m_connected && (m_compositorMutableProperties & property))
         return false;
     exceptionState.throwDOMException(NoModificationAllowedError,
         m_connected ? "Attempted to mutate non-mutable attribute." : "Attempted to mutate attribute on a disconnected proxy.");
@@ -250,9 +243,9 @@ void CompositorProxy::disconnect()
 {
     m_connected = false;
     if (isMainThread())
-        decrementCountForElement(m_elementId);
+        decrementProxiedPropertyCountsForElement(m_elementId, m_compositorMutableProperties);
     else
-        Platform::current()->mainThread()->taskRunner()->postTask(BLINK_FROM_HERE, threadSafeBind(&decrementCountForElement, m_elementId));
+        Platform::current()->mainThread()->taskRunner()->postTask(BLINK_FROM_HERE, threadSafeBind(&decrementProxiedPropertyCountsForElement, m_elementId, m_compositorMutableProperties));
 }
 
 } // namespace blink