Fix memory leak with layers.

This fix bug http://b/2394813
This is a two-parts CL (need a java modif)

- The main leak is in WebView.cpp -- nativeUpdateLayers could
  bail out if the root layer was nil, without deallocating the vector
  of updates.
- fix a leak in LayerAndroid::evaluateAnimations()
- adoptRef() for the contentLayer in GraphicsLayerAndroid
- simplify AndroidAnimation: remove the reference to the layer
  (the layer already has a reference to AndroidAnimation)
- modify the AndroidAnimation copy() methods to return directly
  a PassRefPtr, for consistency.

diff --git a/WebCore/platform/graphics/android/AndroidAnimation.cpp b/WebCore/platform/graphics/android/AndroidAnimation.cpp
index 052e0a2..5ea8d2a 100644 (file)
--- a/WebCore/platform/graphics/android/AndroidAnimation.cpp
+++ b/WebCore/platform/graphics/android/AndroidAnimation.cpp
@@ -49,10 +49,8 @@ long AndroidAnimation::instancesCount()
     return gDebugAndroidAnimationInstances;
 }
 
-AndroidAnimation::AndroidAnimation(LayerAndroid* contentLayer,
-                 const Animation* animation,
-                 double beginTime) :
-    m_contentLayer(contentLayer),
+AndroidAnimation::AndroidAnimation(const Animation* animation,
+                                   double beginTime) :
     m_beginTime(beginTime),
     m_duration(animation->duration()),
     m_iterationCount(animation->iterationCount()),
@@ -67,7 +65,6 @@ AndroidAnimation::AndroidAnimation(LayerAndroid* contentLayer,
 }
 
 AndroidAnimation::AndroidAnimation(AndroidAnimation* anim) :
-    m_contentLayer(anim->m_contentLayer),
     m_beginTime(anim->m_beginTime),
     m_duration(anim->m_duration),
     m_iterationCount(anim->m_iterationCount),
@@ -128,19 +125,27 @@ bool AndroidAnimation::checkIterationsAndProgress(double time, float* finalProgr
     return true;
 }
 
-PassRefPtr<AndroidOpacityAnimation> AndroidOpacityAnimation::create(LayerAndroid* contentLayer,
-    float fromValue, float toValue,
-    const Animation* animation, double beginTime)
+PassRefPtr<AndroidAnimationValue> AndroidAnimation::result()
 {
-    return adoptRef(new AndroidOpacityAnimation(contentLayer,
-                    fromValue, toValue, animation, beginTime));
+    if (!m_result)
+        return 0;
+    return m_result.release();
+}
+
+PassRefPtr<AndroidOpacityAnimation> AndroidOpacityAnimation::create(
+                                                float fromValue,
+                                                float toValue,
+                                                const Animation* animation,
+                                                double beginTime)
+{
+    return adoptRef(new AndroidOpacityAnimation(fromValue, toValue,
+                                                animation, beginTime));
 }
 
-AndroidOpacityAnimation::AndroidOpacityAnimation(LayerAndroid* contentLayer,
-                           float fromValue, float toValue,
-                           const Animation* animation,
-                           double beginTime)
-    : AndroidAnimation(contentLayer, animation, beginTime),
+AndroidOpacityAnimation::AndroidOpacityAnimation(float fromValue, float toValue,
+                                                 const Animation* animation,
+                                                 double beginTime)
+    : AndroidAnimation(animation, beginTime),
       m_fromValue(fromValue), m_toValue(toValue)
 {
 }
@@ -152,9 +157,9 @@ AndroidOpacityAnimation::AndroidOpacityAnimation(AndroidOpacityAnimation* anim)
 {
 }
 
-AndroidAnimation* AndroidOpacityAnimation::copy()
+PassRefPtr<AndroidAnimation> AndroidOpacityAnimation::copy()
 {
-    return new AndroidOpacityAnimation(this);
+    return adoptRef(new AndroidOpacityAnimation(this));
 }
 
 void AndroidOpacityAnimation::swapDirection()
@@ -164,7 +169,7 @@ void AndroidOpacityAnimation::swapDirection()
     m_fromValue = m_toValue;
 }
 
-bool AndroidOpacityAnimation::evaluate(double time)
+bool AndroidOpacityAnimation::evaluate(LayerAndroid* layer, double time)
 {
     float progress;
     if (!checkIterationsAndProgress(time, &progress))
@@ -174,20 +179,20 @@ bool AndroidOpacityAnimation::evaluate(double time)
         return true;
 
     float value = m_fromValue + ((m_toValue - m_fromValue) * progress);
-    m_result = AndroidOpacityAnimationValue::create(m_contentLayer.get(), value);
+    m_result = AndroidOpacityAnimationValue::create(layer, value);
     return true;
 }
 
-PassRefPtr<AndroidTransformAnimation> AndroidTransformAnimation::create(LayerAndroid* contentLayer,
-    const Animation* animation, double beginTime)
+PassRefPtr<AndroidTransformAnimation> AndroidTransformAnimation::create(
+                                                     const Animation* animation,
+                                                     double beginTime)
 {
-    return adoptRef(new AndroidTransformAnimation(contentLayer, animation, beginTime));
+    return adoptRef(new AndroidTransformAnimation(animation, beginTime));
 }
 
-AndroidTransformAnimation::AndroidTransformAnimation(LayerAndroid* contentLayer,
-                           const Animation* animation,
-                           double beginTime)
-    : AndroidAnimation(contentLayer, animation, beginTime),
+AndroidTransformAnimation::AndroidTransformAnimation(const Animation* animation,
+                                                     double beginTime)
+    : AndroidAnimation(animation, beginTime),
     m_doTranslation(false),
     m_doScaling(false),
     m_doRotation(false)
@@ -208,9 +213,9 @@ AndroidTransformAnimation::AndroidTransformAnimation(AndroidTransformAnimation*
 {
 }
 
-AndroidAnimation* AndroidTransformAnimation::copy()
+PassRefPtr<AndroidAnimation> AndroidTransformAnimation::copy()
 {
-    return new AndroidTransformAnimation(this);
+    return adoptRef(new AndroidTransformAnimation(this));
 }
 
 void AndroidTransformAnimation::setRotation(float fA, float tA)
@@ -272,7 +277,7 @@ void AndroidTransformAnimation::swapDirection()
     }
 }
 
-bool AndroidTransformAnimation::evaluate(double time)
+bool AndroidTransformAnimation::evaluate(LayerAndroid* layer, double time)
 {
     float progress;
     if (!checkIterationsAndProgress(time, &progress))
@@ -291,11 +296,12 @@ bool AndroidTransformAnimation::evaluate(double time)
 
     FloatPoint translation(x, y);
     FloatPoint3D scale(sx, sy, sz);
-    m_result = AndroidTransformAnimationValue::create(m_contentLayer.get(),
-                                                      translation, scale, a);
-    m_result->setDoTranslation(m_doTranslation);
-    m_result->setDoScaling(m_doScaling);
-    m_result->setDoRotation(m_doRotation);
+    RefPtr<AndroidTransformAnimationValue> result =
+        AndroidTransformAnimationValue::create(layer, translation, scale, a);
+    result->setDoTranslation(m_doTranslation);
+    result->setDoScaling(m_doScaling);
+    result->setDoRotation(m_doRotation);
+    m_result = result.release();
     return true;
 }
 

diff --git a/WebCore/platform/graphics/android/AndroidAnimation.h b/WebCore/platform/graphics/android/AndroidAnimation.h
index 1ecc22f..c4be10b 100644 (file)
--- a/WebCore/platform/graphics/android/AndroidAnimation.h
+++ b/WebCore/platform/graphics/android/AndroidAnimation.h
@@ -33,9 +33,6 @@ class AndroidAnimation;
 class GraphicsLayerAndroid;
 class TimingFunction;
 
-typedef Vector<RefPtr<AndroidAnimation> > AnimsVector;
-typedef HashMap<RefPtr<LayerAndroid>, AnimsVector* > LayersAnimsMap;
-
 class AndroidAnimationValue : public RefCounted<AndroidAnimationValue> {
   public:
     AndroidAnimationValue(LayerAndroid* layer) : m_layer(layer) { }
@@ -92,26 +89,23 @@ class AndroidTransformAnimationValue : public AndroidAnimationValue {
 
 class AndroidAnimation : public RefCounted<AndroidAnimation> {
   public:
-    AndroidAnimation(LayerAndroid* contentLayer,
-                     const Animation* animation,
+    AndroidAnimation(const Animation* animation,
                      double beginTime);
     AndroidAnimation(AndroidAnimation* anim);
 
     virtual ~AndroidAnimation();
-    virtual AndroidAnimation* copy() = 0;
+    virtual PassRefPtr<AndroidAnimation> copy() = 0;
     float currentProgress(double time);
     bool checkIterationsAndProgress(double time, float* finalProgress);
     virtual void swapDirection() = 0;
-    virtual bool evaluate(double time) = 0;
-    LayerAndroid* contentLayer() { return m_contentLayer.get(); }
+    virtual bool evaluate(LayerAndroid* layer, double time) = 0;
     static long instancesCount();
-    void setLayer(LayerAndroid* layer) { m_contentLayer = layer; }
     void setName(const String& name) { m_name = name; }
     String name() { return m_name; }
-    virtual PassRefPtr<AndroidAnimationValue> result() = 0;
+    virtual PassRefPtr<AndroidAnimationValue> result();
 
   protected:
-    RefPtr<LayerAndroid> m_contentLayer;
+    RefPtr<AndroidAnimationValue> m_result;
     double m_beginTime;
     double m_elapsedTime;
     double m_duration;
@@ -124,38 +118,33 @@ class AndroidAnimation : public RefCounted<AndroidAnimation> {
 
 class AndroidOpacityAnimation : public AndroidAnimation {
   public:
-    static PassRefPtr<AndroidOpacityAnimation> create(LayerAndroid* contentLayer,
-                                        float fromValue, float toValue,
-                                        const Animation* animation,
-                                        double beginTime);
-    AndroidOpacityAnimation(LayerAndroid* contentLayer,
-                            float fromValue, float toValue,
+    static PassRefPtr<AndroidOpacityAnimation> create(float fromValue,
+                                                     float toValue,
+                                                     const Animation* animation,
+                                                     double beginTime);
+    AndroidOpacityAnimation(float fromValue, float toValue,
                             const Animation* animation,
                             double beginTime);
     AndroidOpacityAnimation(AndroidOpacityAnimation* anim);
-    virtual AndroidAnimation* copy();
-    virtual PassRefPtr<AndroidAnimationValue> result() { return m_result.release(); }
+    virtual PassRefPtr<AndroidAnimation> copy();
 
     virtual void swapDirection();
-    virtual bool evaluate(double time);
+    virtual bool evaluate(LayerAndroid* layer, double time);
 
   private:
-    RefPtr<AndroidOpacityAnimationValue> m_result;
     float m_fromValue;
     float m_toValue;
 };
 
 class AndroidTransformAnimation : public AndroidAnimation {
   public:
-    static PassRefPtr<AndroidTransformAnimation> create(LayerAndroid* contentLayer,
-                                        const Animation* animation,
-                                        double beginTime);
-    AndroidTransformAnimation(LayerAndroid* contentLayer,
-                             const Animation* animation,
-                             double beginTime);
+    static PassRefPtr<AndroidTransformAnimation> create(
+                                                     const Animation* animation,
+                                                     double beginTime);
+    AndroidTransformAnimation(const Animation* animation, double beginTime);
 
     AndroidTransformAnimation(AndroidTransformAnimation* anim);
-    virtual AndroidAnimation* copy();
+    virtual PassRefPtr<AndroidAnimation> copy();
 
     void setOriginalPosition(FloatPoint position) { m_position = position; }
     void setRotation(float fA, float tA);
@@ -164,11 +153,9 @@ class AndroidTransformAnimation : public AndroidAnimation {
     void setScale(float fX, float fY, float fZ,
                   float tX, float tY, float tZ);
     virtual void swapDirection();
-    virtual bool evaluate(double time);
-    virtual PassRefPtr<AndroidAnimationValue> result() { return m_result.release(); }
+    virtual bool evaluate(LayerAndroid* layer, double time);
 
   private:
-    RefPtr<AndroidTransformAnimationValue> m_result;
     bool m_doTranslation;
     bool m_doScaling;
     bool m_doRotation;

diff --git a/WebCore/platform/graphics/android/GraphicsLayerAndroid.cpp b/WebCore/platform/graphics/android/GraphicsLayerAndroid.cpp
index 068a55b..bcfe13d 100644 (file)
--- a/WebCore/platform/graphics/android/GraphicsLayerAndroid.cpp
+++ b/WebCore/platform/graphics/android/GraphicsLayerAndroid.cpp
@@ -105,7 +105,7 @@ GraphicsLayerAndroid::GraphicsLayerAndroid(GraphicsLayerClient* client) :
     m_currentTranslateY(0),
     m_currentPosition(0, 0)
 {
-    m_contentLayer = new LayerAndroid(true);
+    m_contentLayer = adoptRef(new LayerAndroid(true));
     if (client) {
       RenderLayerBacking* backing = static_cast<RenderLayerBacking*>(client);
       RenderLayer* renderLayer = backing->owningLayer();
@@ -526,8 +526,7 @@ bool GraphicsLayerAndroid::createAnimationFromKeyframes(const KeyframeValueList&
             static_cast<const FloatAnimationValue*>(valueList.at(0));
         const FloatAnimationValue* endVal =
             static_cast<const FloatAnimationValue*>(valueList.at(1));
-        RefPtr<AndroidOpacityAnimation> anim = AndroidOpacityAnimation::create(m_contentLayer.get(),
-                                                                               startVal->value(),
+        RefPtr<AndroidOpacityAnimation> anim = AndroidOpacityAnimation::create(startVal->value(),
                                                                                endVal->value(),
                                                                                animation,
                                                                                beginTime);
@@ -711,8 +710,7 @@ bool GraphicsLayerAndroid::createTransformAnimationsFromKeyframes(const Keyframe
         }
     }
 
-    RefPtr<AndroidTransformAnimation> anim = AndroidTransformAnimation::create(m_contentLayer.get(),
-                                                                               animation, beginTime);
+    RefPtr<AndroidTransformAnimation> anim = AndroidTransformAnimation::create(animation, beginTime);
 
     if (keyframesName.isEmpty())
         anim->setName(propertyIdToString(valueList.property()));

diff --git a/WebCore/platform/graphics/android/LayerAndroid.cpp b/WebCore/platform/graphics/android/LayerAndroid.cpp
index 3b5d5b5..1788f2d 100644 (file)
--- a/WebCore/platform/graphics/android/LayerAndroid.cpp
+++ b/WebCore/platform/graphics/android/LayerAndroid.cpp
@@ -105,11 +105,7 @@ LayerAndroid::LayerAndroid(LayerAndroid* layer) :
 
     KeyframesMap::const_iterator end = layer->m_animations.end();
     for (KeyframesMap::const_iterator it = layer->m_animations.begin(); it != end; ++it)
-        m_animations.add((it->second)->name(), adoptRef((it->second)->copy()));
-
-    end = m_animations.end();
-    for (KeyframesMap::const_iterator it = m_animations.begin(); it != end; ++it)
-        (it->second)->setLayer(this);
+        m_animations.add((it->second)->name(), (it->second)->copy());
 
     gDebugLayerAndroidInstances++;
 }
@@ -127,10 +123,11 @@ static int gDebugNbAnims = 0;
 Vector<RefPtr<AndroidAnimationValue> >* LayerAndroid::evaluateAnimations() const
 {
     double time = WTF::currentTime();
-    Vector<RefPtr<AndroidAnimationValue> >* result = new Vector<RefPtr<AndroidAnimationValue> >();
+    Vector<RefPtr<AndroidAnimationValue> >* results = new Vector<RefPtr<AndroidAnimationValue> >();
     gDebugNbAnims = 0;
-    if (evaluateAnimations(time, result))
-      return result;
+    if (evaluateAnimations(time, results))
+      return results;
+    delete results;
     return 0;
 }
 
@@ -144,22 +141,25 @@ bool LayerAndroid::hasAnimations() const
 }
 
 bool LayerAndroid::evaluateAnimations(double time,
-                                      Vector<RefPtr<AndroidAnimationValue> >* result) const
+                                      Vector<RefPtr<AndroidAnimationValue> >* results) const
 {
     bool hasRunningAnimations = false;
     for (unsigned int i = 0; i < m_children.size(); i++) {
-        if (m_children[i]->evaluateAnimations(time, result))
+        if (m_children[i]->evaluateAnimations(time, results))
             hasRunningAnimations = true;
     }
     KeyframesMap::const_iterator end = m_animations.end();
     for (KeyframesMap::const_iterator it = m_animations.begin(); it != end; ++it) {
         gDebugNbAnims++;
-        if ((it->second)->evaluate(time)) {
-            result->append((it->second)->result());
+        LayerAndroid* currentLayer = const_cast<LayerAndroid*>(this);
+        if ((it->second)->evaluate(currentLayer, time)) {
+            RefPtr<AndroidAnimationValue> result = (it->second)->result();
+            if (result)
+                results->append(result);
             hasRunningAnimations = true;
         }
     }
-    
+
     return hasRunningAnimations;
 }
 

diff --git a/WebCore/platform/graphics/android/LayerAndroid.h b/WebCore/platform/graphics/android/LayerAndroid.h
index 467c7dd..8436921 100644 (file)
--- a/WebCore/platform/graphics/android/LayerAndroid.h
+++ b/WebCore/platform/graphics/android/LayerAndroid.h
@@ -83,7 +83,7 @@ public:
     void removeAnimation(const String& name);
     Vector<RefPtr<AndroidAnimationValue> >* evaluateAnimations() const;
     bool evaluateAnimations(double time,
-             Vector<RefPtr<AndroidAnimationValue> >* result) const;
+             Vector<RefPtr<AndroidAnimationValue> >* results) const;
     bool hasAnimations() const;
 
 private:

diff --git a/WebKit/android/nav/WebView.cpp b/WebKit/android/nav/WebView.cpp
index dae93fc..e2a7708 100644 (file)
--- a/WebKit/android/nav/WebView.cpp
+++ b/WebKit/android/nav/WebView.cpp
@@ -1533,13 +1533,10 @@ static void nativeDrawLayers(JNIEnv *env, jobject obj,
 #endif
 }
 
-static void nativeUpdateLayers(JNIEnv *env, jobject obj,
-                               jint layer, jint updates)
+static void nativeUpdateLayers(JNIEnv *env, jobject obj, jint updates)
 {
     if (!env)
         return;
-    if (!layer)
-        return;
     if (!updates)
         return;
 
@@ -2138,7 +2135,7 @@ static JNINativeMethod gJavaWebViewMethods[] = {
         (void*) nativeEvaluateLayersAnimations },
     { "nativeDrawLayers", "(IIIIIFLandroid/graphics/Canvas;)V",
         (void*) nativeDrawLayers },
-    { "nativeUpdateLayers", "(II)V",
+    { "nativeUpdateLayers", "(I)V",
         (void*) nativeUpdateLayers },
     { "nativeDrawMatches", "(Landroid/graphics/Canvas;)V",
         (void*) nativeDrawMatches },