Wrap CSS length conversion arguments in an object

Source/core/css/CSSToLengthConversionData.cpp

+/*
+ * Copyright (C) 2013 Google Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ *     * Neither the name of Google Inc. nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+#include "core/css/CSSToLengthConversionData.h"
+
+#include "core/rendering/style/RenderStyle.h"
+#include "wtf/Assertions.h"
+
+namespace WebCore {
+
+CSSToLengthConversionData::CSSToLengthConversionData(const RenderStyle* style, const RenderStyle* rootStyle, float zoom, bool computingFontSize)
+    : m_style(style)
+    , m_rootStyle(rootStyle)
+    , m_zoom(zoom)
+    , m_computingFontSize(computingFontSize)
+{
+    ASSERT(zoom == -1 || zoom > 0);

[Julien - ping for review, 2013/11/26 05:31:07]

I don't understand why we need the default value is -1 (magic constant btw). Why
can't we just fill m_zoom from the current style's effective zoom in the
constructor?

[Timothy Loh, 2013/11/27 05:44:51]

Note that we'd also need to change the zoom whenever the style or style's zoom
is changed or style is changed (this is probably just
StyleResolverState::setStyle/setZoom/setEffectiveZoom). I went with changing the
magic constant to a separate boolean which says if we want to use the style's
zoom (this requires an extra ctor). WDYT?

+}
+
+float CSSToLengthConversionData::zoom() const
+{
+    if (m_zoom > 0)
+        return m_zoom;
+    return m_style ? m_style->effectiveZoom() : 1;
+}
+
+CSSToLengthConversionData CSSToLengthConversionData::copyWithAdjustedZoom(float newZoom) const

[Julien - ping for review, 2013/11/26 05:31:07]

The need for this function makes me wonder if this is helping us. As you point
out, the new class requires some copying due to changes in zoom, which is really
annoying.

One solution would be to move zoom out of the class and pass it along to
computeLength(). Another solution is to expose some setZoom setter.

[Timothy Loh, 2013/11/27 05:44:51]

Agreed that this is a bit annoying. Having a setter is probably not what we
want; we'd have to probably set the zoom back after and it makes it a bit messy
(also we'll have to unconst the StyleResolverState getter). Passing the zoom
alongside the rest of the data is what this patch is trying to avoid.

The copying here is pretty cheap, and presumably we do about as much currently
anyway when all the arguments get plumbed through everywhere.

+{
+    ASSERT(newZoom == -1 || newZoom > 0);
+    CSSToLengthConversionData copy = *this;
+    copy.m_zoom = newZoom;
+    return copy;
+}
+
+} // namespace WebCore