Fixes style and adds some comments to layers code

Also removes FrameCachePermission::AllowNewest. This should have been removed
as part of https://android-git.corp.google.com/g/#/c/2712.

Bug: 5262656
Change-Id: I4d819cc5087653e4a1180d04ca0b5b20db7e2305

4 files changed, 36 insertions, 19 deletions

diff --git a/Source/WebCore/platform/graphics/android/Layer.h b/Source/WebCore/platform/graphics/android/Layer.h
index 6450fde..107c457 100644
--- a/Source/WebCore/platform/graphics/android/Layer.h
+++ b/Source/WebCore/platform/graphics/android/Layer.h
@@ -122,14 +122,17 @@ private:
         kInheritFromRootTransform_Flag = 0x01
     };
 
-    Layer*    fParent;
-    SkScalar    m_opacity;
-    SkSize      m_size;
-    SkPoint     m_position;
-    SkPoint     m_anchorPoint;
-    SkMatrix    fMatrix;
-    SkMatrix    fChildrenMatrix;
-    uint32_t    fFlags;
+    Layer* fParent;
+    SkScalar m_opacity;
+    SkSize m_size;
+    // The position of the origin of the layer, relative to the parent layer.
+    SkPoint m_position;
+    // The point in the layer used as the origin for local transformations,
+    // expressed as a fraction of the layer size.
+    SkPoint m_anchorPoint;
+    SkMatrix fMatrix;
+    SkMatrix fChildrenMatrix;
+    uint32_t fFlags;
 
     SkTDArray<Layer*> m_children;
 
diff --git a/Source/WebCore/platform/graphics/android/LayerAndroid.cpp b/Source/WebCore/platform/graphics/android/LayerAndroid.cpp
index 48dcaaa..95ecb4a 100644
--- a/Source/WebCore/platform/graphics/android/LayerAndroid.cpp
+++ b/Source/WebCore/platform/graphics/android/LayerAndroid.cpp
@@ -267,7 +267,7 @@ void LayerAndroid::removeAnimationsForKeyframes(const String& name)
 }
 
 // We only use the bounding rect of the layer as mask...
-// TODO: use a real mask?
+// FIXME: use a real mask?
 void LayerAndroid::setMaskLayer(LayerAndroid* layer)
 {
     if (layer)
@@ -546,7 +546,7 @@ void LayerAndroid::updatePositions()
     if (!m_isFixed) {
         // turn our fields into a matrix.
         //
-        // TODO: this should happen in the caller, and we should remove these
+        // FIXME: this should happen in the caller, and we should remove these
         // fields from our subclass
         SkMatrix matrix;
         GLUtils::toSkMatrix(matrix, m_transform);
@@ -926,6 +926,11 @@ SkRect LayerAndroid::subtractLayers(const SkRect& visibleRect) const
 {
     SkRect result;
     if (m_recordingPicture) {
+        // FIXME: This seems wrong. localToGlobal() applies the full local transform,
+        // se surely we should operate globalMatrix on size(), not bounds() with
+        // the position removed? Perhaps we never noticed the bug because most
+        // layers don't use a local transform?
+        // See http://b/5338388
         SkRect globalRect = bounds();
         globalRect.offset(-getPosition()); // localToGlobal adds in position
         SkMatrix globalMatrix;
diff --git a/Source/WebCore/platform/graphics/android/LayerAndroid.h b/Source/WebCore/platform/graphics/android/LayerAndroid.h
index 7192aaf..a223499 100644
--- a/Source/WebCore/platform/graphics/android/LayerAndroid.h
+++ b/Source/WebCore/platform/graphics/android/LayerAndroid.h
@@ -109,6 +109,10 @@ public:
     void setBackfaceVisibility(bool value) { m_backfaceVisibility = value; }
     void setTransform(const TransformationMatrix& matrix) { m_transform = matrix; }
     FloatPoint translation() const;
+    // Returns a rect describing the bounds of the layer with the local
+    // transformation applied, expressed relative to the parent layer.
+    // FIXME: Currently we use only the translation component of the local
+    // transformation.
     SkRect bounds() const;
     IntRect clippedRect() const;
     bool outsideViewport();
@@ -183,9 +187,10 @@ public:
 
     SkPicture* picture() const { return m_recordingPicture; }
 
-    // remove layers bounds from visible rectangle to show what can be
-    // scrolled into view; returns original minus layer bounds in global space.
-    SkRect subtractLayers(const SkRect& visibleRect) const;
+    // Given a rect in global space, subtracts from it the bounds of this layer
+    // and of all of its children. Returns the bounding rectangle of the result,
+    // in global space.
+    SkRect subtractLayers(const SkRect&) const;
 
     void dumpLayers(FILE*, int indentLevel) const;
     void dumpToLog() const;
diff --git a/Source/WebCore/platform/graphics/android/ScrollableLayerAndroid.h b/Source/WebCore/platform/graphics/android/ScrollableLayerAndroid.h
index c3fdecd..b59b4e1 100644
--- a/Source/WebCore/platform/graphics/android/ScrollableLayerAndroid.h
+++ b/Source/WebCore/platform/graphics/android/ScrollableLayerAndroid.h
@@ -42,15 +42,16 @@ public:
 
     virtual LayerAndroid* copy() const { return new ScrollableLayerAndroid(*this); }
 
-    // Returns true if the content position has changed.
-    bool scrollTo(int dx, int dy);
+    // Scrolls to the given position in the layer.
+    // Returns whether or not any scrolling was required.
+    bool scrollTo(int x, int y);
 
-    // Fills the rect with the current scroll offset and the maximum scroll.
+    // Fills the rect with the current scroll offset and the maximum scroll offset.
     // fLeft   = scrollX
     // fTop    = scrollY
-    // fRight  = maxX
-    // fBottom = maxY
-    void getScrollRect(SkIRect* out) const;
+    // fRight  = maxScrollX
+    // fBottom = maxScrollY
+    void getScrollRect(SkIRect*) const;
 
     void setScrollLimits(float x, float y, float width, float height)
     {
@@ -61,6 +62,9 @@ public:
     friend LayerAndroid* android::deserializeLayer(SkStream* stream);
 
 private:
+    // The position of the visible area of the layer, relative to the parent
+    // layer. This is fixed during scrolling. We acheive scrolling by modifying
+    // the position of the layer.
     SkRect m_scrollLimits;
 };