Attachment 174539 Details for Bug 102441 – Proposed refactoring.

[patch] Proposed refactoring.

bug-102441-20121115160520.patch (text/plain), 4.61 KB, created by Julien Chaffraix on 2012-11-15 16:07:23 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Julien Chaffraix

Created: 2012-11-15 16:07:23 PST

Size: 4.61 KB

patch

obsolete

>Subversion Revision: 134295
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 6827f4dad89513e5870720514ae7de805d028dc6..288bb924e1928016ef27a1ec4041bcd88ce7cfb9 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,26 @@
>+2012-11-15  Julien Chaffraix  <jchaffraix@webkit.org>
>+
>+        RenderGrid should have a function to resolve grid position
>+        https://bugs.webkit.org/show_bug.cgi?id=102441
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The code was doing this conversion implicitly inside RenderGrid::findChildLogicalPosition.
>+        Also note that we also provided a fallback by returning LayoutPoint() (ie the (0, 0) position
>+        on the grid) if we couldn't handle the value. The explicit conversion is needed in order to
>+        support render areas and add a proper grid model to RenderGrid.
>+
>+        No expected change in behavior.
>+
>+        * rendering/RenderGrid.h:
>+        * rendering/RenderGrid.cpp:
>+        (WebCore::RenderGrid::resolveGridPosition):
>+        Added this new function to handle the conversion. We re-use Length but should never see
>+        a lot of the <length> values so I added some ASSERTs to enforce and catch that.
>+
>+        (WebCore::RenderGrid::findChildLogicalPosition):
>+        Simplified the function now that it just use resolveGridPosition.
>+
> 2012-11-12  Christophe Dumez  <christophe.dumez@intel.com>
> 
>         Fix memory leak in createSurfaceForBackingStore()
>diff --git a/Source/WebCore/rendering/RenderGrid.cpp b/Source/WebCore/rendering/RenderGrid.cpp
>index 6e293cc0c4895af59e85891d12792759c9eb8c28..c5b903c305dd7e1906dc46d2a7f316e71412c155 100644
>--- a/Source/WebCore/rendering/RenderGrid.cpp
>+++ b/Source/WebCore/rendering/RenderGrid.cpp
>@@ -175,23 +175,47 @@ void RenderGrid::layoutGridItems()
>         setLogicalHeight(logicalHeight() + rowTracks[i].m_usedBreadth);
> }
> 
>-LayoutPoint RenderGrid::findChildLogicalPosition(RenderBox* child, const Vector<GridTrack>& columnTracks, const Vector<GridTrack>& rowTracks)
>+size_t RenderGrid::resolveGridPosition(const Length& position) const
> {
>-    Length column = child->style()->gridItemColumn();
>-    Length row = child->style()->gridItemRow();
>-
>-    // FIXME: What does a non-positive integer mean for a column/row?
>-    if (!column.isPositive() || !row.isPositive())
>-        return LayoutPoint();
>-
>     // FIXME: Handle other values for grid-{row,column} like ranges or line names.
>-    if (!column.isFixed() || !row.isFixed())
>-        return LayoutPoint();
>+    switch (position.type()) {
>+    case Fixed:
>+        // FIXME: What does a non-positive integer mean for a column/row?
>+        if (!position.isPositive())
>+            return 0;
>+
>+        return position.intValue() - 1;
>+    case Auto:
>+        // FIXME: We should follow 'grid-auto-flow' for resolution.
>+        // Until then, we use the 'grid-auto-flow: none' behavior (which is the default)
>+        // and resolve 'auto' as the first row / column.
>+        return 0;
>+    case Relative:
>+    case Percent:
>+    case Intrinsic:
>+    case MinIntrinsic:
>+    case MinContent:
>+    case MaxContent:
>+    case FillAvailable:
>+    case FitContent:
>+    case Calculated:
>+    case ViewportPercentageWidth:
>+    case ViewportPercentageHeight:
>+    case ViewportPercentageMin:
>+    case Undefined:
>+        break;
>+    }
>+    ASSERT_NOT_REACHED();
>+    return 0;
>+}
> 
>-    size_t columnTrack = static_cast<size_t>(column.intValue()) - 1;
>-    size_t rowTrack = static_cast<size_t>(row.intValue()) - 1;
>+LayoutPoint RenderGrid::findChildLogicalPosition(RenderBox* child, const Vector<GridTrack>& columnTracks, const Vector<GridTrack>& rowTracks)
>+{
>+    size_t columnTrack = resolveGridPosition(child->style()->gridItemColumn());
>+    size_t rowTrack = resolveGridPosition(child->style()->gridItemRow());
> 
>     LayoutPoint offset;
>+    // FIXME: |columnTrack| and |rowTrack| should be smaller than our column / row count.
>     for (size_t i = 0; i < columnTrack && i < columnTracks.size(); ++i)
>         offset.setX(offset.x() + columnTracks[i].m_usedBreadth);
>     for (size_t i = 0; i < rowTrack && i < rowTracks.size(); ++i)
>diff --git a/Source/WebCore/rendering/RenderGrid.h b/Source/WebCore/rendering/RenderGrid.h
>index e5017fae0fdaacc8ceeffc801ddad12d7758f583..b3f4ca50f13e791111c9dff56ae68a6e02a45b71 100644
>--- a/Source/WebCore/rendering/RenderGrid.h
>+++ b/Source/WebCore/rendering/RenderGrid.h
>@@ -50,6 +50,7 @@ private:
>     void layoutGridItems();
> 
>     LayoutPoint findChildLogicalPosition(RenderBox*, const Vector<GridTrack>& columnTracks, const Vector<GridTrack>& rowTracks);
>+    size_t resolveGridPosition(const Length&) const;
> };
> 
> } // namespace WebCore

Actions: View | Formatted Diff | Diff

Attachments on bug 102441: 174539