Modified: trunk/Source/WebCore/ChangeLog (125551 => 125552)
--- trunk/Source/WebCore/ChangeLog 2012-08-14 12:34:23 UTC (rev 125551)
+++ trunk/Source/WebCore/ChangeLog 2012-08-14 12:39:17 UTC (rev 125552)
@@ -1,3 +1,34 @@
+2012-08-14 Ed Baker <[email protected]>
+
+ [BlackBerry] LayerAnimation is not immutable, which makes dereferencing an expensive operation
+ https://bugs.webkit.org/show_bug.cgi?id=93946
+
+ Reviewed by Antonio Gomes.
+
+ Make LayerAnimation immutable so it can be dereferenced from the main
+ WebKit thread without having to dispatch to the compositing thread,
+ which is an expensive operation.
+
+ TransformOperation and TimingFunction need to be made thread safe as
+ they are referenced in LayerAnimation, but that effort is being tracked
+ by a separate bug, #86483.
+
+ Reviewed internally by Arvid Nilsson.
+
+ No change in behavior, no new tests.
+
+ * platform/graphics/blackberry/LayerAnimation.h:
+ (WebCore::LayerAnimation::name):
+ (LayerAnimation):
+ (WebCore::LayerAnimation::LayerAnimation):
+ (WebCore::LayerAnimation::setName):
+ * platform/graphics/blackberry/LayerCompositingThread.cpp:
+ (WebCore):
+ * platform/graphics/blackberry/LayerCompositingThread.h:
+ (LayerCompositingThread):
+ * platform/graphics/blackberry/LayerWebKitThread.cpp:
+ (WebCore::LayerWebKitThread::~LayerWebKitThread):
+
2012-08-14 Arvid Nilsson <[email protected]>
[BlackBerry] GraphicsLayerBlackBerry::willBeDestroyed() must call superclass implementation
Modified: trunk/Source/WebCore/platform/graphics/blackberry/LayerAnimation.h (125551 => 125552)
--- trunk/Source/WebCore/platform/graphics/blackberry/LayerAnimation.h 2012-08-14 12:34:23 UTC (rev 125551)
+++ trunk/Source/WebCore/platform/graphics/blackberry/LayerAnimation.h 2012-08-14 12:39:17 UTC (rev 125552)
@@ -34,17 +34,8 @@
class LayerCompositingThread;
class TransformationMatrix;
-// This class uses non-threadsafe refcounting in the WebCore::Animation and
-// WebCore::String members, so using threadsafe refcounting here would be a big
-// cover-up. Instead, you must be careful to use ref/deref this class only on
-// the WebKit thread, or when the WebKit and compositing threads are in sync.
-class LayerAnimation : public RefCounted<LayerAnimation> {
+class LayerAnimation : public ThreadSafeRefCounted<LayerAnimation> {
public:
- // WebKit thread only
- // Because the refcounting done in constructor and destructor is not thread safe,
- // the LayerAnimation must be created or destroyed on the WebKit thread, or when
- // the WebKit and compositing threads are in sync.
- // Also, the name property is using a String which has non-threadsafe refcounting.
// The setStartTime method is not threadsafe and must only be called on a newly
// created LayerAnimation before sending it off to the compositing thread.
static PassRefPtr<LayerAnimation> create(const KeyframeValueList& values, const IntSize& boxSize, const Animation* animation, const String& name, double timeOffset)
@@ -65,7 +56,13 @@
{
}
- const String& name() const { return m_name; }
+ String name() const
+ {
+ if (m_name.isEmpty())
+ return String("");
+ return String(m_name);
+ }
+
void setStartTime(double time) { m_startTime = time; }
// These functions are thread safe (immutable state).
@@ -77,14 +74,10 @@
double timeOffset() const { return m_timeOffset; }
double startTime() const { return m_startTime; }
size_t valueCount() const { return m_values.size(); }
-
- // NOTE: Don't try to ref a TimingFunction, that's not a threadsafe operation.
const TimingFunction* timingFunction() const { return m_timingFunction.get(); }
double duration() const { return m_duration; }
int iterationCount() const { return m_iterationCount; }
Animation::AnimationDirection direction() const { return m_direction; }
-
- // NOTE: Don't try to clone() an AnimationValue, that's not a threadsafe operation since it mutates refcounts.
const AnimationValue* valueAt(size_t i) const { return m_values.at(i); }
bool finished() const { return m_finished; }
@@ -97,7 +90,6 @@
: m_id(reinterpret_cast<int>(animation))
, m_values(values)
, m_boxSize(boxSize)
- , m_name(name)
, m_timeOffset(timeOffset)
, m_startTime(0)
, m_timingFunction(0)
@@ -110,14 +102,14 @@
m_timingFunction = animation->timingFunction();
validateTransformLists();
+ setName(name);
}
LayerAnimation(const LayerAnimation& other)
- : RefCounted<LayerAnimation>()
+ : ThreadSafeRefCounted<LayerAnimation>()
, m_id(other.m_id)
, m_values(other.m_values)
, m_boxSize(other.m_boxSize)
- , m_name(other.m_name)
, m_timeOffset(other.m_timeOffset)
, m_startTime(other.m_startTime)
, m_transformFunctionListValid(other.m_transformFunctionListValid)
@@ -127,17 +119,26 @@
, m_direction(other.m_direction)
, m_finished(false)
{
+ setName(other.name());
}
void validateTransformLists();
+ void setName(const String& name)
+ {
+ unsigned length = name.length();
+ m_name.resize(length);
+ if (length)
+ memcpy(m_name.data(), name.characters(), sizeof(UChar) * length);
+ }
+
int m_id;
// NOTE: Don't expose the KeyframeValueList directly, since its copy
// constructor mutates refcounts and thus is not thread safe.
KeyframeValueList m_values;
IntSize m_boxSize;
- String m_name;
+ Vector<UChar> m_name; // Must not use String member when deriving from ThreadSafeRefCounted
double m_timeOffset;
double m_startTime;
bool m_transformFunctionListValid;
Modified: trunk/Source/WebCore/platform/graphics/blackberry/LayerCompositingThread.cpp (125551 => 125552)
--- trunk/Source/WebCore/platform/graphics/blackberry/LayerCompositingThread.cpp 2012-08-14 12:34:23 UTC (rev 125551)
+++ trunk/Source/WebCore/platform/graphics/blackberry/LayerCompositingThread.cpp 2012-08-14 12:39:17 UTC (rev 125552)
@@ -357,21 +357,6 @@
}
#endif
-void LayerCompositingThread::clearAnimations()
-{
- // Animations don't use thread safe refcounting, and must only be
- // touched when the two threads are in sync.
- if (!isCompositingThread()) {
- dispatchSyncCompositingMessage(BlackBerry::Platform::createMethodCallMessage(
- &LayerCompositingThread::clearAnimations,
- this));
- return;
- }
-
- m_runningAnimations.clear();
- m_suspendedAnimations.clear();
-}
-
void LayerCompositingThread::removeSublayer(LayerCompositingThread* sublayer)
{
ASSERT(isCompositingThread());
Modified: trunk/Source/WebCore/platform/graphics/blackberry/LayerCompositingThread.h (125551 => 125552)
--- trunk/Source/WebCore/platform/graphics/blackberry/LayerCompositingThread.h 2012-08-14 12:34:23 UTC (rev 125551)
+++ trunk/Source/WebCore/platform/graphics/blackberry/LayerCompositingThread.h 2012-08-14 12:39:17 UTC (rev 125552)
@@ -131,7 +131,6 @@
#if ENABLE(VIDEO)
void setMediaPlayer(MediaPlayer*);
#endif
- void clearAnimations();
// Not thread safe
Modified: trunk/Source/WebCore/platform/graphics/blackberry/LayerWebKitThread.cpp (125551 => 125552)
--- trunk/Source/WebCore/platform/graphics/blackberry/LayerWebKitThread.cpp 2012-08-14 12:34:23 UTC (rev 125551)
+++ trunk/Source/WebCore/platform/graphics/blackberry/LayerWebKitThread.cpp 2012-08-14 12:39:17 UTC (rev 125552)
@@ -76,8 +76,6 @@
LayerWebKitThread::~LayerWebKitThread()
{
- m_layerCompositingThread->clearAnimations();
-
if (m_frontBufferLock)
pthread_mutex_destroy(m_frontBufferLock);
