CC Animation: Move files from cc_blink to Source/platform/animation

third_party/WebKit/Source/platform/CompositorFactory.cpp

Index: third_party/WebKit/Source/platform/CompositorFactory.cpp
diff --git a/third_party/WebKit/Source/platform/CompositorFactory.cpp b/third_party/WebKit/Source/platform/CompositorFactory.cpp
new file mode 100644
index 0000000000000000000000000000000000000000..3e23d6a78ccec320edd4440685af5500c27466f7
--- /dev/null
+++ b/third_party/WebKit/Source/platform/CompositorFactory.cpp
@@ -0,0 +1,98 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "platform/CompositorFactory.h"
+
+#include "cc/animation/animation_player.h"
+#include "cc/animation/animation_timeline.h"
+#include "platform/animation/CompositorAnimation.h"
+#include "platform/animation/CompositorAnimationPlayer.h"
+#include "platform/animation/CompositorAnimationTimeline.h"
+#include "platform/animation/CompositorFilterAnimationCurve.h"
+#include "platform/animation/CompositorFloatAnimationCurve.h"
+#include "platform/animation/CompositorTransformAnimationCurve.h"
+#include "platform/animation/CompositorTransformOperations.h"
+#include "platform/graphics/CompositorFilterOperations.h"
+
+namespace blink {
+
+class CompositorFactoryImpl : public CompositorFactory {
+public:
+    CompositorFilterAnimationCurve* createFilterAnimationCurve() override
+    {
+        return new CompositorFilterAnimationCurve();
+    }
+
+    CompositorFloatAnimationCurve* createFloatAnimationCurve() override
+    {
+        return new CompositorFloatAnimationCurve();
+    }
+
+    CompositorScrollOffsetAnimationCurve* createScrollOffsetAnimationCurve(
+        WebFloatPoint targetValue,
+        CompositorAnimationCurve::TimingFunctionType timingFunctionType,
+        CompositorScrollOffsetAnimationCurve::ScrollDurationBehavior durationBehavior)
+    {
+        return new CompositorScrollOffsetAnimationCurve(targetValue, timingFunctionType,
+            durationBehavior);
+    }
+
+    CompositorTransformAnimationCurve* createTransformAnimationCurve() override
+    {
+        return new CompositorTransformAnimationCurve();
+    }
+
+    CompositorTransformOperations* createTransformOperations() override
+    {
+        return new CompositorTransformOperations();
+    }
+
+    CompositorFilterOperations* createFilterOperations() override
+    {
+        return new CompositorFilterOperations();
+    }
+
+    CompositorAnimation* createAnimation(const blink::CompositorAnimationCurve& curve, blink::CompositorAnimation::TargetProperty target, int groupId, int animationId) override
+    {
+        return new CompositorAnimation(curve, target, animationId, groupId);
+    }
+
+    CompositorAnimationPlayer* createAnimationPlayer()
+    {
+        return new CompositorAnimationPlayer();
+    }
+
+    CompositorAnimationTimeline* createAnimationTimeline()
+    {
+        return new CompositorAnimationTimeline();
+    }
+};
+
+static CompositorFactory* s_factory = 0;
+
+void CompositorFactory::initializeDefault()
+{
+    delete s_factory;
+    s_factory = new CompositorFactoryImpl();
+}
+
+void CompositorFactory::initializeForTesting(CompositorFactory* factory)

[jbroman, 2016/01/29 15:29:18]

Passing ownership through a raw pointer = :(.

[loyso (OOO), 2016/02/01 05:31:32]

Actually, I have CompositorFactoryMock-type pointer owned by test fixture. But I
can change this to OwnPtr. Done.

[jbroman, 2016/02/01 15:44:36]

The delete calls make this an owning pointer, and it's confusing to have two
(exclusively) owning pointers at once. (If the test fixture owned it, I'd expect
the static pointer not to manage deletion.)

This looks fine now.

+{
+    delete s_factory;
+    s_factory = factory;
+}
+
+void CompositorFactory::shutdown()
+{
+    delete s_factory;

[jbroman, 2016/01/29 15:29:18]

Why is it necessary to shut down this factory?

[loyso (OOO), 2016/02/01 05:31:32]

In this CL I want to preserve the behavior of CompositorSupport object.

+    s_factory = nullptr;
+}
+
+CompositorFactory& CompositorFactory::current()
+{
+    ASSERT(s_factory);

[jbroman, 2016/01/29 15:29:18]

How about something like:

// Does not assume ownership.
// Reset by passing nullptr after each test, before destroying the mock factory.
void CompositorFactory::setForTesting(CompositorFactory* factory)
{
    s_factoryForTesting = factory;
}

CompositorFactory& CompositorFactory::current()
{
    if (s_factoryForTesting)
        return s_factoryForTesting;
    DEFINE_STATIC_LOCAL(CompositorFactoryImpl, defaultFactory, ());
    return defaultFactory;
}

[loyso (OOO), 2016/02/01 05:31:32]

Technically, this is an extra conditional branch in assembly code.
In general, this would be the Meyer's lazy initialized singleton with known
drawbacks.

I would like to keep following "the scope" idiom (so we know the init-time
explicitly and we can shutdown the subsystem). It starts to make sense if you
have non-trivial initialization and shutdown procedures. See WebLayer factory
and WebLayerImpl::SetLayerSettings, for example.

FTR, it looks like our blink (compositor animation?) tests are rather chaotic.
We call Platform::initialize multiple times without calling Platform::shutdown.
I would like to solve this problem as a separate CL.

In total, yes. We can go with STATIC_LOCAL in this case. No objections. Would be
nice to have a design policy for singletons+factories+TDD (IoC containers? :) )

[jbroman, 2016/02/01 15:44:36]

I'm not too worried about that (there's a lot of much more expensive stuff going
on at the same time), and I'd expect a branch predictor to be pretty good at
"this branch always goes right" if it did get hot.
Yeah, it's just a common pattern in Blink to have singletons using
DEFINE_STATIC_LOCAL.
Ouch. With that in mind, addressing separately seems OK.
Unit testing is still fairly new in Blink, so there's not a lot of precedent for
unit testing infrastructure in general.

+    return *s_factory;
+}
+
+} // namespace blink